Project

General

Profile

Task #3802

Task #3801: Improve EventBus implementation

Unify implementation and API of ClusteredEventBus and LocalEventBus

Added by Andrzej Wójcik IoT 1 CloudTigaseTeam over 3 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Normal
Target version:
Start date:
2016-01-04
Due date:
2016-02-22
% Done:

100%

Estimated time:
40.00 h
Database:
n/a

Description

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).

Associated revisions

Revision 916f80ca (diff)
Added by Bartosz Małkowski TigaseTeam over 3 years ago

Task #3802: Unify implementation and API of ClusteredEventBus and LocalEventBus

Revision 4ed7ccf3 (diff)
Added by Bartosz Małkowski TigaseTeam about 3 years ago

Task #3802: Unify implementation and API of ClusteredEventBus and LocalEventBus

Revision e8f9c4da (diff)
Added by Andrzej Wójcik IoT 1 CloudTigaseTeam about 3 years ago

Issue #3802 - updated Shutdown implementation to work using new EventBus

Revision 2bfb7c28 (diff)
Added by Andrzej Wójcik IoT 1 CloudTigaseTeam about 3 years ago

Issue #3802 - added test cases - important for #3802 and Shutdown event to work (must pass to make sure Shutdown event will work correctly)

Revision 7bdb9b90 (diff)
Added by Bartosz Małkowski TigaseTeam about 3 years ago

Task #3802: Unify implementation and API of ClusteredEventBus and LocalEventBus

fix errors in serializer
fix error in listeners executions

Revision 617673be (diff)
Added by Andrzej Wójcik IoT 1 CloudTigaseTeam about 3 years ago

Issue #3802 - added licensing informations and minor change in retrieval of listeners for event

Revision 27115050 (diff)
Added by Bartosz Małkowski TigaseTeam about 3 years ago

Task #3802: Unify implementation and API of ClusteredEventBus and LocalEventBus

  • rename removeListenerHandler into removeHandler to match addHandler method
  • internal classes and interfaces moved to impl package
  • added missing copyright headers
  • added Serializer interface

History

#1 Updated by Andrzej Wójcik IoT 1 CloudTigaseTeam over 3 years ago

  • Assignee changed from Artur Hefczyc to Bartosz Małkowski
  • Estimated time set to 40.00 h

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

#2 Avatar?id=6098&size=24x24 Updated by Bartosz Małkowski TigaseTeam over 3 years ago

  • % Done changed from 0 to 50

#3 Avatar?id=6098&size=24x24 Updated by Bartosz Małkowski TigaseTeam about 3 years ago

  • Status changed from New to In QA
  • Assignee changed from Bartosz Małkowski to Andrzej Wójcik
  • % Done changed from 50 to 100

Done.

%andrzej.wojcik would you like to update events to new version event "shutdown"?

#4 Updated by Andrzej Wójcik IoT 1 CloudTigaseTeam about 3 years ago

  • Status changed from In QA to Feedback
  • Assignee changed from Andrzej Wójcik to Bartosz Małkowski

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.

#5 Avatar?id=6098&size=24x24 Updated by Bartosz Małkowski TigaseTeam about 3 years ago

  • Status changed from Feedback to In QA
  • Assignee changed from Bartosz Małkowski to Andrzej Wójcik

Fixed. All tests passes.

#6 Updated by Andrzej Wójcik IoT 1 CloudTigaseTeam about 3 years ago

  • Status changed from In QA to Feedback
  • Assignee changed from Andrzej Wójcik to Bartosz Małkowski

Few inputs from me about new implementation:

  1. Almost all looks OK and works as expected

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

  3. 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.

  4. 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.

  5. 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

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

  7. It would be good to allow to use other serializer for events - ie. to be able to add and use some custom serializer

#7 Avatar?id=6098&size=24x24 Updated by Bartosz Małkowski TigaseTeam about 3 years ago

  • Status changed from Feedback to In QA
  • Assignee changed from Bartosz Małkowski to Andrzej Wójcik

Implemented.

#8 Updated by Andrzej Wójcik IoT 1 CloudTigaseTeam about 3 years ago

  • Due date set to 2016-02-22
  • Status changed from In QA to Closed

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).

Also available in: Atom PDF