Skip to content

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

Merged
merged 5 commits into from
Dec 11, 2024

Conversation

prdoyle
Copy link
Contributor

@prdoyle prdoyle commented Dec 10, 2024

These tasks don't support batching, so submit each in its own queue.

@prdoyle prdoyle added >non-issue :Core/Infra/Core Core issues without another label labels Dec 10, 2024
@prdoyle prdoyle requested a review from DaveCTurner December 10, 2024 15:09
@prdoyle prdoyle self-assigned this Dec 10, 2024
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.0.0 labels Dec 10, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

"reserved cluster state [" + namespace + "]",
new ReservedStateUpdateTask(
namespace,
reservedStateChunk,
versionCheck,
handlers,
orderedHandlers,
ReservedClusterStateService.this::updateErrorState,
this::updateErrorState,
Copy link
Contributor Author

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.

Copy link
Contributor

@DaveCTurner DaveCTurner left a 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.

@prdoyle prdoyle force-pushed the file-watch-single-task-queues branch from de382f8 to 4b1dacb Compare December 10, 2024 20:45
@prdoyle prdoyle force-pushed the file-watch-single-task-queues branch from 4b1dacb to aa9dfd6 Compare December 10, 2024 20:47
@prdoyle
Copy link
Contributor Author

prdoyle commented Dec 10, 2024

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:

  1. A refactoring to expose a method I want to call from the tests
  2. The tests. They fail without the fix.
  3. The fix. These make the tests pass.

If you want to review commit-by-commit, it should be readable now.

Copy link
Contributor

@DaveCTurner DaveCTurner left a 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants