Performance issue on big amount of upcasters (Checked on Axon v4.0.3)

Imported from Google Groups
1

Dear Axon developers,

I have found a possible performance issue in the method
GenericUpcasterChain.upcast
Link to the method
which I would like to share with you and get feedback.

In the current implementation for each event read by EventStorageEngine, canUpcast method of all upcasters is called (internally called from the overridden method upcast of Upcaster class), even if a very small portion of registered upcasters is related to the payload type of the event.
For example, let’s assume:

  1. I have an event which registered with a payload type eventpackage.EventXCreated
  2. I have registered n upcasters for up-casting deferent version of the event with payload type eventpackage.EventXCreated
  3. Besides of above-mentioned upcasters I have registered m upcasters for other events, which of-course won’t be used for events with eventpackage.EventXCreated payload type
    Then the amount of canUpcast method calls for a single incoming event can be measured with the following formula
    O(n + m)

Before I’ll jump the suggestion of possible fix for this issue, let me tell why I consider this us performance issue

  1. Axon doesn’t have control of over amount of upcasters registered in the chain. this number can be very big depending on the complexity of task and amount of changes needed in the event schemas during the time
  2. Axon doesn’t have control over performance of canUpcast method. in case of needing to do complex checking inside this overridden method, each canUpcast method can come with a big cost
  3. Amount of canUpcast calls for the specific event payload type should not have a linear dependency from the amount of all registered upcasters.

I can suggest the following possible solution (maybe it will need some tweaks)
Step #1
Create the intermediate abstract class which will be constructed with payload type (Class<?>), and will have a getter method for that member used in the next step
This abstract class should be extended by all concrete generic upcaster classes provided by axon
Step #2
change the construct of Upcaster chain to get upcasters which extend from abstract class from step #1 and create a mapping from payloadType(retrieved by the new method introduced in step #1) to list of upcasters for that type.
Step #3
change the upcast method of Upcaster chain to first get supported upcastors for the event representation. Then call upcast methods of already filtered upcasters.

This way part of the filtering of upcasters will be owned by axon (payload type match part), as a result, to do the first filtering we’ll spend something like O(log(n+m)), but the amount of upcast calles will be reduced to O(n) we’ll spend.

Thanks in advance for reading through,
Best regards,
Hayk

2

Hi Hayk,

Thanks for voicing your concern regarding the Upcaster approach.
Any input on any topic regarding the implementation approaches helps us out tremendously.

However, I think there are a couple of caveats which you might not have taken into account, which I want to lay before you:

  1. The Payload Type alone is not enough to deduce whether the Upcaster is applicable to a given event. The Revision Number should always be taken in to account as well.
  2. When upcasting an Event, you are not inclined to maintain the Payload Type during the upcasting process. Actually, it is quite a regular operation to adjust the payload type of an event from an old format to a new format. Doing so, will make it tougher to filter a set of Upcasters before hand.
  3. There are Upcaster interfaces present which will give you the handle to merge several events in to one and vice versa. Again, this will have great impact on how a filter on the upcasters to use for a given event could work.
    In the past, I’ve taken a pragmatic, and conceptually a no-no, approach to resolving the problem of having a large (large being about 200, on an Axon 2.x project) set of upcasters.
    We created a single use tool which read our entire event stream, upcasted all the events and stored them again.
    Thus, we actually updated our history, so that we could remove our entire suite of upcasters.

As pointed out, conceptually this isn’t to nice. Events are history and you shouldn’t be changing them, ever.
However in practice, things like this fall apart in favor of performance and maintainability.
I feel it would be nice if Axon Framework or Axon Server provides some instrumentation to service such an operation.

Concluding, do my points chance something in your assumption on improving the performance?

Cheers,

Steven van Beelen

Axon Framework Lead Developer

AxonIQ

3

Hi Hayk,

I’d like to add one thing: the canUpcast method is supposed to be cheap. If you need to do expensive operations, I recommend doing the quick checks in the canUpcast method, and the more expensive ones in the doUpcast. It is fine if the doUpcast method doesn’t actually upcast any results. In other words, canUpcast may have some false positives.

Cheers,