Skip to content

Refactor SnapshotInfo dataflow in finalization #124336

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

DaveCTurner
Copy link
Contributor

There's no need to have a SnapshotInfo consumer to run at the end of
finalization, we only pass it the value we already calculated earlier.
This replaces it with a bare Runnable instead.

There's no need to have a `SnapshotInfo` consumer to run at the end of
finalization, we only pass it the value we already calculated earlier.
This replaces it with a bare `Runnable` instead.
@DaveCTurner DaveCTurner added :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >refactoring auto-backport Automatically create backport pull requests when merged v8.18.1 v8.19.0 v9.0.1 v9.1.0 labels Mar 7, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Mar 7, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@DaveCTurner
Copy link
Contributor Author

Small change arising from work towards a fix for #108907 - snapshot finalization is a bit of a tangle of code split across SnapshotsService and BlobStoreRepository today and it's getting in the way of a good fix for the bug. Not sure exactly how this is going to work out yet but it's going to be nicer than it is today for sure.

Copy link
Contributor

@DiannaHohensee DiannaHohensee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DiannaHohensee
Copy link
Contributor

I might change the commit message to say "Refactor" instead of "Clean up"

@DaveCTurner DaveCTurner changed the title Clean up SnapshotInfo dataflow in finalization Refactor SnapshotInfo dataflow in finalization Mar 7, 2025
@DaveCTurner DaveCTurner added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Mar 7, 2025
@elasticsearchmachine elasticsearchmachine merged commit 88eeb8a into elastic:main Mar 7, 2025
17 checks passed
@DaveCTurner DaveCTurner deleted the 2025/03/07/finalize-snapshot-info-dataflow branch March 7, 2025 22:17
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Mar 7, 2025
There's no need to have a `SnapshotInfo` consumer to run at the end of
finalization, we only pass it the value we already calculated earlier.
This replaces it with a bare `Runnable` instead.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

elasticsearchmachine pushed a commit that referenced this pull request Mar 7, 2025
There's no need to have a `SnapshotInfo` consumer to run at the end of
finalization, we only pass it the value we already calculated earlier.
This replaces it with a bare `Runnable` instead.
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Mar 11, 2025
There's no need to have a `SnapshotInfo` consumer to run at the end of
finalization, we only pass it the value we already calculated earlier.
This replaces it with a bare `Runnable` instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >refactoring Team:Distributed Coordination Meta label for Distributed Coordination team v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants