Skip to content

Conversation

biochimia
Copy link
Contributor

In the face of backpressure, spillover records that couldn't be offered to the queue would be registered, and wouldn't always be cleared afterwards, leading to duplicate records being offered. This is addressed by ensuring spillover records are always reset at the end of the poll loop.

In an earlier iteration of #1369 spillover records were reset early in the poll loop. At the end of the loop only remaining spillover records needed to be recorded. In the approach that ended up being submitted spillover records are reset only once at the end of the poll loop, but logic from the earlier iteration was inadvertently left behind that skipped this reset operation in the case there were no remaining spillover records to register.

In the face of backpressure, spillover records that couldn't be offered
to the queue would be registered, and wouldn't alwayd be cleared
afterwards, leading to duplicate records being offered. This is
addressed by ensuring spillover records are always reset at the end of
the poll loop.

In an earlier iteration of fd4s#1369 spillover records were reset early in
the poll loop. At the end of the loop only remaining spillover records
needed to be recorded. In the approach that ended up being submitted
spillover records are reset only once at the end of the poll loop, but
logic from the earlier iteration was inadvertently left behind that
skipped this reset operation in the case there were no remaining
spillover records to register.
@aartigao aartigao merged commit cf7e0c9 into fd4s:series/3.x Mar 28, 2025
8 checks passed
@aartigao
Copy link
Contributor

RC2 published!

@biochimia
Copy link
Contributor Author

Thank you, @aartigao.

This time around, all is looking good on our side.

We can see a drop in the time to complete commitAsync operations from @rodrigo-molina's #1375. The effect mainly comes from waiting one less poll cycle before the commit call is actually made in the consumer.

Screenshot 2025-03-28 at 10 37 52

For my changes, I will claim that test times went down in CI 😅 , but it's harder to find measurable effects in our metrics.

@aartigao
Copy link
Contributor

Glad to hear that 🥇

I'll keep this RC2 around for the weekend and make the proper release next week (without CE 3.6.0 changes).

Thanks for the hard work 💪🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants