diff --git a/CHANGELOG.md b/CHANGELOG.md index eaefa369f4159..29821edef2752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix bulk API ignores ingest pipeline for upsert ([#12883](https://github.com/opensearch-project/OpenSearch/pull/12883)) - Fix issue with feature flags where default value may not be honored ([#12849](https://github.com/opensearch-project/OpenSearch/pull/12849)) - 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 ([#13260](https://github.com/opensearch-project/OpenSearch/pull/13260)) ### Security diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index 88e80ab7c0fe9..2b88dbc64e2af 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -574,7 +574,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") @@ -591,7 +591,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 539e941982606..1ec404856ce84 100644 --- a/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java +++ b/server/src/main/java/org/opensearch/cluster/SnapshotsInProgress.java @@ -776,7 +776,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_2_14_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)); @@ -983,7 +988,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; @@ -1014,6 +1020,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 + "]"); }