#180 Event Manager Impementation

Closed
sarah wants to merge 0 commits from eventmanager into master
sarah commented 11 months ago

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.
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/321
dan commented 11 months ago
Owner

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 commented 11 months ago
Owner

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 commented 11 months ago
Owner

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
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/323
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/325
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/327
sarah commented 11 months ago
Owner

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.
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/329
dan commented 11 months ago
Owner

lgtm!

did you want erinn to review too before merge?

lgtm! did you want erinn to review too before merge?
dan commented 11 months ago
Owner

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 commented 11 months ago
Owner

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)
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No Milestone
No Assignees
4 Participants
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
Cancel
Save
There is no content yet.