Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Select snapshots using RepositoryData#snapshotsDetails where possible #95345

Open
DaveCTurner opened this issue Apr 18, 2023 · 3 comments
Open

Select snapshots using RepositoryData#snapshotsDetails where possible #95345

DaveCTurner opened this issue Apr 18, 2023 · 3 comments
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination.

Comments

@DaveCTurner
Copy link
Contributor

DaveCTurner commented Apr 18, 2023 •
edited
Loading

The get-snapshots API accepts a size parameter to limit the number of snapshots returned, but today it still retrieves all potentially-matching SnapshotInfo blobs before applying the limit. In repositories containing hundreds/thousands of snapshots this can take many minutes, and consume a large amount of heap. However we have enough information in the RepositoryData to apply the limit before retrieving any SnapshotInfo blobs if sorting by name, start time, or duration. We should do that to save time and memory.

Workaround

Note that the from_sort_value parameter does restrict the SnapshotInfo blobs retrieved if sorting by name, start time, or duration, so you can say sort=start_time&order=asc&from_sort_value=XXXXXX to restrict things to just the snapshots which started after time XXXXXX (in milliseconds since the Unix epoch). If you pick a time close enough to the present then the API still works.
@DaveCTurner DaveCTurner added >enhancement :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Apr 18, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Apr 18, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@alisonelizabeth
Copy link
Contributor

This issue also appears to be impacting the performance of the Snapshot & Restore UI. See: elastic/kibana#122522

@ElenaStoeva ElenaStoeva mentioned this issue Oct 30, 2023
Closed
@DaveCTurner DaveCTurner mentioned this issue Jan 22, 2024
Open
@yuliacech yuliacech mentioned this issue Feb 5, 2024
Open
@DaveCTurner DaveCTurner mentioned this issue Feb 19, 2024
Merged
@DaveCTurner
Copy link
Contributor Author

DaveCTurner commented Mar 22, 2024 •
edited
Loading

Turns out it's pretty tricky to make this change neatly today, the implementation has accumulated a good deal of cruft over the years which gets in the way quite badly. Some recent cleanups in TransportGetSnapshotsAction that are working towards enabling this change:

@DaveCTurner DaveCTurner mentioned this issue Apr 8, 2024
Merged
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Aug 11, 2024
@DaveCTurner
Add GetSnapshotsIT#testAllFeatures
7b54f9f 
The features of get-snapshots API are all tested in isolation or small
combinations, but there's no one test which pins down exactly how they
all interact. This commit adds such a test, to verify that any future
optimization work preserves the observable behaviour.

Relates elastic#95345
Relates elastic#104607
@DaveCTurner DaveCTurner mentioned this issue Aug 11, 2024
Merged
elasticsearchmachine pushed a commit that referenced this issue Aug 13, 2024
@DaveCTurner
Add GetSnapshotsIT#testAllFeatures (#111786)
d2bd374 
The features of get-snapshots API are all tested in isolation or small
combinations, but there's no one test which pins down exactly how they
all interact. This commit adds such a test, to verify that any future
optimization work preserves the observable behaviour.

Relates #95345 Relates #104607
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Sep 4, 2024
@DaveCTurner @cbuescher
Add GetSnapshotsIT#testAllFeatures (elastic#111786)
e1ea785 
The features of get-snapshots API are all tested in isolation or small
combinations, but there's no one test which pins down exactly how they
all interact. This commit adds such a test, to verify that any future
optimization work preserves the observable behaviour.

Relates elastic#95345 Relates elastic#104607
davidkyle pushed a commit to davidkyle/elasticsearch that referenced this issue Sep 5, 2024
@DaveCTurner @davidkyle
Add GetSnapshotsIT#testAllFeatures (elastic#111786)
2125deb 
The features of get-snapshots API are all tested in isolation or small
combinations, but there's no one test which pins down exactly how they
all interact. This commit adds such a test, to verify that any future
optimization work preserves the observable behaviour.

Relates elastic#95345 Relates elastic#104607
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@DaveCTurner @alisonelizabeth @elasticsearchmachine