-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(rollups): rollup a batch if more than 2 seconds elapsed since last batch #6118
Conversation
…and atomics. Benchmark shows no degradation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @manishrjain and @martinmr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the benchmark result? Can you paste the results here?
Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @manishrjain)
So, (1) and (2) are comparable at ~65 ns/op However, I have not done the benchmark directly on master as it is today. Let me do that next. |
So, on master, it is ~19 ns/op but it suffers from the issue that rollups dont happen as often. FWIW, I live-loaded 21 million data set with master and my branch. It took 13m30sec and 13m49sec respectively. So, not much change there. cc @manishrjain |
The |
Based on the above comments, I think we can still use this fix. |
…vel var and atomics. Benchmark shows no degradation." This reverts commit 905f703.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain and @parasssh)
posting/mvcc.go, line 120 at r2 (raw file):
dorollup
nit: doRollup
fix(rollup): rollup a batch every 2 seconds or 64 keys
This change is