-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Reindex Throttle Checkpointing #46763
Reindex Throttle Checkpointing #46763
Conversation
Rather than write every time, we now only write at certain intervals. Revamped the single-writer and finish handling around this to avoid any waiting. Relates elastic#42612
Pinging @elastic/es-distributed |
ci/2 failure reported here: #46907 so |
…_throttle_checkpointing
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.
LGTM. I think we should also consider at some point just having a repeated scheduled checkpoint update at a certain interval. But this seems reasonable.
Only send back task-id to client after all subtasks have started to stay compatible with tests requiring this. Relates elastic#46763 (test failure) and elastic#42612
this.value = newValue; | ||
if (scheduledWrite == null) { | ||
// schedule is non-blocking | ||
scheduledWrite = threadPool.schedule(this::onScheduleTimeout, getDelay(now), ThreadPool.Names.SAME); |
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.
schedule
loses the thread context. Given that we are afterwards writing to an index (possibly running with security enabled), I wonder if that thread context is relevant.
ci/1 failure to run |
…_throttle_checkpointing
Rather than write checkpoints every time, we now only write at certain intervals.
Revamped the single-writer and finish handling around this to avoid any
waiting/blocking.
Relates #42612