Skip to content

Commit eb6b9e8

Browse files
github-actions[bot]github-actions[bot]
committed
Snapshot _status API: Include in-progress snapshots in total shard count and index filter (#16394)
Signed-off-by: Lakshya Taragi <lakshya.taragi@gmail.com> (cherry picked from commit 6c7581e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent a31e7f2 commit eb6b9e8

File tree

4 files changed

+477
-72
lines changed

4 files changed

+477
-72
lines changed

‎server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java

+283-11
Original file line numberDiff line numberDiff line change
@@ -619,9 +619,9 @@ public void testSnapshotStatusApiFailureForTooManyShardsAcrossSnapshots() throws
619619
);
620620
assertEquals(exception.status(), RestStatus.TOO_MANY_REQUESTS);
621621
assertTrue(
622-
exception.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request")
622+
exception.getMessage().contains(" is more than the maximum allowed value of shard count [2] for snapshot status request")
623623
);
624-
}, 1, TimeUnit.MINUTES);
624+
});
625625

626626
// across multiple snapshots
627627
assertBusy(() -> {
@@ -636,13 +636,13 @@ public void testSnapshotStatusApiFailureForTooManyShardsAcrossSnapshots() throws
636636
);
637637
assertEquals(exception.status(), RestStatus.TOO_MANY_REQUESTS);
638638
assertTrue(
639-
exception.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request")
639+
exception.getMessage().contains(" is more than the maximum allowed value of shard count [2] for snapshot status request")
640640
);
641-
}, 1, TimeUnit.MINUTES);
641+
});
642642

643643
logger.info("Reset MAX_SHARDS_ALLOWED_IN_STATUS_API to default value");
644644
updateSettingsRequest.persistentSettings(Settings.builder().putNull(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey()));
645-
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
645+
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
646646
}
647647

