Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AsyncDisruptorAppender not releasing events "fast enough" when receiving batches of events #568

Closed
brenuart opened this issue Jul 9, 2021 · 0 comments · Fixed by #571
Closed
Milestone

Comments

@brenuart
Copy link
Collaborator

brenuart commented Jul 9, 2021

Consider the following scenario:

  1. A RingBuffer with a size of 4 elements
  2. Append 4 events in a raw to fill the buffer - these 4 events will form a batch
  3. At this point, the buffer is full and new events will be dropped
  4. Unlock the EventHandler and let it process the first of the 4 events out of the queue.
  5. We now expect the buffer to contain only 3 events and there should be enough room to accept another. However, any attempt to append a new event will result in a drop (as indicated by the warn status message logged by the appender).

This behaviour due to the Disruptor BatchingEventProcessor updating the sequence (and releasing space in the buffer) only after the entire batch is processed. See https://github.com/LMAX-Exchange/disruptor/blob/3.4.4/src/main/java/com/lmax/disruptor/BatchEventProcessor.java#L165-L172, especially the last line...

This means that space is reserved in the RingBuffer longer that needed. The retained space is proportional to the batch size, and therefore the logging rate of the application.

The Disruptor library comes with the SequenceReportingEventHandler interface for this exact purpose. It allows event handlers to notify the batch processor when it has finished to process an event by updating the sequence accordingly.

Another interesting comment found around the net:

The Disruptor will only update the processed sequence for that event handler until the batch is complete (the default is a batch that is the size of all of the available messages). This reduces the number of updates that it needs to make to the sequence variable used by the publisher to determine if space is available. If you need to make space available earlier than the default then you can do that using a SequenceReportingEventHandler.

--> see how updating the sequence after each event effectively affects performance. If needed, only update the sequence after x events.

brenuart added a commit that referenced this issue Jul 14, 2021
Modify the EventClearingEventHandler to notify the BatchEventProcessor that the sequence has progressed.
Without this callback the sequence would not be progressed until the batch has completely finished.

See #568
brenuart added a commit that referenced this issue Jul 14, 2021
Modify the EventClearingEventHandler to notify the BatchEventProcessor that the sequence has progressed.
Without this callback the sequence would not be progressed until the batch has completely finished.

See #568
@philsttr philsttr linked a pull request Jul 15, 2021 that will close this issue
philsttr pushed a commit that referenced this issue Jul 15, 2021
* Clear and release LogEvent before end of batch

Modify the EventClearingEventHandler to notify the BatchEventProcessor that the sequence has progressed.
Without this callback the sequence would not be progressed until the batch has completely finished.

Fixes #568
@philsttr philsttr added this to the 7.0 milestone Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants