From 274af5c1db7e3ba8eae8df89bff713182e42fbf0 Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Thu, 21 Mar 2024 10:13:51 +0530 Subject: [PATCH 01/10] Snapshot _status API to return correct status for partial snapshots Signed-off-by: aggarwalShivani --- .../snapshots/SnapshotStatusApisIT.java | 16 ++++++++++++++++ .../status/TransportSnapshotsStatusAction.java | 6 +++--- .../opensearch/cluster/SnapshotsInProgress.java | 5 ++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index c574233d25051..4eebec8968e9e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java @@ -357,6 +357,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"; + createRepository(repoName, "fs"); + createIndex("test-idx"); + logger.info("--> stopping data node before creating snapshot"); + stopNode(dataNode); + startFullSnapshot(repoName, snapshotName, true).get(); + final List snapshotStatus = clusterAdmin().snapshotsStatus( + new SnapshotsStatusRequest(repoName, new String[] { snapshotName }) + ).actionGet().getSnapshots(); + assertThat(snapshotStatus.size(), equalTo(1)); + assertEquals(SnapshotsInProgress.State.PARTIAL, snapshotStatus.get(0).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..d3d8ecbba7018 100644 --- a/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java +++ b/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java @@ -937,7 +937,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 +969,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 + "]"); } From 455cfb3c79c40347369e0de2b78b8a15a7f593cd Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Thu, 28 Mar 2024 12:59:48 +0530 Subject: [PATCH 02/10] Updated CHANGELOG.md Signed-off-by: aggarwalShivani --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b5a7cc705a79..53051dca844b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,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 From 001564bb9b5cb4ac48bce07b95742cf47cbcd20d Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Fri, 29 Mar 2024 21:16:19 +0530 Subject: [PATCH 03/10] Updated test case Signed-off-by: aggarwalShivani --- .../snapshots/DedicatedClusterSnapshotRestoreIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 { From 30dbde496d702e578fd21aa38b399f02ca0b559d Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Tue, 2 Apr 2024 00:05:42 +0530 Subject: [PATCH 04/10] Setting snapshot status to SUCCESS for older versions for bwc Signed-off-by: aggarwalShivani --- .../snapshots/status/TransportSnapshotsStatusAction.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 4fc2acb2caa51..51ad70184a553 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 @@ -34,6 +34,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.Version; import org.opensearch.action.ActionRunnable; import org.opensearch.action.StepListener; import org.opensearch.action.support.ActionFilters; @@ -359,7 +360,11 @@ private void loadRepositoryData( state = SnapshotsInProgress.State.SUCCESS; break; case PARTIAL: - state = SnapshotsInProgress.State.PARTIAL; + if (Version.CURRENT.onOrAfter(Version.V_3_0_0)) + state = SnapshotsInProgress.State.PARTIAL; + else + // Setting to SUCCESS in older versions to maintain backward compatibility + state = SnapshotsInProgress.State.SUCCESS; break; default: throw new IllegalArgumentException("Unknown snapshot state " + snapshotInfo.state()); From d5e1380456f28107af887ef3418e1efa5f3e9935 Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Tue, 2 Apr 2024 00:05:42 +0530 Subject: [PATCH 05/10] Setting snapshot status to SUCCESS for older versions for bwc Signed-off-by: aggarwalShivani --- .../snapshots/status/TransportSnapshotsStatusAction.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 51ad70184a553..97a55cf137361 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 @@ -360,8 +360,7 @@ private void loadRepositoryData( state = SnapshotsInProgress.State.SUCCESS; break; case PARTIAL: - if (Version.CURRENT.onOrAfter(Version.V_3_0_0)) - state = SnapshotsInProgress.State.PARTIAL; + if (Version.CURRENT.onOrAfter(Version.V_3_0_0)) state = SnapshotsInProgress.State.PARTIAL; else // Setting to SUCCESS in older versions to maintain backward compatibility state = SnapshotsInProgress.State.SUCCESS; From 0a216964d407321fb5d83604ab3186d1e069ff37 Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Fri, 5 Apr 2024 13:34:07 +0530 Subject: [PATCH 06/10] Moved BWC change to SnapshotsInProgress.java for partial snapshots Signed-off-by: aggarwalShivani --- .../snapshots/status/TransportSnapshotsStatusAction.java | 6 +----- .../java/org/opensearch/cluster/SnapshotsInProgress.java | 7 ++++++- 2 files changed, 7 insertions(+), 6 deletions(-) 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 97a55cf137361..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 @@ -34,7 +34,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.Version; import org.opensearch.action.ActionRunnable; import org.opensearch.action.StepListener; import org.opensearch.action.support.ActionFilters; @@ -360,10 +359,7 @@ private void loadRepositoryData( state = SnapshotsInProgress.State.SUCCESS; break; case PARTIAL: - if (Version.CURRENT.onOrAfter(Version.V_3_0_0)) state = SnapshotsInProgress.State.PARTIAL; - else - // Setting to SUCCESS in older versions to maintain backward compatibility - state = SnapshotsInProgress.State.SUCCESS; + 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 d3d8ecbba7018..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)); From 3424b6a65f423986a8b0caa958c521f05ab02d7e Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Tue, 9 Apr 2024 19:35:28 +0530 Subject: [PATCH 07/10] Fix for flaky test testSnapshotStatusOnPartialSnapshot Signed-off-by: aggarwalShivani --- .../java/org/opensearch/snapshots/SnapshotStatusApisIT.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index 4eebec8968e9e..e607b668fb334 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java @@ -361,8 +361,11 @@ 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"); - createIndex("test-idx"); + // 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(); From ab01a692ac63299ef5a2e80e00c6bd1b04040b22 Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Fri, 12 Apr 2024 23:47:43 +0530 Subject: [PATCH 08/10] Updated the testcases to reuse existing getSnapshotStatus() method Signed-off-by: aggarwalShivani --- CHANGELOG-3.0.md | 1 + .../snapshots/SnapshotStatusApisIT.java | 17 +++++------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md index 0715c6de49ca4..7ce1516fb4bad 100644 --- a/CHANGELOG-3.0.md +++ b/CHANGELOG-3.0.md @@ -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 diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index e607b668fb334..c9af40f12b42a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java @@ -101,13 +101,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() { @@ -369,11 +365,8 @@ public void testSnapshotStatusOnPartialSnapshot() throws Exception { logger.info("--> stopping data node before creating snapshot"); stopNode(dataNode); startFullSnapshot(repoName, snapshotName, true).get(); - final List snapshotStatus = clusterAdmin().snapshotsStatus( - new SnapshotsStatusRequest(repoName, new String[] { snapshotName }) - ).actionGet().getSnapshots(); - assertThat(snapshotStatus.size(), equalTo(1)); - assertEquals(SnapshotsInProgress.State.PARTIAL, snapshotStatus.get(0).getState()); + final SnapshotStatus snapshotStatus = getSnapshotStatus(repoName, snapshotName); + assertEquals(SnapshotsInProgress.State.PARTIAL, snapshotStatus.getState()); } public void testStatusAPICallInProgressShallowSnapshot() throws Exception { From d1e66568a4a5b56e13730b3b55b1603cdeb55c84 Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Sat, 13 Apr 2024 00:29:26 +0530 Subject: [PATCH 09/10] Fixed formatting issues detected in spotlessJavaCheck Signed-off-by: aggarwalShivani --- .../java/org/opensearch/snapshots/SnapshotStatusApisIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index c9af40f12b42a..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; From c0b1ae18a15f679ce70eb5d33d37b375926c1210 Mon Sep 17 00:00:00 2001 From: aggarwalShivani Date: Mon, 15 Apr 2024 23:14:42 +0530 Subject: [PATCH 10/10] Moved the entry to CHANGELOG.md Signed-off-by: aggarwalShivani --- CHANGELOG-3.0.md | 1 - CHANGELOG.md | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md index 7ce1516fb4bad..0715c6de49ca4 100644 --- a/CHANGELOG-3.0.md +++ b/CHANGELOG-3.0.md @@ -98,7 +98,6 @@ 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 88a8f57c0afdc..68a2c746ec89b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,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