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

Snapshot _status API to return correct status for partial snapshots #12812

Merged
merged 17 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
274af5c
Snapshot _status API to return correct status for partial snapshots
aggarwalShivani Mar 21, 2024
455cfb3
Updated CHANGELOG.md
aggarwalShivani Mar 28, 2024
001564b
Updated test case
aggarwalShivani Mar 29, 2024
30dbde4
Setting snapshot status to SUCCESS for older versions for bwc
aggarwalShivani Apr 1, 2024
d5e1380
Setting snapshot status to SUCCESS for older versions for bwc
aggarwalShivani Apr 1, 2024
271fc8c
Merge branch 'opensearch-project:main' into snapshot-status-fix
aggarwalShivani Apr 3, 2024
21ad55c
Merge branch 'opensearch-project:main' into snapshot-status-fix
aggarwalShivani Apr 5, 2024
0a21696
Moved BWC change to SnapshotsInProgress.java for partial snapshots
aggarwalShivani Apr 5, 2024
2fee046
Merge branch 'opensearch-project:main' into snapshot-status-fix
aggarwalShivani Apr 5, 2024
7d174cc
Merge branch 'opensearch-project:main' into snapshot-status-fix
aggarwalShivani Apr 9, 2024
3424b6a
Fix for flaky test testSnapshotStatusOnPartialSnapshot
aggarwalShivani Apr 9, 2024
db064b3
Merge branch 'main' into snapshot-status-fix
aggarwalShivani Apr 12, 2024
ab01a69
Updated the testcases to reuse existing getSnapshotStatus() method
aggarwalShivani Apr 12, 2024
d1e6656
Fixed formatting issues detected in spotlessJavaCheck
aggarwalShivani Apr 12, 2024
054e53e
Merge branch 'opensearch-project:main' into snapshot-status-fix
aggarwalShivani Apr 15, 2024
c0b1ae1
Moved the entry to CHANGELOG.md
aggarwalShivani Apr 15, 2024
9066b85
Merge branch 'opensearch-project:main' into snapshot-status-fix
aggarwalShivani Apr 16, 2024
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 @@ -98,6 +98,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Bug] Check phase name before SearchRequestOperationsListener onPhaseStart ([#12035](https://github.com/opensearch-project/OpenSearch/pull/12035))
- Fix Span operation names generated from RestActions ([#12005](https://github.com/opensearch-project/OpenSearch/pull/12005))
- Fix error in RemoteSegmentStoreDirectory when debug logging is enabled ([#12328](https://github.com/opensearch-project/OpenSearch/pull/12328))
- Fix snapshot _status API to return correct status for partial snapshots ([#12812](https://github.com/opensearch-project/OpenSearch/pull/12812))

### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ public void testRestoreIndexWithMissingShards() throws Exception {
List<SnapshotStatus> snapshotStatuses = snapshotsStatusResponse.getSnapshots();
assertEquals(snapshotStatuses.size(), 1);
logger.trace("current snapshot status [{}]", snapshotStatuses.get(0));
assertTrue(snapshotStatuses.get(0).getState().completed());
assertThat(getSnapshot("test-repo", "test-snap-2").state(), equalTo(SnapshotState.PARTIAL));
}, 1, TimeUnit.MINUTES);
SnapshotsStatusResponse snapshotsStatusResponse = clusterAdmin().prepareSnapshotStatus("test-repo")
.setSnapshots("test-snap-2")
Expand All @@ -589,7 +589,6 @@ public void testRestoreIndexWithMissingShards() throws Exception {
// After it was marked as completed in the cluster state - we need to check if it's completed on the file system as well
assertBusy(() -> {
SnapshotInfo snapshotInfo = getSnapshot("test-repo", "test-snap-2");
assertTrue(snapshotInfo.state().completed());
assertEquals(SnapshotState.PARTIAL, snapshotInfo.state());
}, 1, TimeUnit.MINUTES);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,25 @@ public void testSnapshotStatusOnFailedSnapshot() throws Exception {
assertEquals(SnapshotsInProgress.State.FAILED, snapshotsStatusResponse.getSnapshots().get(0).getState());
}

public void testSnapshotStatusOnPartialSnapshot() throws Exception {
reta marked this conversation as resolved.
Show resolved Hide resolved
final String dataNode = internalCluster().startDataOnlyNode();
final String repoName = "test-repo";
final String snapshotName = "test-snap";
final String indexName = "test-idx";
createRepository(repoName, "fs");
// create an index with a single shard on the data node, that will be stopped
createIndex(indexName, singleShardOneNode(dataNode));
index(indexName, "_doc", "some_doc_id", "foo", "bar");
logger.info("--> stopping data node before creating snapshot");
stopNode(dataNode);
startFullSnapshot(repoName, snapshotName, true).get();
final List<SnapshotStatus> snapshotStatus = clusterAdmin().snapshotsStatus(
new SnapshotsStatusRequest(repoName, new String[] { snapshotName })
).actionGet().getSnapshots();
aggarwalShivani marked this conversation as resolved.
Show resolved Hide resolved
assertThat(snapshotStatus.size(), equalTo(1));
assertEquals(SnapshotsInProgress.State.PARTIAL, snapshotStatus.get(0).getState());
}

public void testStatusAPICallInProgressShallowSnapshot() throws Exception {
internalCluster().startClusterManagerOnlyNode();
internalCluster().startDataOnlyNode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,11 @@
state = SnapshotsInProgress.State.FAILED;
break;
case SUCCESS:
case PARTIAL:
// Translating both PARTIAL and SUCCESS to SUCCESS for now
// TODO: add the differentiation on the metadata level in the next major release
state = SnapshotsInProgress.State.SUCCESS;
break;
case PARTIAL:
state = SnapshotsInProgress.State.PARTIAL;
break;

Check warning on line 363 in server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java#L362-L363

Added lines #L362 - L363 were not covered by tests
default:
throw new IllegalArgumentException("Unknown snapshot state " + snapshotInfo.state());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,12 @@
snapshot.writeTo(out);
out.writeBoolean(includeGlobalState);
out.writeBoolean(partial);
out.writeByte(state.value());
if ((out.getVersion().before(Version.V_3_0_0)) && state == State.PARTIAL) {
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
// Setting to SUCCESS for partial snapshots in older versions to maintain backward compatibility
out.writeByte(State.SUCCESS.value());

Check warning on line 752 in server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java#L752

Added line #L752 was not covered by tests
} else {
out.writeByte(state.value());
}
out.writeList(indices);
out.writeLong(startTime);
out.writeMap(shards, (o, v) -> v.writeTo(o), (o, v) -> v.writeTo(o));
Expand Down Expand Up @@ -937,7 +942,8 @@
STARTED((byte) 1, false),
SUCCESS((byte) 2, true),
FAILED((byte) 3, true),
ABORTED((byte) 4, false);
ABORTED((byte) 4, false),
PARTIAL((byte) 5, false);
reta marked this conversation as resolved.
Show resolved Hide resolved

private final byte value;

Expand Down Expand Up @@ -968,6 +974,8 @@
return FAILED;
case 4:
return ABORTED;
case 5:
return PARTIAL;

Check warning on line 978 in server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java#L978

Added line #L978 was not covered by tests
default:
throw new IllegalArgumentException("No snapshot state for value [" + value + "]");
}
Expand Down
Loading