648648
public void testSnapshotStatusForIndexFilter() throws Exception {
@@ -666,6 +666,7 @@ public void testSnapshotStatusForIndexFilter() throws Exception {
666666
String snapshot = "test-snap-1";
667667
createSnapshot(repositoryName, snapshot, List.of(index1, index2, index3));
668668

669+
// for a completed snapshot
669670
assertBusy(() -> {
670671
SnapshotStatus snapshotsStatus = client().admin()
671672
.cluster()
@@ -682,6 +683,96 @@ public void testSnapshotStatusForIndexFilter() throws Exception {
682683
}, 1, TimeUnit.MINUTES);
683684
}
684685

686+
public void testSnapshotStatusForIndexFilterForInProgressSnapshot() throws Exception {
687+
String repositoryName = "test-repo";
688+
createRepository(repositoryName, "mock", Settings.builder().put("location", randomRepoPath()).put("block_on_data", true));
689+
690+
logger.info("Create indices");
691+
String index1 = "test-idx-1";
692+
String index2 = "test-idx-2";
693+
String index3 = "test-idx-3";
694+
createIndex(index1, index2, index3);
695+
ensureGreen();
696+
697+
logger.info("Indexing some data");
698+
for (int i = 0; i < 10; i++) {
699+
index(index1, "_doc", Integer.toString(i), "foo", "bar" + i);
700+
index(index2, "_doc", Integer.toString(i), "foo", "baz" + i);
701+
index(index3, "_doc", Integer.toString(i), "foo", "baz" + i);
702+
}
703+
refresh();
704+
String inProgressSnapshot = "test-in-progress-snapshot";
705+
706+
logger.info("Create snapshot");
707+
ActionFuture<CreateSnapshotResponse> createSnapshotResponseActionFuture = startFullSnapshot(repositoryName, inProgressSnapshot);
708+
709+
logger.info("Block data node");
710+
waitForBlockOnAnyDataNode(repositoryName, TimeValue.timeValueMinutes(1));
711+
awaitNumberOfSnapshotsInProgress(1);
712+
713+
// test normal functioning of index filter for in progress snapshot
714+
assertBusy(() -> {
715+
SnapshotStatus snapshotsStatus = client().admin()
716+
.cluster()
717+
.prepareSnapshotStatus(repositoryName)
718+
.setSnapshots(inProgressSnapshot)
719+
.setIndices(index1, index2)
720+
.get()
721+
.getSnapshots()
722+
.get(0);
723+
Map<String, SnapshotIndexStatus> snapshotIndexStatusMap = snapshotsStatus.getIndices();
724+
// Although the snapshot contains 3 indices, the response of status api call only contains results for 2
725+
assertEquals(snapshotIndexStatusMap.size(), 2);
726+
assertEquals(snapshotIndexStatusMap.keySet(), Set.of(index1, index2));
727+
});
728+
729+
// when a non-existent index is requested in the index-filter
730+
assertBusy(() -> {
731+
// failure due to index not found in snapshot
732+
final String nonExistentIndex1 = "non-existent-index-1";
733+
final String nonExistentIndex2 = "non-existent-index-2";
734+
Exception ex = expectThrows(
735+
Exception.class,
736+
() -> client().admin()
737+
.cluster()
738+
.prepareSnapshotStatus(repositoryName)
739+
.setSnapshots(inProgressSnapshot)
740+
.setIndices(index1, index2, nonExistentIndex1, nonExistentIndex2)
741+
.execute()
742+
.actionGet()
743+
);
744+
String cause = String.format(
745+
Locale.ROOT,
746+
"indices [%s] missing in snapshot [%s] of repository [%s]",
747+
String.join(", ", List.of(nonExistentIndex2, nonExistentIndex1)),
748+
inProgressSnapshot,
749+
repositoryName
750+
);
751+
assertEquals(cause, ex.getCause().getMessage());
752+
753+
// no error for ignore_unavailable = true and status response contains only the found indices
754+
SnapshotStatus snapshotsStatus = client().admin()
755+
.cluster()
756+
.prepareSnapshotStatus(repositoryName)
757+
.setSnapshots(inProgressSnapshot)
758+
.setIndices(index1, index2, nonExistentIndex1, nonExistentIndex2)
759+
.setIgnoreUnavailable(true)
760+
.get()
761+
.getSnapshots()
762+
.get(0);
763+
764+
Map<String, SnapshotIndexStatus> snapshotIndexStatusMap = snapshotsStatus.getIndices();
765+
assertEquals(snapshotIndexStatusMap.size(), 2);
766+
assertEquals(snapshotIndexStatusMap.keySet(), Set.of(index1, index2));
767+
});
768+
769+
logger.info("Unblock data node");
770+
unblockAllDataNodes(repositoryName);
771+
772+
logger.info("Wait for snapshot to finish");
773+
waitForCompletion(repositoryName, inProgressSnapshot, TimeValue.timeValueSeconds(60));
774+
}
775+
685776
public void testSnapshotStatusFailuresWithIndexFilter() throws Exception {
686777
String repositoryName = "test-repo";
687778
String index1 = "test-idx-1";
@@ -705,6 +796,39 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception {
705796
createSnapshot(repositoryName, snapshot1, List.of(index1, index2, index3));
706797
createSnapshot(repositoryName, snapshot2, List.of(index1));
707798

799+
assertBusy(() -> {
800+
// failure due to passing index filter for _all value of repository param
801+
Exception ex = expectThrows(
802+
Exception.class,
803+
() -> client().admin()
804+
.cluster()
805+
.prepareSnapshotStatus("_all")
806+
.setSnapshots(snapshot1)
807+
.setIndices(index1, index2, index3)
808+
.execute()
809+
.actionGet()
810+
);
811+
String cause =
812+
"index list filter is supported only when a single 'repository' is passed, but found 'repository' param = [_all]";
813+
assertTrue(ex.getMessage().contains(cause));
814+
});
815+
816+
assertBusy(() -> {
817+
// failure due to passing index filter for _all value of snapshot param --> gets translated as a blank array
818+
Exception ex = expectThrows(
819+
Exception.class,
820+
() -> client().admin()
821+
.cluster()
822+
.prepareSnapshotStatus(repositoryName)
823+
.setSnapshots()
824+
.setIndices(index1, index2, index3)
825+
.execute()
826+
.actionGet()
827+
);
828+
String cause = "index list filter is supported only when a single 'snapshot' is passed, but found 'snapshot' param = [_all]";
829+
assertTrue(ex.getMessage().contains(cause));
830+
});
831+
708832
assertBusy(() -> {
709833
// failure due to passing index filter for multiple snapshots
710834
ActionRequestValidationException ex = expectThrows(
@@ -717,9 +841,10 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception {
717841
.execute()
718842
.actionGet()
719843
);
720-
String cause = "index list filter is supported only for a single snapshot";
844+
String cause =
845+
"index list filter is supported only when a single 'snapshot' is passed, but found 'snapshot' param = [[test-snap-1, test-snap-2]]";
721846
assertTrue(ex.getMessage().contains(cause));
722-
}, 1, TimeUnit.MINUTES);
847+
});
723848

724849
assertBusy(() -> {
725850
// failure due to index not found in snapshot
@@ -743,7 +868,18 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception {
743868
);
744869
assertEquals(cause, ex.getCause().getMessage());
745870

746-
}, 1, TimeUnit.MINUTES);
871+
// no error for ignore_unavailable = true and status response contains only the found indices
872+
SnapshotStatus snapshotsStatus = client().admin()
873+
.cluster()
874+
.prepareSnapshotStatus(repositoryName)
875+
.setSnapshots(snapshot2)
876+
.setIndices(index1, index2, index3)
877+
.setIgnoreUnavailable(true)
878+
.get()
879+
.getSnapshots()
880+
.get(0);
881+
assertEquals(1, snapshotsStatus.getIndices().size());
882+
});
747883

748884
assertBusy(() -> {
749885
// failure due to too many shards requested
@@ -763,12 +899,148 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception {
763899
.actionGet()
764900
);
765901
assertEquals(ex.status(), RestStatus.TOO_MANY_REQUESTS);
766-
assertTrue(ex.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request"));
902+
assertTrue(ex.getMessage().contains(" is more than the maximum allowed value of shard count [2] for snapshot status request"));
767903

768904
logger.info("Reset MAX_SHARDS_ALLOWED_IN_STATUS_API to default value");
769905
updateSettingsRequest.persistentSettings(Settings.builder().putNull(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey()));
770-
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
771-
}, 2, TimeUnit.MINUTES);
906+
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
907+
});
908+
}
909+
910+
public void testSnapshotStatusShardLimitOfResponseForInProgressSnapshot() throws Exception {
911+
logger.info("Create repository");
912+
String repositoryName = "test-repo";
913+
createRepository(
914+
repositoryName,
915+
"mock",
916+
Settings.builder()
917+
.put("location", randomRepoPath())
918+
.put("compress", false)
919+
.put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)
920+
.put("wait_after_unblock", 200)
921+
);
922+
923+
logger.info("Create indices");
924+
String index1 = "test-idx-1";
925+
String index2 = "test-idx-2";
926+
String index3 = "test-idx-3";
927+
assertAcked(prepareCreate(index1, 1, Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0)));
928+
assertAcked(prepareCreate(index2, 1, Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0)));
929+
assertAcked(prepareCreate(index3, 1, Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0)));
930+
ensureGreen();
931+
932+
logger.info("Index some data");
933+
indexRandomDocs(index1, 10);
934+
indexRandomDocs(index2, 10);
935+
indexRandomDocs(index3, 10);
936+
937+
logger.info("Create completed snapshot");
938+
String completedSnapshot = "test-completed-snapshot";
939+
String blockedNode = blockNodeWithIndex(repositoryName, index1);
940+
client().admin().cluster().prepareCreateSnapshot(repositoryName, completedSnapshot).setWaitForCompletion(false).get();
941+
waitForBlock(blockedNode, repositoryName, TimeValue.timeValueSeconds(60));
942+
unblockNode(repositoryName, blockedNode);
943+
waitForCompletion(repositoryName, completedSnapshot, TimeValue.timeValueSeconds(60));
944+
945+
logger.info("Index some more data");
946+
indexRandomDocs(index1, 10);
947+
indexRandomDocs(index2, 10);
948+
indexRandomDocs(index3, 10);
949+
refresh();
950+
951+
logger.info("Create in-progress snapshot");
952+
String inProgressSnapshot = "test-in-progress-snapshot";
953+
blockedNode = blockNodeWithIndex(repositoryName, index1);
954+
client().admin().cluster().prepareCreateSnapshot(repositoryName, inProgressSnapshot).setWaitForCompletion(false).get();
955+
waitForBlock(blockedNode, repositoryName, TimeValue.timeValueSeconds(60));
956+
List<SnapshotStatus> snapshotStatuses = client().admin()
957+
.cluster()
958+
.prepareSnapshotStatus(repositoryName)
959+
.setSnapshots(inProgressSnapshot, completedSnapshot)
960+
.get()
961+
.getSnapshots();
962+
963+
assertEquals(2, snapshotStatuses.size());
964+
assertEquals(SnapshotsInProgress.State.STARTED, snapshotStatuses.get(0).getState());
965+
assertEquals(SnapshotsInProgress.State.SUCCESS, snapshotStatuses.get(1).getState());
966+
967+
logger.info("Set MAX_SHARDS_ALLOWED_IN_STATUS_API to a low value");
968+
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest();
969+
updateSettingsRequest.persistentSettings(Settings.builder().put(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey(), 1));
970+
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
971+
972+
// shard limit exceeded due to inProgress snapshot alone @ without index-filter
973+
assertBusy(() -> {
974+
CircuitBreakingException exception = expectThrows(
975+
CircuitBreakingException.class,
976+
() -> client().admin()
977+
.cluster()
978+
.prepareSnapshotStatus(repositoryName)
979+
.setSnapshots(inProgressSnapshot)
980+
.execute()
981+
.actionGet()
982+
);
983+
assertEquals(exception.status(), RestStatus.TOO_MANY_REQUESTS);
984+
assertTrue(
985+
exception.getMessage().contains(" is more than the maximum allowed value of shard count [1] for snapshot status request")
986+
);
987+
});
988+
989+
// shard limit exceeded due to inProgress snapshot alone @ with index-filter
990+
assertBusy(() -> {
991+
CircuitBreakingException exception = expectThrows(
992+
CircuitBreakingException.class,
993+
() -> client().admin()
994+
.cluster()
995+
.prepareSnapshotStatus(repositoryName)
996+
.setSnapshots(inProgressSnapshot)
997+
.setIndices(index1, index2)
998+
.execute()
999+
.actionGet()
1000+
);
1001+
assertEquals(exception.status(), RestStatus.TOO_MANY_REQUESTS);
1002+
assertTrue(
1003+
exception.getMessage().contains(" is more than the maximum allowed value of shard count [1] for snapshot status request")
1004+
);
1005+
});
1006+
1007+
logger.info("Set MAX_SHARDS_ALLOWED_IN_STATUS_API to a slightly higher value");
1008+
updateSettingsRequest.persistentSettings(Settings.builder().put(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey(), 5));
1009+
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
1010+
1011+
// shard limit exceeded due to passing for inProgress but failing for current + completed
1012+
1013+
assertBusy(() -> {
1014+
SnapshotStatus inProgressSnapshotStatus = client().admin()
1015+
.cluster()
1016+
.prepareSnapshotStatus(repositoryName)
1017+
.setSnapshots(inProgressSnapshot)
1018+
.get()
1019+
.getSnapshots()
1020+
.get(0);
1021+
assertEquals(3, inProgressSnapshotStatus.getShards().size());
1022+
1023+
CircuitBreakingException exception = expectThrows(
1024+
CircuitBreakingException.class,
1025+
() -> client().admin()
1026+
.cluster()
1027+
.prepareSnapshotStatus(repositoryName)
1028+
.setSnapshots(inProgressSnapshot, completedSnapshot)
1029+
.execute()
1030+
.actionGet()
1031+
);
1032+
assertEquals(exception.status(), RestStatus.TOO_MANY_REQUESTS);
1033+
assertTrue(
1034+
exception.getMessage().contains(" is more than the maximum allowed value of shard count [5] for snapshot status request")
1035+
);
1036+
});
1037+
1038+
unblockNode(repositoryName, blockedNode);
1039+
waitForCompletion(repositoryName, inProgressSnapshot, TimeValue.timeValueSeconds(60));
1040+
1041+
logger.info("Reset MAX_SHARDS_ALLOWED_IN_STATUS_API to default value");
1042+
updateSettingsRequest.persistentSettings(Settings.builder().putNull(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey()));
1043+
assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
7721044
}
7731045

