From b899a27acc5e3092671a96f0035b03f9940fdfb3 Mon Sep 17 00:00:00 2001 From: aggarwalShivani <39588384+aggarwalShivani@users.noreply.github.com> Date: Wed, 17 Apr 2024 11:47:30 +0530 Subject: [PATCH] Snapshot _status API to return correct status for partial snapshots (#12812) * Snapshot _status API to return correct status for partial snapshots Signed-off-by: aggarwalShivani * Updated CHANGELOG.md Signed-off-by: aggarwalShivani * Updated test case Signed-off-by: aggarwalShivani * Setting snapshot status to SUCCESS for older versions for bwc Signed-off-by: aggarwalShivani * Setting snapshot status to SUCCESS for older versions for bwc Signed-off-by: aggarwalShivani * Moved BWC change to SnapshotsInProgress.java for partial snapshots Signed-off-by: aggarwalShivani * Fix for flaky test testSnapshotStatusOnPartialSnapshot Signed-off-by: aggarwalShivani * Updated the testcases to reuse existing getSnapshotStatus() method Signed-off-by: aggarwalShivani * Fixed formatting issues detected in spotlessJavaCheck Signed-off-by: aggarwalShivani * Moved the entry to CHANGELOG.md Signed-off-by: aggarwalShivani --------- Signed-off-by: aggarwalShivani Signed-off-by: aggarwalShivani <39588384+aggarwalShivani@users.noreply.github.com> --- CHANGELOG.md | 1 + .../DedicatedClusterSnapshotRestoreIT.java | 3 +-- .../snapshots/SnapshotStatusApisIT.java | 27 +++++++++++++------ .../TransportSnapshotsStatusAction.java | 6 ++--- .../cluster/SnapshotsInProgress.java | 12 +++++++-- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb2f4287950c..fe6458937f791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix UOE While building Exists query for nested search_as_you_type field ([#12048](https://github.com/opensearch-project/OpenSearch/pull/12048)) - Client with Java 8 runtime and Apache HttpClient 5 Transport fails with java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer ([#13100](https://github.com/opensearch-project/opensearch-java/pull/13100)) - Fix implement mark() and markSupported() in class FilterStreamInput ([#13098](https://github.com/opensearch-project/OpenSearch/pull/13098)) +- Fix snapshot _status API to return correct status for partial snapshots ([#12812](https://github.com/opensearch-project/OpenSearch/pull/12812)) ### Security diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index 7a52c8aa5018e..54db951eb41c2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -572,7 +572,7 @@ public void testRestoreIndexWithMissingShards() throws Exception { List 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") @@ -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 { diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index c574233d25051..fb69209f7adda 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java @@ -40,7 +40,6 @@ import org.opensearch.action.admin.cluster.snapshots.status.SnapshotIndexShardStatus; import org.opensearch.action.admin.cluster.snapshots.status.SnapshotStats; import org.opensearch.action.admin.cluster.snapshots.status.SnapshotStatus; -import org.opensearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest; import org.opensearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; import org.opensearch.client.Client; import org.opensearch.cluster.SnapshotsInProgress; @@ -101,13 +100,9 @@ public void testStatusApiConsistency() { assertThat(snapshotInfo.state(), equalTo(SnapshotState.SUCCESS)); assertThat(snapshotInfo.version(), equalTo(Version.CURRENT)); - final List snapshotStatus = clusterAdmin().snapshotsStatus( - new SnapshotsStatusRequest("test-repo", new String[] { "test-snap" }) - ).actionGet().getSnapshots(); - assertThat(snapshotStatus.size(), equalTo(1)); - final SnapshotStatus snStatus = snapshotStatus.get(0); - assertEquals(snStatus.getStats().getStartTime(), snapshotInfo.startTime()); - assertEquals(snStatus.getStats().getTime(), snapshotInfo.endTime() - snapshotInfo.startTime()); + final SnapshotStatus snapshotStatus = getSnapshotStatus("test-repo", "test-snap"); + assertEquals(snapshotStatus.getStats().getStartTime(), snapshotInfo.startTime()); + assertEquals(snapshotStatus.getStats().getTime(), snapshotInfo.endTime() - snapshotInfo.startTime()); } public void testStatusAPICallForShallowCopySnapshot() { @@ -357,6 +352,22 @@ public void testSnapshotStatusOnFailedSnapshot() throws Exception { assertEquals(SnapshotsInProgress.State.FAILED, snapshotsStatusResponse.getSnapshots().get(0).getState()); } + public void testSnapshotStatusOnPartialSnapshot() throws Exception { + 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 SnapshotStatus snapshotStatus = getSnapshotStatus(repoName, snapshotName); + assertEquals(SnapshotsInProgress.State.PARTIAL, snapshotStatus.getState()); + } + public void testStatusAPICallInProgressShallowSnapshot() throws Exception { internalCluster().startClusterManagerOnlyNode(); internalCluster().startDataOnlyNode(); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java index 7f6c039cf2ecc..4fc2acb2caa51 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java @@ -356,11 +356,11 @@ private void loadRepositoryData( 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; default: throw new IllegalArgumentException("Unknown snapshot state " + snapshotInfo.state()); } diff --git a/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java b/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java index 3de23d2490c63..8dbdcaa541734 100644 --- a/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java +++ b/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java @@ -747,7 +747,12 @@ public void writeTo(StreamOutput out) throws IOException { 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) { + // Setting to SUCCESS for partial snapshots in older versions to maintain backward compatibility + out.writeByte(State.SUCCESS.value()); + } else { + out.writeByte(state.value()); + } out.writeList(indices); out.writeLong(startTime); out.writeMap(shards, (o, v) -> v.writeTo(o), (o, v) -> v.writeTo(o)); @@ -937,7 +942,8 @@ public enum State { STARTED((byte) 1, false), SUCCESS((byte) 2, true), FAILED((byte) 3, true), - ABORTED((byte) 4, false); + ABORTED((byte) 4, false), + PARTIAL((byte) 5, false); private final byte value; @@ -968,6 +974,8 @@ public static State fromValue(byte value) { return FAILED; case 4: return ABORTED; + case 5: + return PARTIAL; default: throw new IllegalArgumentException("No snapshot state for value [" + value + "]"); }