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.

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

fix snapshot restore and recovery endpoint #611

Merged
merged 3 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Fixed mapping and analysis types ([#600](https://github.com/opensearch-project/opensearch-api-specification/pull/600))
- Fixed `RestStatus` responses in `DELETE /_plugins/_notifications/configs/{config_id}` ([#594](https://github.com/opensearch-project/opensearch-api-specification/pull/594))
- Fixed `GET /_snapshot_/{repository}/{snapshot}` ([#608](https://github.com/opensearch-project/opensearch-api-specification/pull/608))
- Fixed `POST /_snapshot/{repository}/{snapshot}/_restore` when `wait_for_completion` is `false` and `GET /{index}/_recovery` ([#611](https://github.com/opensearch-project/opensearch-api-specification/pull/611))

### Security

Expand Down
7 changes: 4 additions & 3 deletions spec/namespaces/snapshot.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ components:
type: object
properties:
accepted:
description: Equals `true` if the snapshot was accepted. Present when the request had `wait_for_completion` set to `false`
description: Returns `true` if the snapshot was accepted. Present when the request had `wait_for_completion` set to `false`.
type: boolean
snapshot:
$ref: '../schemas/snapshot._common.yaml#/components/schemas/SnapshotInfo'
Expand Down Expand Up @@ -427,10 +427,11 @@ components:
schema:
type: object
properties:
accepted:
description: Returns `true` if the restore was accepted. Present when the request had `wait_for_completion` set to `false`.
type: boolean
snapshot:
$ref: '../schemas/snapshot.restore.yaml#/components/schemas/SnapshotRestore'
required:
- snapshot
snapshot.status@200:
content:
application/json:
Expand Down
8 changes: 8 additions & 0 deletions spec/schemas/indices.recovery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,14 @@ components:
$ref: '_common.yaml#/components/schemas/Uuid'
index:
$ref: '_common.yaml#/components/schemas/IndexName'
isSearchableSnapshot:
dblock marked this conversation as resolved.
Show resolved Hide resolved
type: boolean
remoteStoreIndexShallowCopy:
type: boolean
sourceRemoteStoreRepository:
type: ['null', string]
sourceRemoteTranslogRepository:
type: ['null', string]
RecoveryStartStatus:
type: object
properties:
Expand Down
101 changes: 101 additions & 0 deletions tests/snapshot/snapshot/restore.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
$schema: ../../../json_schemas/test_story.schema.yaml

description: Test _snapshot/{repository}/{snapshot}/_restore endpoints.
epilogues:
- path: /_snapshot/{repository}/{snapshot}
method: DELETE
status: [200, 404]
parameters:
repository: my-fs-repository
snapshot: my-test-snapshot
- path: /_snapshot/{repository}
method: DELETE
status: [200, 404]
parameters:
repository: my-fs-repository
- path: /movies
method: DELETE
status: [200, 404]
- path: /books
method: DELETE
status: [200, 404]
prologues:
- path: /_snapshot/{repository}
method: PUT
parameters:
repository: my-fs-repository
request:
payload:
type: fs
settings:
location: /tmp/opensearch/repo
- path: /movies
method: PUT
- path: /books
method: PUT
- path: /_snapshot/{repository}/{snapshot}
method: PUT
parameters:
repository: my-fs-repository
snapshot: my-test-snapshot
wait_for_completion: true
request:
payload:
indices:
- books
- movies
ignore_unavailable: true
include_global_state: false
partial: true
- path: /movies
method: DELETE
status: [200, 404]
- path: /books
method: DELETE
status: [200, 404]
chapters:
- synopsis: Restore snapshot with wait_for_completion true.
path: /_snapshot/{repository}/{snapshot}/_restore
method: POST
parameters:
repository: my-fs-repository
snapshot: my-test-snapshot
wait_for_completion: true
request:
payload:
indices: movies
response:
status: 200
payload:
snapshot:
snapshot: my-test-snapshot
- synopsis: Restore snapshot with wait_for_completion false.
path: /_snapshot/{repository}/{snapshot}/_restore
method: POST
parameters:
repository: my-fs-repository
snapshot: my-test-snapshot
wait_for_completion: false
request:
payload:
indices: books
response:
status: 200
payload:
accepted: true
- synopsis: Wait finish async restore.
Copy link
Member

@dblock dblock Oct 16, 2024

Choose a reason for hiding this comment

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

This should live in its own file, so restore.yaml and recovery.yaml where the snapshot would be a prologue. This way you don't need multiple-paths-detected warning suppression and the sample can be used in a separate documentation page for _recovery.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The following error occurs when a snapshot is deleted in the epilogue immediately after an asynchronous restore.

ERROR   EPILOGUES
        ERROR   DELETE /_snapshot/{repository}/{snapshot} ([my-fs-repository:my-test-snapshot/KIVGLRXyR168qOKeAS1sIw] cannot delete snapshot during a restore in progress in [RestoreInProgress[{9gz8AVvDRPm9e2ZU8Gdw9A}{my-fs-repository:my-test-snapshot/KIVGLRXyR168qOKeAS1sIw}]])

Therefore, I placed the test for the _recovery endpoint in the chapters to wait for the restore to complete(response cannot be specified in epilogue).
I would be grateful if you could let me know if there is a better method.

Copy link
Member

Choose a reason for hiding this comment

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

We can leave it like this for now, no big deal.

Copy link
Member

Choose a reason for hiding this comment

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

Could you please open an issue describing this? I think epilogues should be able to deal with the response, too.

path: /{index}/_recovery
warnings:
multiple-paths-detected: false
method: GET
parameters:
index: books
response:
status: 200
payload:
books:
shards:
- stage: DONE
type: SNAPSHOT
retry:
count: 3
Loading