Event Manager Impementation #180

Закрито
sarah хоче злити 0 комітів з eventmanager в master
sarah прокоментував(ла) 2019-01-02 22:37:37 +00:00
Власник

Barebones Event Manager Implementation.

The idea is that the cwtch app will implement an Event Manager, and each Subsystem (UI, Storage, ProtocolEngines) will have (one or more) EventQueue(s) that they can use to process the events that arrive into their subsystems.

Currently has 100% Test Coverage.

Barebones Event Manager Implementation. The idea is that the cwtch app will implement an Event Manager, and each Subsystem (UI, Storage, ProtocolEngines) will have (one or more) EventQueue(s) that they can use to process the events that arrive into their subsystems. Currently has 100% Test Coverage.
buildbot прокоментував(ла) 2019-01-03 01:28:35 +00:00
Учасник
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/321
dan прокоментував(ла) 2019-01-03 01:42:56 +00:00
Власник

Queue and Manager need .Shutdown() methods, otherwise they leak threads

it prolly doesnt matter as much (I cant make as strong an argument for it, but maybe we should discus and settle on a style) but i like the style

  • lowerCase struct
  • UpperCase interface on it

It's not clear to me how Queue is to be used, it has an internal process() go routine but that just shunts messages from a channel to an internal queue. it's still left to the implementer to implement another listen thread on the queue? which doesn't have blocking options? Off the top of my head (still warming up tho) what about taking in a process function as a initialize argument that process calls on the event. Then we don't even need the queue for storage or another go routine for the client to implement

Queue and Manager need .Shutdown() methods, otherwise they leak threads it prolly doesnt matter as much (I cant make as strong an argument for it, but maybe we should discus and settle on a style) but i like the style - lowerCase struct - UpperCase interface on it It's not clear to me how Queue is to be used, it has an internal process() go routine but that just shunts messages from a channel to an internal queue. it's still left to the implementer to implement another listen thread on the queue? which doesn't have blocking options? Off the top of my head (still warming up tho) what about taking in a process function as a initialize argument that process calls on the event. Then we don't even need the queue for storage or another go routine for the client to implement
sarah прокоментував(ла) 2019-01-03 02:19:57 +00:00
Author
Власник

Will add shutdown handler.

Queue is used to prevent slow subsystems from shutting down the event bus (as go channels are blocking) - instead of blocking we automatically take the event off the event bus channel and into a private subsystem queue where we can better manage the backlog (it also allows us to identify when a given subsystem is running slow - as we can check the backlog length).

The implementer e.g. Storage has a handler which calls .Next() on Queue (which now that I think through this means that .Next() should almost certainly be blocking) - and then processes whatever event it would like.

Similar reasoning applies to not using callbacks, a slow/broken callback function would block everything - it's not too big a deal now, but when we start talking about plugins etc. I'd like to isolate each process well from the others.

I'd rather not go overboard on the struct/interface dichotomy for now - but we can discuss next time we get together.

Will add shutdown handler. Queue is used to prevent slow subsystems from shutting down the event bus (as go channels are blocking) - instead of blocking we automatically take the event off the event bus channel and into a private subsystem queue where we can better manage the backlog (it also allows us to identify when a given subsystem is running slow - as we can check the backlog length). The implementer e.g. Storage has a handler which calls .Next() on Queue (which now that I think through this means that .Next() should almost certainly be blocking) - and then processes whatever event it would like. Similar reasoning applies to not using callbacks, a slow/broken callback function would block everything - it's not too big a deal now, but when we start talking about plugins etc. I'd like to isolate each process well from the others. I'd rather not go overboard on the struct/interface dichotomy for now - but we can discuss next time we get together.
dan прокоментував(ла) 2019-01-03 18:52:42 +00:00
Власник

cool makes sense
so yeah, .Next() needs to block,

try writing a simple actual use case, and you'll quickly see what you need

cool makes sense so yeah, .Next() needs to block, try writing a simple actual use case, and you'll quickly see what you need
buildbot прокоментував(ла) 2019-01-03 20:22:39 +00:00
Учасник
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/323
buildbot прокоментував(ла) 2019-01-03 20:29:24 +00:00
Учасник
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/325
buildbot прокоментував(ла) 2019-01-03 20:38:18 +00:00
Учасник
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/327
sarah прокоментував(ла) 2019-01-03 20:57:59 +00:00
Author
Власник

Simplified, Shutdown and Explicit Backlog Handling Added.

event.Queue is now a very light wrapper around channels, though my intuition says this likely won't be the case for long - perhaps a case of premature api optimization but once this is merged I will begin splitting out parts of peer/ into the Protocol Engine subsystem and using this code directly - which should start to poke this further.

Simplified, Shutdown and Explicit Backlog Handling Added. event.Queue is now a very light wrapper around channels, though my intuition says this likely won't be the case for long - perhaps a case of premature api optimization but once this is merged I will begin splitting out parts of peer/ into the Protocol Engine subsystem and using this code directly - which should start to poke this further.
buildbot прокоментував(ла) 2019-01-03 21:00:16 +00:00
Учасник
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/329
dan прокоментував(ла) 2019-01-03 22:03:39 +00:00
Власник

lgtm!

did you want erinn to review too before merge?

lgtm! did you want erinn to review too before merge?
dan прокоментував(ла) 2019-01-04 02:01:27 +00:00
Власник

the currently fast way to use it then is like

for event := queue.Next() {
  go process(event)
}

so we can play with that, next step may be pass in a process func to initialize and have the queue processor back and call go process(event)

like you said, we can try a few things once we have usage :)

the currently fast way to use it then is like ``` for event := queue.Next() { go process(event) } ``` so we can play with that, next step may be pass in a process func to initialize and have the queue processor back and call go process(event) like you said, we can try a few things once we have usage :)
erinn прокоментував(ла) 2019-01-07 20:20:36 +00:00
Власник

add a test for publishing an event with a type that has no subscribers? other than that, lgtm (Y)

add a test for publishing an event with a type that has no subscribers? other than that, lgtm (Y)
sarah закрив цей запит на злиття 2019-01-08 00:39:50 +00:00

Pull request closed

Підпишіться щоб приєднатися до обговорення.
Немає рецензентів
Етап відсутній
Немає виконавця
4 учасників
Сповіщення
Дата завершення
Термін дії не дійсний або знаходиться за межами допустимого діапазону. Будь ласка використовуйте формат 'yyyy-mm-dd'.

Термін виконання не встановлений.

Залежності

No dependencies set.

Reference: cwtch.im/cwtch#180
No description provided.