Test get-snapshots API with missing details #111903
Merged
Conversation
This file contains 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
Extends the test added in elastic#111786 to check that the API still works correctly even in the BwC case that the details needed are not in the `RepositoryData` and must be read from the individual `SnapshotInfo` blobs.
DaveCTurner
added
>test
elasticsearchmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java
Outdated
Show resolved
Hide resolved
| @@ -1000,4 +1020,97 @@ public void testAllFeatures() { | |||
|
|
|||
| assertEquals(0, remaining); | |||
| } | |||
|
|
|||
| private static Set<SnapshotId> removeRandomSnapshotDetails(List<String> repositories) { | |||
There was a problem hiding this comment.
pls comment method with explanation about randomly exercising backwards compatibility, and what backwards compatibility specifically.
| return Set.of(); | ||
| } | ||
| final var masterRepositoriesService = internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class); | ||
| return safeAwait(l0 -> { |
There was a problem hiding this comment.
I have no idea what l0 is abbreviating and thought it was a ten at first -- how about safeAwaitListener?
There was a problem hiding this comment.
:) fair enough, fixed now.
| final var masterRepositoriesService = internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class); | ||
| return safeAwait(l0 -> { | ||
| final Set<SnapshotId> snapshotsWithoutDetails = ConcurrentCollections.newConcurrentSet(); | ||
| try (var listeners = new RefCountingListener(l0.map(v -> Set.copyOf(snapshotsWithoutDetails)))) { |
There was a problem hiding this comment.
Suggested change
| try (var listeners = new RefCountingListener(l0.map(v -> Set.copyOf(snapshotsWithoutDetails)))) { | |
| // Once we finish going through the repositories, complete the safeAwait listener with a copy of snapshotsWithoutDetails. | |
| try (var listeners = new RefCountingListener(l0.map(v -> Set.copyOf(snapshotsWithoutDetails)))) { |
There was a problem hiding this comment.
This took me 10+ minutes to decipher all the utilities and how they work together, I'd really appreciate a comment to boost understanding/retention for the next poor sucker (possibly me again) to come along.
server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java
Outdated
Show resolved
Hide resolved
server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java
Outdated
Show resolved
Hide resolved
| @@ -1000,4 +1020,97 @@ public void testAllFeatures() { | |||
|
|
|||
| assertEquals(0, remaining); | |||
| } | |||
|
|
|||
| private static Set<SnapshotId> removeRandomSnapshotDetails(List<String> repositories) { | |||
There was a problem hiding this comment.
Might better to name it removeSnapshotDetailsFromRandomSnapshots, or something. Right now it suggests to me that random details will be removed -- sometimes this field, sometimes that other field. But a comment will clarify 🤷♀️
|
Thanks @DiannaHohensee - this was extracted out of a larger change so yes might not make quite so much sense on its own. Hopefully the changes I just pushed improve things, please take another look. |
DaveCTurner
deleted the
2024/08/14/GetSnapshotsIT-removeRandomSnapshotDetails
branch
weizijun
added a commit
to weizijun/elasticsearch
that referenced
this pull request
Aug 19, 2024
* upstream/main: Fail `indexDocs()` on rejection (elastic#111962) Move repo analyzer to its own package (elastic#111963) Add generated evaluators for DateNanos conversion functions (elastic#111961) Clean the last traces from global retention in templates (elastic#111669) Fix known issue docs for elastic#111866 (elastic#111956) x-pack/plugin/otel: introduce x-pack-otel plugin (elastic#111091) Improve reaction to blob store corruptions (elastic#111954) Introduce `StreamingXContentResponse` (elastic#111933) Revert "Add 8.15.0 known issue for memory locking in Windows (elastic#111949)" Test get-snapshots API with missing details (elastic#111903) Add 8.15.0 known issue for memory locking in Windows (elastic#111949) # Conflicts: # server/src/main/java/org/elasticsearch/TransportVersions.java
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Sep 4, 2024
Extends the test added in elastic#111786 to check that the API still works correctly even in the BwC case that the details needed are not in the `RepositoryData` and must be read from the individual `SnapshotInfo` blobs.
davidkyle
pushed a commit
to davidkyle/elasticsearch
that referenced
this pull request
Sep 5, 2024
Extends the test added in elastic#111786 to check that the API still works correctly even in the BwC case that the details needed are not in the `RepositoryData` and must be read from the individual `SnapshotInfo` blobs.
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.
Extends the test added in #111786 to check that the API still works
correctly even in the BwC case that the details needed are not in the
RepositoryDataand must be read from the individualSnapshotInfoblobs.