Unify implementation and API of ClusteredEventBus and LocalEventBus (#623)

Andrzej Wójcik (Tigase) opened 9 years ago

Due Date
2016-02-21

Right now we have 2 separate implementations of EventBus:

DefaultClusteredEventBus - for clustered events based on Element, name and XMLNS
DefaultLocalEventBus - for local events based on Event interface and it's implementations

It would be good to have one API so we would not have to keep to separate APIs and implementations and think which is better for particular use case.

While I talked Bartosz we got to conclusion that it would be best to have 1 API based on Event interface and possible second interface which would be responsible for conversion of Event class instance into Element and back to allow this event to be forwarded between cluster nodes if needed.

Usage of Event interfaces is in this case required as we sometimes need to pass more information that just element, ie. instance of a Map/DMap like in DMap (clustered map implementation).

Parent Issue
- Improve EventBus implementation (#622) Closed

Activities

Andrzej Wójcik (Tigase) commented 9 years ago

I assume in 40h it will be possible to implement this feature with unit test and documentation.

%bmalkow - correct my estimate if I'm wrong
Bartosz Małkowski commented 9 years ago

Done.

%andrzej.wojcik would you like to update events to new version event "shutdown"?
Andrzej Wójcik (Tigase) commented 9 years ago

I updated implementation of Shutdown event and added test cases with inheritance, method visibility and serialization which I think are important to pass before we can mark this task as done.
Bartosz Małkowski commented 9 years ago

Fixed. All tests passes.
Andrzej Wójcik (Tigase) commented 9 years ago
Few inputs from me about new implementation:

Almost all looks OK and works as expected

I'm not sure that EventBusException should extend @RuntimeException@. Could you explain this choice?

In EventBusImplementation we have addHandler method and removeListenerHandler@. I think it would be good to look into naming of methods and clean up - ie. rename @removeListenerHandler into removeHandler to match addHandler method.

In EventBusImplementation in getListenersForEvent method, I see 2 loops - one creates list of classes and second retrieves listener - as I expect this may be called very ofter I changed this into single loop.

Now we have a lot of classes in main package tigase.events@, maybe it would be good to separate them, ie. into @impl@, @reflection@, @annotations

Missing headers in newly added java files - added some, but worth checking.

It would be good to allow to use other serializer for events - ie. to be able to add and use some custom serializer
Bartosz Małkowski commented 9 years ago

Implemented.
Andrzej Wójcik (Tigase) commented 9 years ago

Implementatin works ok.

%bmalkow - I made few changes to EventBus while I was working on and testing #1601

I changed default executor of event handlers and increased number of threads to (by default) use 4 * no of CPU - same as XMPPProcessors as in my opinion this feature required more threads and to algorithm to scale when there is more CPUs available.

I also implemented support for serialization of collections in events (time logged in #1601 as it was required there).
Login to comment

Type	Task
Priority	Normal
Assignee	Andrzej Wójcik (Tigase)
RedmineID	3802
Version	tigase-server-8.0.0
Estimation	0
Spent time	0

Issue Votes (0)

Watchers (0)

Reference

tigase/_server/server-core#623