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

Increasing batch size in ingest mode #3822

Closed
wants to merge 1 commit into from

Conversation

nacrooks
Copy link
Contributor

@nacrooks nacrooks commented Jul 31, 2020

As suggested by @frankmcsherry, this PR reduces the frequency at which we downgrade capabilities (thus creating larger batch sizes) when we detect that we are trailing behind a source (when the number of known messages in the topic is greater than the number of processed messages by X).

I wanted to get this PR up before leaving but I have not verified that it actually improves performance.


This change is Reviewable

@frankmcsherry frankmcsherry added A-integration Area: external integrations T-performance Theme: performance improvements labels Aug 5, 2020
@rjnn
Copy link
Contributor

rjnn commented Aug 14, 2020

Assigned to @umanwizard to merge and verify it helps!

@frankmcsherry
Copy link
Contributor

This dovetails with a related ask for consistency: that on start-up, one be able to force all timestamps of existing records to at most now() (and possibly exactly now()), for the purposes of ensuring that users are able to maintain consistency across restarts. This is one candidate remedy, so it's not dictum that it should happen, but we would like to find a mechanism for users to ensure that previously seen records are not timestamped in their future, and this may be such a mechanism.

@frankmcsherry
Copy link
Contributor

This PR (or something like it) probably would have helped prevent some of the issues encountered in #5013.

@benesch
Copy link
Member

benesch commented Feb 9, 2021

This unfortunately went a bit stale and will need to be rewritten for changes that have occurred to the timestamper. I think this is still desired, so I filed #5690 to track.

@benesch benesch closed this Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-integration Area: external integrations T-performance Theme: performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants