Create a utility class for static SnapshotsService methods #127419
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DiannaHohensee
added
>non-issue
:Distributed Coordination/Snapshot/Restore
DiannaHohensee
force-pushed
the
2025/04/25/snapshot-service-refactor-static
branch
from
d0a8866 to
1268eee
Compare
This moves the static methods out of the SnapshotsService to make the stateful code more visible. This is one step towards refactoring the SnapshotsService.
DiannaHohensee
force-pushed
the
2025/04/25/snapshot-service-refactor-static
branch
from
1268eee to
76b71c6
Compare
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
DiannaHohensee
commented
Apr 25, 2025
| DiscoveryNodes nodes, | ||
| Predicate<String> nodeIdRemovalPredicate, | ||
| Map<RepositoryShardId, SnapshotsInProgress.ShardSnapshotStatus> knownFailures, | ||
| Logger logger |
There was a problem hiding this comment.
This, and a couple other methods I've marked, are the only slight changes I made to the methods: passing in the SnapshotService's logger.
The rest of the changes in this file are purely IDE move operations out of the SnapshotsService file into this one, with a little method ordering done by me.
| } | ||
| } | ||
|
|
||
| static <T> void failListenersIgnoringException(@Nullable List<ActionListener<T>> listeners, Exception failure, Logger logger) { |
There was a problem hiding this comment.
This is the other change to add the logger from SnapshotsService, along with completeListenersIgnoringException() below.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves the static methods out of the SnapshotsService to
make the stateful code more visible. This is one step towards
refactoring the SnapshotsService.
I'd like to eventually get the SnapshotsService into a state where it manages MasterService task queues, and submitting work to them, but the executor logic moved out to other files. Classes like SnapshotTaskExecutor (and related code, like SnapshotShardsUpdateContext) and UpdateNodeIdsForRemovalTask would get moved out.
Another future refactor target would be the logic stuffed into SnapshotsService#cloneSnapshot(): there's an anonymous class extension here and here. I'm guessing that can be made easier to follow.
Also can easily boot out UpdateSnapshotStatusAction into its own Transport* file just to get it out of there. Like TransportCloneSnapshotAction already is.
Let me know what you think of ^ @DaveCTurner