7741046
private static SnapshotIndexShardStatus stateFirstShard(SnapshotStatus snapshotStatus, String indexName) {

‎server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java

+15-2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.opensearch.core.common.io.stream.StreamOutput;
4242

4343
import java.io.IOException;
44+
import java.util.Arrays;
4445

4546
import static org.opensearch.action.ValidateActions.addValidationError;
4647

@@ -124,8 +125,20 @@ public ActionRequestValidationException validate() {
124125
if (snapshots == null) {
125126
validationException = addValidationError("snapshots is null", validationException);
126127
}
127-
if (indices.length != 0 && snapshots.length != 1) {
128-
validationException = addValidationError("index list filter is supported only for a single snapshot", validationException);
128+
if (indices.length != 0) {
129+
if (repository.equals("_all")) {
130+
String error =
131+
"index list filter is supported only when a single 'repository' is passed, but found 'repository' param = [_all]";
132+
validationException = addValidationError(error, validationException);
133+
}
134+
if (snapshots.length != 1) {
135+
// snapshot param was '_all' (length = 0) or a list of snapshots (length > 1)
136+
String snapshotParamValue = snapshots.length == 0 ? "_all" : Arrays.toString(snapshots);
137+
String error = "index list filter is supported only when a single 'snapshot' is passed, but found 'snapshot' param = ["
138+
+ snapshotParamValue
139+
+ "]";
140+
validationException = addValidationError(error, validationException);
141+
}
129142
}
130143
return validationException;
131144
}

0 commit comments

Comments
 (0)