From b5cab5b368c9cba8d0d8c83d5004f2651ad9f416 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Thu, 29 Aug 2024 16:40:02 +0530 Subject: [PATCH] Register and use custom exception Signed-off-by: Lakshya Taragi --- .../rest-api-spec/api/snapshot.status.json | 22 +++++- .../RemoteIndexSnapshotStatusApiIT.java | 22 +++--- .../snapshots/SnapshotStatusApisIT.java | 40 +++++------ .../opensearch/OpenSearchServerException.java | 9 +++ .../status/SnapshotsStatusRequest.java | 4 +- .../TransportSnapshotsStatusAction.java | 33 +++------ ...oManyShardsInSnapshotsStatusException.java | 69 +++++++++++++++++++ .../ExceptionSerializationTests.java | 2 + 8 files changed, 146 insertions(+), 55 deletions(-) create mode 100644 server/src/main/java/org/opensearch/snapshots/TooManyShardsInSnapshotsStatusException.java diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.status.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.status.json index 1ac6042941013..354d3c35d2bda 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.status.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.status.json @@ -40,6 +40,26 @@ "description":"A comma-separated list of snapshot names" } } + }, + { + "path":"/_snapshot/{repository}/{snapshot}/{index}/_status", + "methods":[ + "GET" + ], + "parts":{ + "repository":{ + "type":"string", + "description":"A repository name" + }, + "snapshot":{ + "type":"string", + "description":"A snapshot name" + }, + "index":{ + "type": "list", + "description":"A comma-separated list of index names" + } + } } ] }, @@ -58,7 +78,7 @@ }, "ignore_unavailable":{ "type":"boolean", - "description":"Whether to ignore unavailable snapshots, defaults to false which means a SnapshotMissingException is thrown" + "description":"Whether to ignore unavailable snapshots and indices, defaults to false which means a SnapshotMissingException or IndexNotFoundException is thrown" } } } diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java index 7d349462e6707..e84de36df2fca 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/RemoteIndexSnapshotStatusApiIT.java @@ -41,6 +41,7 @@ import org.opensearch.cluster.SnapshotsInProgress; import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; +import org.opensearch.indices.RemoteStoreSettings; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.test.OpenSearchIntegTestCase; import org.opensearch.threadpool.ThreadPool; @@ -201,20 +202,25 @@ public void testStatusAPICallInProgressShallowSnapshot() throws Exception { public void testStatusAPICallForShallowV2Snapshot() throws Exception { disableRepoConsistencyCheck("Remote store repository is being used for the test"); - internalCluster().startClusterManagerOnlyNode(); - internalCluster().startDataOnlyNodes(2); + Settings pinnedTimestampSettings = Settings.builder() + .put(RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED.getKey(), true) + .build(); + internalCluster().startClusterManagerOnlyNode(pinnedTimestampSettings); + internalCluster().startDataOnlyNodes(2, pinnedTimestampSettings); - logger.info("Create repository for shallow V2 snapshots"); + final String index1 = "remote-index-1"; + final String index2 = "remote-index-2"; + final String index3 = "remote-index-3"; final String snapshotRepoName = "snapshot-repo-name"; + final String snapshot = "snapshot"; + + logger.info("Create repository for shallow V2 snapshots"); Settings.Builder snapshotV2RepoSettings = snapshotRepoSettingsForShallowCopy().put( BlobStoreRepository.SHALLOW_SNAPSHOT_V2.getKey(), Boolean.TRUE ); createRepository(snapshotRepoName, "fs", snapshotV2RepoSettings); - final String index1 = "remote-index-1"; - final String index2 = "remote-index-2"; - final String index3 = "remote-index-3"; final Settings remoteStoreEnabledIndexSettings = getRemoteStoreBackedIndexSettings(); createIndex(index1, remoteStoreEnabledIndexSettings); createIndex(index2, remoteStoreEnabledIndexSettings); @@ -229,7 +235,6 @@ public void testStatusAPICallForShallowV2Snapshot() throws Exception { } refresh(); - final String snapshot = "snapshot"; SnapshotInfo snapshotInfo = createFullSnapshot(snapshotRepoName, snapshot); assertTrue(snapshotInfo.getPinnedTimestamp() > 0); // to assert creation of a shallow v2 snapshot @@ -238,8 +243,8 @@ public void testStatusAPICallForShallowV2Snapshot() throws Exception { updateSettingsRequest.persistentSettings(Settings.builder().put(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey(), 2)); assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); - // without index filter assertBusy(() -> { + // without index filter // although no. of shards in snapshot (3) is greater than the max value allowed in a status api call, the request does not fail SnapshotStatus snapshotStatusWithoutIndexFilter = client().admin() .cluster() @@ -252,6 +257,7 @@ public void testStatusAPICallForShallowV2Snapshot() throws Exception { assertShallowV2SnapshotStatus(snapshotStatusWithoutIndexFilter, false); + // with index filter SnapshotStatus snapshotStatusWithIndexFilter = client().admin() .cluster() .prepareSnapshotStatus(snapshotRepoName) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java index d1a4bf71fb4bb..513c1fa578589 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SnapshotStatusApisIT.java @@ -53,6 +53,7 @@ import org.opensearch.core.common.Strings; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.rest.RestStatus; +import org.opensearch.index.IndexNotFoundException; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.threadpool.ThreadPool; @@ -601,21 +602,20 @@ public void testSnapshotStatusApiFailureForTooManyShardsAcrossSnapshots() throws // across a single snapshot assertBusy(() -> { - SnapshotException snapshotException = expectThrows( - SnapshotException.class, + TooManyShardsInSnapshotsStatusException exception = expectThrows( + TooManyShardsInSnapshotsStatusException.class, () -> client().admin().cluster().prepareSnapshotStatus(repositoryName).setSnapshots(snapshot1).execute().actionGet() ); - assertEquals(snapshotException.status(), RestStatus.REQUEST_ENTITY_TOO_LARGE); + assertEquals(exception.status(), RestStatus.REQUEST_ENTITY_TOO_LARGE); assertTrue( - snapshotException.getMessage() - .endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request") + exception.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request") ); }, 1, TimeUnit.MINUTES); // across multiple snapshots assertBusy(() -> { - SnapshotException snapshotException = expectThrows( - SnapshotException.class, + TooManyShardsInSnapshotsStatusException exception = expectThrows( + TooManyShardsInSnapshotsStatusException.class, () -> client().admin() .cluster() .prepareSnapshotStatus(repositoryName) @@ -623,10 +623,9 @@ public void testSnapshotStatusApiFailureForTooManyShardsAcrossSnapshots() throws .execute() .actionGet() ); - assertEquals(snapshotException.status(), RestStatus.REQUEST_ENTITY_TOO_LARGE); + assertEquals(exception.status(), RestStatus.REQUEST_ENTITY_TOO_LARGE); assertTrue( - snapshotException.getMessage() - .endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request") + exception.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request") ); }, 1, TimeUnit.MINUTES); @@ -713,8 +712,8 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception { assertBusy(() -> { // failure due to index not found in snapshot - SnapshotException ex = expectThrows( - SnapshotException.class, + IndexNotFoundException ex = expectThrows( + IndexNotFoundException.class, () -> client().admin() .cluster() .prepareSnapshotStatus(repositoryName) @@ -726,13 +725,12 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception { assertEquals(ex.status(), RestStatus.NOT_FOUND); String cause = String.format( Locale.ROOT, - "[%s:%s] indices [%s] missing in snapshot [%s]", - repositoryName, - snapshot2, + "indices [%s] missing in snapshot [%s] of repository [%s]", String.join(", ", List.of(index2, index3)), - snapshot2 + snapshot2, + repositoryName ); - assertEquals(cause, ex.getMessage()); + assertEquals(cause, ex.getCause().getMessage()); }, 1, TimeUnit.MINUTES); @@ -743,8 +741,8 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception { updateSettingsRequest.persistentSettings(Settings.builder().put(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey(), 2)); assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); - SnapshotException ex3 = expectThrows( - SnapshotException.class, + TooManyShardsInSnapshotsStatusException ex = expectThrows( + TooManyShardsInSnapshotsStatusException.class, () -> client().admin() .cluster() .prepareSnapshotStatus(repositoryName) @@ -753,8 +751,8 @@ public void testSnapshotStatusFailuresWithIndexFilter() throws Exception { .execute() .actionGet() ); - assertEquals(ex3.status(), RestStatus.REQUEST_ENTITY_TOO_LARGE); - assertTrue(ex3.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request")); + assertEquals(ex.status(), RestStatus.REQUEST_ENTITY_TOO_LARGE); + assertTrue(ex.getMessage().endsWith(" is more than the maximum allowed value of shard count [2] for snapshot status request")); logger.info("Reset MAX_SHARDS_ALLOWED_IN_STATUS_API to default value"); updateSettingsRequest.persistentSettings(Settings.builder().putNull(MAX_SHARDS_ALLOWED_IN_STATUS_API.getKey())); diff --git a/server/src/main/java/org/opensearch/OpenSearchServerException.java b/server/src/main/java/org/opensearch/OpenSearchServerException.java index c5a5ce12b238c..6cd835eb2ed1a 100644 --- a/server/src/main/java/org/opensearch/OpenSearchServerException.java +++ b/server/src/main/java/org/opensearch/OpenSearchServerException.java @@ -11,6 +11,7 @@ import static org.opensearch.OpenSearchException.OpenSearchExceptionHandle; import static org.opensearch.OpenSearchException.OpenSearchExceptionHandleRegistry.registerExceptionHandle; import static org.opensearch.OpenSearchException.UNKNOWN_VERSION_ADDED; +import static org.opensearch.Version.CURRENT; import static org.opensearch.Version.V_2_10_0; import static org.opensearch.Version.V_2_13_0; import static org.opensearch.Version.V_2_1_0; @@ -1201,6 +1202,14 @@ public static void registerExceptions() { V_2_13_0 ) ); + registerExceptionHandle( + new OpenSearchExceptionHandle( + org.opensearch.snapshots.TooManyShardsInSnapshotsStatusException.class, + org.opensearch.snapshots.TooManyShardsInSnapshotsStatusException::new, + 174, + CURRENT + ) + ); registerExceptionHandle( new OpenSearchExceptionHandle( org.opensearch.cluster.block.IndexCreateBlockException.class, diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java index 6c3438fbe58f6..87e863f8dd286 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java @@ -78,7 +78,7 @@ public SnapshotsStatusRequest(StreamInput in) throws IOException { repository = in.readString(); snapshots = in.readStringArray(); ignoreUnavailable = in.readBoolean(); - if (in.getVersion().onOrAfter(Version.V_2_17_0)) { + if (in.getVersion().onOrAfter(Version.CURRENT)) { indices = in.readOptionalStringArray(); } } @@ -89,7 +89,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(repository); out.writeStringArray(snapshots); out.writeBoolean(ignoreUnavailable); - if (out.getVersion().onOrAfter(Version.V_2_17_0)) { + if (out.getVersion().onOrAfter(Version.CURRENT)) { out.writeOptionalStringArray(indices); } } 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 5b7affd51cb11..f2a9b88f790c9 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 @@ -52,14 +52,13 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.util.CollectionUtils; import org.opensearch.core.index.shard.ShardId; -import org.opensearch.core.rest.RestStatus; +import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.snapshots.IndexShardSnapshotStatus; import org.opensearch.repositories.IndexId; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; import org.opensearch.repositories.RepositoryData; import org.opensearch.snapshots.Snapshot; -import org.opensearch.snapshots.SnapshotException; import org.opensearch.snapshots.SnapshotId; import org.opensearch.snapshots.SnapshotInfo; import org.opensearch.snapshots.SnapshotMissingException; @@ -67,6 +66,7 @@ import org.opensearch.snapshots.SnapshotShardsService; import org.opensearch.snapshots.SnapshotState; import org.opensearch.snapshots.SnapshotsService; +import org.opensearch.snapshots.TooManyShardsInSnapshotsStatusException; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; @@ -458,17 +458,13 @@ private Map snapshotsInfo( snapshotsInfoMap.put(snapshotId, snapshotInfo); } if (totalShardsAcrossSnapshots > maximumAllowedShardCount && request.indices().length == 0) { - String cause = "Total shard count [" + String message = "Total shard count [" + totalShardsAcrossSnapshots + "] is more than the maximum allowed value of shard count [" + maximumAllowedShardCount + "] for snapshot status request"; - throw new SnapshotException(repositoryName, String.join(", ", requestedSnapshotNames), cause) { - @Override - public RestStatus status() { - return RestStatus.REQUEST_ENTITY_TOO_LARGE; - } - }; + + throw new TooManyShardsInSnapshotsStatusException(repositoryName, message, request.snapshots()); } return unmodifiableMap(snapshotsInfoMap); } @@ -524,19 +520,15 @@ private Map snapshotShards( } if (totalShardsAcrossIndices > maximumAllowedShardCount && requestedIndexNames.isEmpty() == false && isV2Snapshot == false) { - String cause = "Total shard count [" + String message = "Total shard count [" + totalShardsAcrossIndices + "] across the requested indices [" + requestedIndexNames.stream().collect(Collectors.joining(", ")) + "] is more than the maximum allowed value of shard count [" + maximumAllowedShardCount + "] for snapshot status request"; - throw new SnapshotException(repositoryName, snapshotName, cause) { - @Override - public RestStatus status() { - return RestStatus.REQUEST_ENTITY_TOO_LARGE; - } - }; + + throw new TooManyShardsInSnapshotsStatusException(repositoryName, message, snapshotName); } final Map shardStatus = new HashMap<>(); @@ -587,13 +579,8 @@ private void handleIndexNotFound(String index, SnapshotsStatusRequest request, S repositoryName ); } else { - String cause = "indices [" + index + "] missing in snapshot [" + snapshotName + "]"; - throw new SnapshotException(repositoryName, snapshotName, cause) { - @Override - public RestStatus status() { - return RestStatus.NOT_FOUND; - } - }; + String cause = "indices [" + index + "] missing in snapshot [" + snapshotName + "] of repository [" + repositoryName + "]"; + throw new IndexNotFoundException(index, new IllegalArgumentException(cause)); } } diff --git a/server/src/main/java/org/opensearch/snapshots/TooManyShardsInSnapshotsStatusException.java b/server/src/main/java/org/opensearch/snapshots/TooManyShardsInSnapshotsStatusException.java new file mode 100644 index 0000000000000..1689b3e4941ec --- /dev/null +++ b/server/src/main/java/org/opensearch/snapshots/TooManyShardsInSnapshotsStatusException.java @@ -0,0 +1,69 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.snapshots; + +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.rest.RestStatus; + +import java.io.IOException; + +/** + * Thrown if the number of shards across the requested resources (snapshot(s) or the index/indices of a particular snapshot) + * breaches the limit of snapshot.max_shards_allowed_in_status_api cluster setting + * + * @opensearch.internal + */ +public class TooManyShardsInSnapshotsStatusException extends SnapshotException { + + public TooManyShardsInSnapshotsStatusException( + final String repositoryName, + final SnapshotId snapshotId, + final String message, + final Throwable cause + ) { + super(repositoryName, snapshotId, message, cause); + } + + public TooManyShardsInSnapshotsStatusException(final String repositoryName, final String message, String... snapshotName) { + super(repositoryName, String.join(", ", snapshotName), message); + } + + public TooManyShardsInSnapshotsStatusException(StreamInput in) throws IOException { + super(in); + } + + @Override + public RestStatus status() { + return RestStatus.REQUEST_ENTITY_TOO_LARGE; + } +} diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java index d7026159d9ec0..682f8cced82a3 100644 --- a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java @@ -118,6 +118,7 @@ import org.opensearch.snapshots.SnapshotId; import org.opensearch.snapshots.SnapshotInProgressException; import org.opensearch.snapshots.SnapshotInUseDeletionException; +import org.opensearch.snapshots.TooManyShardsInSnapshotsStatusException; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; import org.opensearch.transport.ActionNotFoundTransportException; @@ -896,6 +897,7 @@ public void testIds() { ids.put(171, CryptoRegistryException.class); ids.put(172, ViewNotFoundException.class); ids.put(173, ViewAlreadyExistsException.class); + ids.put(174, TooManyShardsInSnapshotsStatusException.class); ids.put(10001, IndexCreateBlockException.class); Map, Integer> reverse = new HashMap<>();