-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Use single-task queues in ReservedClusterStateService #118351
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
Conversation
Pinging @elastic/es-core-infra (Team:Core/Infra) |
"reserved cluster state [" + namespace + "]", | ||
new ReservedStateUpdateTask( | ||
namespace, | ||
reservedStateChunk, | ||
versionCheck, | ||
handlers, | ||
orderedHandlers, | ||
ReservedClusterStateService.this::updateErrorState, | ||
this::updateErrorState, |
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.
Huh, IntelliJ did this automatically.
server/src/main/java/org/elasticsearch/reservedstate/service/ReservedClusterStateService.java
Outdated
Show resolved
Hide resolved
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.
Yep that should work. Ideally we'd have some test to show that no batching happens tho.
de382f8
to
4b1dacb
Compare
4b1dacb
to
aa9dfd6
Compare
Sorry for the force-push. I had a lot of messing around and wanted to leave the result in a reviewable state. This branch is now based on three main commits:
If you want to review commit-by-commit, it should be readable now. |
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 (although bleurgh more mocks)
These tasks don't support batching, so submit each in its own queue.