From 9489a21a7466dbc6320b4b53c828ff3bfbce2b22 Mon Sep 17 00:00:00 2001 From: "Spencer G. Jones" Date: Tue, 22 Oct 2024 22:44:24 -0700 Subject: [PATCH 01/17] =?UTF-8?q?Add=20new=20parameters=20to=20snapshot=20?= =?UTF-8?q?restore=20to=20rename=20the=20restored=20aliases=E2=80=A6=20(#1?= =?UTF-8?q?6292)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add new parameters to snapshot restore to rename the restored aliases similar to the existing parameters to rename indexes Signed-off-by: Spencer G. Jones * Fix comment. Update changelog. Signed-off-by: Spencer G. Jones * New parameters needs to only used for new version Signed-off-by: Spencer G. Jones * Add missing equals and hash implemenation for new parameters Signed-off-by: Spencer G. Jones * Add some tests Signed-off-by: Spencer G. Jones * Add some more tests Signed-off-by: Spencer G. Jones * Use CountDownLatch Signed-off-by: Spencer G. Jones * Add two more tests. Refactoring and cleanup. Signed-off-by: Spencer G. Jones * Use CURRENT version to pass backward compatibility tests. Change to V2.18 later once it is backported into that version. Signed-off-by: Spencer G. Jones * Refactoring Signed-off-by: Spencer G. Jones * Overwriting aliases variable causes test failures for reasons I do not understand. Also some refactoring. Signed-off-by: Spencer G. Jones * Convert to paramaterized tests Signed-off-by: Spencer G. Jones --------- Signed-off-by: Spencer G. Jones Signed-off-by: Daniel Widdis Co-authored-by: Daniel Widdis --- CHANGELOG.md | 2 +- .../restore/RestoreSnapshotRequest.java | 83 +++++ .../RestoreSnapshotRequestBuilder.java | 28 ++ .../opensearch/snapshots/RestoreService.java | 30 +- .../restore/RestoreSnapshotRequestTests.java | 6 + .../snapshots/RestoreServiceIntegTests.java | 297 ++++++++++++++++++ .../snapshots/SnapshotRequestsTests.java | 4 + .../snapshots/SnapshotResiliencyTests.java | 70 ++++- 8 files changed, 506 insertions(+), 14 deletions(-) create mode 100644 server/src/test/java/org/opensearch/snapshots/RestoreServiceIntegTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f95cb2484984..332dad2a7370a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add _list/shards API as paginated alternate to _cat/shards ([#14641](https://github.com/opensearch-project/OpenSearch/pull/14641)) - Latency and Memory allocation improvements to Multi Term Aggregation queries ([#14993](https://github.com/opensearch-project/OpenSearch/pull/14993)) - Flat object field use IndexOrDocValuesQuery to optimize query ([#14383](https://github.com/opensearch-project/OpenSearch/issues/14383)) +- Add support for renaming aliases during snapshot restore ([#16292](https://github.com/opensearch-project/OpenSearch/pull/16292)) - Add method to return dynamic SecureTransportParameters from SecureTransportSettingsProvider interface ([#16387](https://github.com/opensearch-project/OpenSearch/pull/16387)) - URI path filtering support in cluster stats API ([#15938](https://github.com/opensearch-project/OpenSearch/pull/15938)) -- [Star Tree - Search] Add support for metric aggregations with/without term query ([15289](https://github.com/opensearch-project/OpenSearch/pull/15289)) ### Dependencies - Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https://github.com/opensearch-project/OpenSearch/pull/15578)) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index f3110cc8f20a5..42c64e04268e3 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -112,6 +112,8 @@ private static StorageType fromString(String string) { private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen(); private String renamePattern; private String renameReplacement; + private String renameAliasPattern; + private String renameAliasReplacement; private boolean waitForCompletion; private boolean includeGlobalState = false; private boolean partial = false; @@ -164,6 +166,13 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_17_0)) { sourceRemoteTranslogRepository = in.readOptionalString(); } + // TODO: change to V_2_18_0 once this is backported into that version + if (in.getVersion().onOrAfter(Version.CURRENT)) { + renameAliasPattern = in.readOptionalString(); + } + if (in.getVersion().onOrAfter(Version.CURRENT)) { + renameAliasReplacement = in.readOptionalString(); + } } @Override @@ -191,6 +200,13 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_17_0)) { out.writeOptionalString(sourceRemoteTranslogRepository); } + // TODO: change to V_2_18_0 once this is backported into that version + if (out.getVersion().onOrAfter(Version.CURRENT)) { + out.writeOptionalString(renameAliasPattern); + } + if (out.getVersion().onOrAfter(Version.CURRENT)) { + out.writeOptionalString(renameAliasReplacement); + } } @Override @@ -361,6 +377,51 @@ public String renameReplacement() { return renameReplacement; } + /** + * Sets rename pattern that should be applied to restored indices' alias. + *

+ * Alias that match the rename pattern will be renamed according to {@link #renameAliasReplacement(String)}. The + * rename pattern is applied according to the {@link java.util.regex.Matcher#appendReplacement(StringBuffer, String)} + * If two or more aliases are renamed into the same name, they will be merged. + * + * @param renameAliasPattern rename pattern + * @return this request + */ + public RestoreSnapshotRequest renameAliasPattern(String renameAliasPattern) { + this.renameAliasPattern = renameAliasPattern; + return this; + } + + /** + * Returns rename alias pattern + * + * @return rename alias pattern + */ + public String renameAliasPattern() { + return renameAliasPattern; + } + + /** + * Sets rename alias replacement + *

+ * See {@link #renameAliasPattern(String)} for more information. + * + * @param renameAliasReplacement rename replacement + */ + public RestoreSnapshotRequest renameAliasReplacement(String renameAliasReplacement) { + this.renameAliasReplacement = renameAliasReplacement; + return this; + } + + /** + * Returns rename alias replacement + * + * @return rename alias replacement + */ + public String renameAliasReplacement() { + return renameAliasReplacement; + } + /** * If this parameter is set to true the operation will wait for completion of restore process before returning. * @@ -625,6 +686,18 @@ public RestoreSnapshotRequest source(Map source) { } else { throw new IllegalArgumentException("malformed rename_replacement"); } + } else if (name.equals("rename_alias_pattern")) { + if (entry.getValue() instanceof String) { + renameAliasPattern((String) entry.getValue()); + } else { + throw new IllegalArgumentException("malformed rename_alias_pattern"); + } + } else if (name.equals("rename_alias_replacement")) { + if (entry.getValue() instanceof String) { + renameAliasReplacement((String) entry.getValue()); + } else { + throw new IllegalArgumentException("malformed rename_alias_replacement"); + } } else if (name.equals("index_settings")) { if (!(entry.getValue() instanceof Map)) { throw new IllegalArgumentException("malformed index_settings section"); @@ -685,6 +758,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (renameReplacement != null) { builder.field("rename_replacement", renameReplacement); } + if (renameAliasPattern != null) { + builder.field("rename_alias_pattern", renameAliasPattern); + } + if (renameAliasReplacement != null) { + builder.field("rename_alias_replacement", renameAliasReplacement); + } builder.field("include_global_state", includeGlobalState); builder.field("partial", partial); builder.field("include_aliases", includeAliases); @@ -733,6 +812,8 @@ public boolean equals(Object o) { && Objects.equals(indicesOptions, that.indicesOptions) && Objects.equals(renamePattern, that.renamePattern) && Objects.equals(renameReplacement, that.renameReplacement) + && Objects.equals(renameAliasPattern, that.renameAliasPattern) + && Objects.equals(renameAliasReplacement, that.renameAliasReplacement) && Objects.equals(indexSettings, that.indexSettings) && Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings) && Objects.equals(snapshotUuid, that.snapshotUuid) @@ -751,6 +832,8 @@ public int hashCode() { indicesOptions, renamePattern, renameReplacement, + renameAliasPattern, + renameAliasReplacement, waitForCompletion, includeGlobalState, partial, diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java index 53c9557a621b7..038d62ad7f4cb 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java @@ -144,6 +144,34 @@ public RestoreSnapshotRequestBuilder setRenameReplacement(String renameReplaceme return this; } + /** + * Sets rename pattern that should be applied to restored indices' aliases. + *

+ * Aliases that match the rename pattern will be renamed according to {@link #setRenameAliasReplacement(String)}. The + * rename pattern is applied according to the {@link java.util.regex.Matcher#appendReplacement(StringBuffer, String)} + * The request will fail if two or more alias will be renamed into the same name. + * + * @param renameAliasPattern rename alias pattern + * @return this builder + */ + public RestoreSnapshotRequestBuilder setRenameAliasPattern(String renameAliasPattern) { + request.renameAliasPattern(renameAliasPattern); + return this; + } + + /** + * Sets rename replacement + *

+ * See {@link #setRenameAliasPattern(String)} for more information. + * + * @param renameAliasReplacement rename alias replacement + * @return this builder + */ + public RestoreSnapshotRequestBuilder setRenameAliasReplacement(String renameAliasReplacement) { + request.renameAliasReplacement(renameAliasReplacement); + return this; + } + /** * If this parameter is set to true the operation will wait for completion of restore process before returning. * diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 79a70d835f773..88eff93e51b38 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -111,6 +111,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static java.util.Collections.unmodifiableSet; @@ -486,9 +487,7 @@ public ClusterState execute(ClusterState currentState) { // Remove all aliases - they shouldn't be restored indexMdBuilder.removeAllAliases(); } else { - for (final String alias : snapshotIndexMetadata.getAliases().keySet()) { - aliases.add(alias); - } + applyAliasesWithRename(snapshotIndexMetadata, indexMdBuilder, aliases); } IndexMetadata updatedIndexMetadata = indexMdBuilder.build(); if (partial) { @@ -533,9 +532,7 @@ public ClusterState execute(ClusterState currentState) { indexMdBuilder.putAlias(alias); } } else { - for (final String alias : snapshotIndexMetadata.getAliases().keySet()) { - aliases.add(alias); - } + applyAliasesWithRename(snapshotIndexMetadata, indexMdBuilder, aliases); } final Settings.Builder indexSettingsBuilder = Settings.builder() .put(snapshotIndexMetadata.getSettings()) @@ -665,6 +662,27 @@ private void checkAliasNameConflicts(Map renamedIndices, Set aliases + ) { + if (request.renameAliasPattern() == null || request.renameAliasReplacement() == null) { + aliases.addAll(snapshotIndexMetadata.getAliases().keySet()); + } else { + Pattern renameAliasPattern = Pattern.compile(request.renameAliasPattern()); + for (final Map.Entry alias : snapshotIndexMetadata.getAliases().entrySet()) { + String currentAliasName = alias.getKey(); + indexMdBuilder.removeAlias(currentAliasName); + String newAliasName = renameAliasPattern.matcher(currentAliasName) + .replaceAll(request.renameAliasReplacement()); + AliasMetadata newAlias = AliasMetadata.newAliasMetadata(alias.getValue(), newAliasName); + indexMdBuilder.putAlias(newAlias); + aliases.add(newAliasName); + } + } + } + private String[] getIgnoreSettingsInternal() { // for non-remote store enabled domain, we will remove all the remote store // related index settings present in the snapshot. diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java index c3de3413edd13..04cc45f3477c6 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java @@ -71,6 +71,12 @@ private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) { if (randomBoolean()) { instance.renameReplacement(randomUnicodeOfLengthBetween(1, 100)); } + if (randomBoolean()) { + instance.renameAliasPattern(randomUnicodeOfLengthBetween(1, 100)); + } + if (randomBoolean()) { + instance.renameAliasReplacement(randomUnicodeOfLengthBetween(1, 100)); + } instance.partial(randomBoolean()); instance.includeAliases(randomBoolean()); diff --git a/server/src/test/java/org/opensearch/snapshots/RestoreServiceIntegTests.java b/server/src/test/java/org/opensearch/snapshots/RestoreServiceIntegTests.java new file mode 100644 index 0000000000000..92da980d70f34 --- /dev/null +++ b/server/src/test/java/org/opensearch/snapshots/RestoreServiceIntegTests.java @@ -0,0 +1,297 @@ +/* + * 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. + */ + +package org.opensearch.snapshots; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; + +import org.opensearch.action.StepListener; +import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; +import org.opensearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest; +import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest; +import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; +import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; +import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; +import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.opensearch.action.admin.indices.close.CloseIndexRequest; +import org.opensearch.action.admin.indices.close.CloseIndexResponse; +import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; +import org.opensearch.action.admin.indices.exists.indices.IndicesExistsRequest; +import org.opensearch.action.admin.indices.exists.indices.IndicesExistsResponse; +import org.opensearch.action.admin.indices.open.OpenIndexRequest; +import org.opensearch.action.admin.indices.open.OpenIndexResponse; +import org.opensearch.action.bulk.BulkRequest; +import org.opensearch.action.bulk.BulkResponse; +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.action.support.WriteRequest; +import org.opensearch.action.support.master.AcknowledgedResponse; +import org.opensearch.common.CheckedConsumer; +import org.opensearch.common.settings.Settings; +import org.opensearch.repositories.fs.FsRepository; +import org.opensearch.search.builder.SearchSourceBuilder; +import org.opensearch.test.OpenSearchIntegTestCase; +import org.opensearch.test.OpenSearchSingleNodeTestCase; +import org.junit.After; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +public class RestoreServiceIntegTests extends OpenSearchSingleNodeTestCase { + private final String indexName = "index_1"; + private final String renamedIndexName = "index_2"; + private final String aliasName = "alias_1"; + private final String renamedAliasName = "alias_2"; + private final String repoName = "repo_1"; + private final String snapShotName = "snap_1"; + private final int waitInSeconds = 60; + private boolean exists; + private boolean closed; + private boolean includeAlias; + private boolean renameAliases; + private boolean renameIndexes; + + public RestoreServiceIntegTests(TestCase testCase) { + this.exists = testCase.exists; + this.closed = testCase.closed; + this.includeAlias = testCase.includeAlias; + this.renameAliases = testCase.renameAliases; + this.renameIndexes = testCase.renameIndexes; + } + + public static class TestCase { + public boolean exists; + public boolean closed; + public boolean includeAlias; + public boolean renameAliases; + public boolean renameIndexes; + + public TestCase(boolean exists, boolean closed, boolean includeAlias, boolean renameAliases, boolean renameIndexes) { + this.exists = exists; + this.closed = closed; + this.includeAlias = includeAlias; + this.renameAliases = renameAliases; + this.renameIndexes = renameIndexes; + } + + public String toString() { + return String.join( + " and ", + new String[] { + exists ? "target index exists and is" + (closed ? "closed" : "open") : "doesn't exist", + includeAlias ? "including aliases" : "not including aliases", + renameIndexes ? "renaming indexes" : "not renaming indexes", + renameAliases ? "renaming aliases" : "not renaming aliases" } + ); + } + } + + @ParametersFactory + public static Collection parameters() { + return Arrays.asList( + new Object[] { new TestCase(false, false, true, true, true) }, + new Object[] { new TestCase(false, false, false, true, true) }, + new Object[] { new TestCase(false, false, true, false, false) }, + new Object[] { new TestCase(false, false, false, false, false) }, + new Object[] { new TestCase(true, false, true, true, true) }, + new Object[] { new TestCase(true, false, false, true, true) }, + new Object[] { new TestCase(true, true, true, true, true) }, + new Object[] { new TestCase(true, true, false, true, true) }, + new Object[] { new TestCase(true, false, false, false, false) }, + new Object[] { new TestCase(true, false, true, false, false) }, + new Object[] { new TestCase(true, true, false, false, false) }, + new Object[] { new TestCase(true, true, true, false, false) } + ); + } + + @After + public void cleanup() throws InterruptedException { + final CountDownLatch allDeleted = new CountDownLatch(3); + for (String indexName : new String[] { indexName, renamedIndexName }) { + final StepListener existsIndexResponseStepListener = new StepListener<>(); + client().admin().indices().exists(new IndicesExistsRequest(indexName), existsIndexResponseStepListener); + continueOrDie(existsIndexResponseStepListener, resp -> { + if (resp.isExists()) { + final StepListener deleteIndexResponseStepListener = new StepListener<>(); + client().admin().indices().delete(new DeleteIndexRequest(indexName), deleteIndexResponseStepListener); + continueOrDie(deleteIndexResponseStepListener, ignored -> allDeleted.countDown()); + } else { + allDeleted.countDown(); + } + }); + } + + final StepListener snapStatusResponseStepListener = new StepListener<>(); + client().admin().cluster().getSnapshots(new GetSnapshotsRequest(repoName), snapStatusResponseStepListener); + continueOrDie(snapStatusResponseStepListener, resp -> { + if (resp.getSnapshots().stream().anyMatch(s -> s.snapshotId().getName().equals(snapShotName))) { + final StepListener deleteSnapResponseStepListener = new StepListener<>(); + client().admin() + .cluster() + .deleteSnapshot(new DeleteSnapshotRequest(repoName, snapShotName), deleteSnapResponseStepListener); + continueOrDie(deleteSnapResponseStepListener, ignored -> allDeleted.countDown()); + } else { + allDeleted.countDown(); + } + }); + + allDeleted.await(waitInSeconds, TimeUnit.SECONDS); + } + + public void testRestoreWithRename() throws Exception { + + assert this.exists || !this.closed; // index close state doesn't exist when the index doesn't exist - so only permit one value of + // closed to avoid pointless duplicate tests + final boolean expectSuccess = !this.exists || this.closed; + final int documents = randomIntBetween(1, 100); + + this.createIndex(indexName); + if (this.exists && this.renameIndexes) { + this.createIndex(renamedIndexName); + } + + final StepListener putRepositoryResponseStepListener = new StepListener<>(); + Settings.Builder settings = Settings.builder().put("location", randomAlphaOfLength(10)); + OpenSearchIntegTestCase.putRepository( + client().admin().cluster(), + repoName, + FsRepository.TYPE, + settings, + putRepositoryResponseStepListener + ); + + final StepListener createAliasResponseStepListener = new StepListener<>(); + client().admin() + .indices() + .aliases( + new IndicesAliasesRequest().addAliasAction(IndicesAliasesRequest.AliasActions.add().alias(aliasName).index(indexName)), + createAliasResponseStepListener + ); + + final CountDownLatch isDocumentFinished = new CountDownLatch(1); + continueOrDie(createAliasResponseStepListener, ignored -> { + final BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + for (int i = 0; i < documents; ++i) { + bulkRequest.add(new IndexRequest(indexName).source(Collections.singletonMap("foo", "bar" + i))); + } + final StepListener bulkResponseStepListener = new StepListener<>(); + client().bulk(bulkRequest, bulkResponseStepListener); + continueOrDie(bulkResponseStepListener, bulkResponse -> { + assertFalse("Failures in bulk response: " + bulkResponse.buildFailureMessage(), bulkResponse.hasFailures()); + assertEquals(documents, bulkResponse.getItems().length); + isDocumentFinished.countDown(); + }); + }); + + isDocumentFinished.await(waitInSeconds, TimeUnit.SECONDS); + + if (this.closed) { + final CountDownLatch isClosed = new CountDownLatch(1); + final StepListener closeIndexResponseStepListener = new StepListener<>(); + final String indexToClose = this.renameIndexes ? renamedIndexName : indexName; + client().admin().indices().close(new CloseIndexRequest(indexToClose), closeIndexResponseStepListener); + + continueOrDie(closeIndexResponseStepListener, ignored -> { isClosed.countDown(); }); + isClosed.await(waitInSeconds, TimeUnit.SECONDS); + } + + final StepListener createSnapshotResponseStepListener = new StepListener<>(); + continueOrDie(putRepositoryResponseStepListener, ignored -> { + client().admin() + .cluster() + .prepareCreateSnapshot(repoName, snapShotName) + .setWaitForCompletion(true) + .setPartial(true) + .execute(createSnapshotResponseStepListener); + }); + + final CountDownLatch isRestorable = new CountDownLatch(1); + + if (!this.exists && !this.renameIndexes) { + final StepListener deleteIndexResponseStepListener = new StepListener<>(); + continueOrDie(createSnapshotResponseStepListener, ignored -> { + client().admin().indices().delete(new DeleteIndexRequest(indexName), deleteIndexResponseStepListener); + }); + continueOrDie(deleteIndexResponseStepListener, ignored -> isRestorable.countDown()); + } else { + continueOrDie(createSnapshotResponseStepListener, ignored -> isRestorable.countDown()); + } + + isRestorable.await(waitInSeconds, TimeUnit.SECONDS); + + final StepListener restoreSnapshotResponseStepListener = new StepListener<>(); + final CountDownLatch isRestored = new CountDownLatch(1); + RestoreSnapshotRequest restoreSnapshotRequest = new RestoreSnapshotRequest(repoName, snapShotName).includeAliases(this.includeAlias) + .waitForCompletion(true); + if (this.renameAliases) { + restoreSnapshotRequest = restoreSnapshotRequest.renameAliasPattern("1").renameAliasReplacement("2"); + } + if (this.renameIndexes) { + restoreSnapshotRequest = restoreSnapshotRequest.renamePattern("1").renameReplacement("2"); + } + client().admin().cluster().restoreSnapshot(restoreSnapshotRequest, restoreSnapshotResponseStepListener); + + restoreSnapshotResponseStepListener.whenComplete(ignored -> { + isRestored.countDown(); + assertTrue("unexpected sucesssful restore", expectSuccess); + }, e -> { + isRestored.countDown(); + if (expectSuccess) { + throw new RuntimeException(e); + } + }); + + isRestored.await(waitInSeconds, TimeUnit.SECONDS); + + if (expectSuccess) { + final String indexToSearch = this.renameIndexes ? renamedIndexName : indexName; + final String aliasToSearch = this.renameAliases ? renamedAliasName : aliasName; + + if (this.closed) { + final CountDownLatch isOpened = new CountDownLatch(1); + final StepListener openIndexResponseStepListener = new StepListener<>(); + client().admin().indices().open(new OpenIndexRequest(indexToSearch).waitForActiveShards(1), openIndexResponseStepListener); + continueOrDie(openIndexResponseStepListener, ignored -> { isOpened.countDown(); }); + + isOpened.await(waitInSeconds, TimeUnit.SECONDS); + } + + final CountDownLatch isSearchDone = new CountDownLatch(this.includeAlias ? 2 : 1); + final StepListener searchIndexResponseListener = new StepListener<>(); + final StepListener searchAliasResponseListener = new StepListener<>(); + client().search( + new SearchRequest(indexToSearch).source(new SearchSourceBuilder().size(0).trackTotalHits(true)), + searchIndexResponseListener + ); + continueOrDie(searchIndexResponseListener, ignored -> { isSearchDone.countDown(); }); + if (this.includeAlias) { + client().search( + new SearchRequest(aliasToSearch).source(new SearchSourceBuilder().size(0).trackTotalHits(true)), + searchAliasResponseListener + ); + continueOrDie(searchAliasResponseListener, ignored -> { isSearchDone.countDown(); }); + } + + isSearchDone.await(waitInSeconds, TimeUnit.SECONDS); + + assertEquals(documents, Objects.requireNonNull(searchIndexResponseListener.result().getHits().getTotalHits()).value); + if (this.includeAlias) { + assertEquals(documents, Objects.requireNonNull(searchAliasResponseListener.result().getHits().getTotalHits()).value); + } + } + } + + private static void continueOrDie(StepListener listener, CheckedConsumer onResponse) { + listener.whenComplete(onResponse, e -> { throw new AssertionError(e); }); + } +} diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotRequestsTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotRequestsTests.java index a00c74f669eac..87ab95fef6a53 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotRequestsTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotRequestsTests.java @@ -77,6 +77,8 @@ public void testRestoreSnapshotRequestParsing() throws IOException { builder.field("allow_no_indices", indicesOptions.allowNoIndices()); builder.field("rename_pattern", "rename-from"); builder.field("rename_replacement", "rename-to"); + builder.field("rename_alias_pattern", "alias-rename-from"); + builder.field("rename_alias_replacement", "alias-rename-to"); boolean partial = randomBoolean(); builder.field("partial", partial); builder.startObject("settings").field("set1", "val1").endObject(); @@ -103,6 +105,8 @@ public void testRestoreSnapshotRequestParsing() throws IOException { assertArrayEquals(request.indices(), new String[] { "foo", "bar", "baz" }); assertEquals("rename-from", request.renamePattern()); assertEquals("rename-to", request.renameReplacement()); + assertEquals("alias-rename-from", request.renameAliasPattern()); + assertEquals("alias-rename-to", request.renameAliasReplacement()); assertEquals(partial, request.partial()); assertArrayEquals(request.ignoreIndexSettings(), new String[] { "set2", "set3" }); boolean expectedIgnoreAvailable = includeIgnoreUnavailable diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index d17e661615b0d..d21282ff0441f 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -66,6 +66,9 @@ import org.opensearch.action.admin.cluster.state.ClusterStateRequest; import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.cluster.state.TransportClusterStateAction; +import org.opensearch.action.admin.indices.alias.IndicesAliasesAction; +import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.opensearch.action.admin.indices.alias.TransportIndicesAliasesAction; import org.opensearch.action.admin.indices.create.CreateIndexAction; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; @@ -141,6 +144,7 @@ import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.MetadataCreateIndexService; import org.opensearch.cluster.metadata.MetadataDeleteIndexService; +import org.opensearch.cluster.metadata.MetadataIndexAliasesService; import org.opensearch.cluster.metadata.MetadataIndexUpgradeService; import org.opensearch.cluster.metadata.MetadataMappingService; import org.opensearch.cluster.node.DiscoveryNode; @@ -958,6 +962,7 @@ public void testConcurrentSnapshotRestoreAndDeleteOther() { String repoName = "repo"; String snapshotName = "snapshot"; final String index = "test"; + final String alias = "test_alias"; final int shards = randomIntBetween(1, 10); TestClusterNodes.TestClusterNode clusterManagerNode = testClusterNodes.currentClusterManager( @@ -967,9 +972,8 @@ public void testConcurrentSnapshotRestoreAndDeleteOther() { final StepListener createSnapshotResponseStepListener = new StepListener<>(); final int documentsFirstSnapshot = randomIntBetween(0, 100); - continueOrDie( - createRepoAndIndex(repoName, index, shards), + createRepoAndIndexAndAlias(repoName, index, shards, alias), createIndexResponse -> indexNDocuments( documentsFirstSnapshot, index, @@ -1009,19 +1013,27 @@ public void testConcurrentSnapshotRestoreAndDeleteOther() { .cluster() .restoreSnapshot( new RestoreSnapshotRequest(repoName, secondSnapshotName).waitForCompletion(true) + .includeAliases(true) .renamePattern("(.+)") - .renameReplacement("restored_$1"), + .renameReplacement("restored_$1") + .renameAliasPattern("(.+)") + .renameAliasReplacement("restored_alias_$1"), restoreSnapshotResponseListener ) ); }); - final StepListener searchResponseListener = new StepListener<>(); + final StepListener searchIndexResponseListener = new StepListener<>(); + final StepListener searchAliasResponseListener = new StepListener<>(); continueOrDie(restoreSnapshotResponseListener, restoreSnapshotResponse -> { assertEquals(shards, restoreSnapshotResponse.getRestoreInfo().totalShards()); client().search( new SearchRequest("restored_" + index).source(new SearchSourceBuilder().size(0).trackTotalHits(true)), - searchResponseListener + searchIndexResponseListener + ); + client().search( + new SearchRequest("restored_alias_" + alias).source(new SearchSourceBuilder().size(0).trackTotalHits(true)), + searchAliasResponseListener ); }); @@ -1029,7 +1041,11 @@ public void testConcurrentSnapshotRestoreAndDeleteOther() { assertEquals( documentsFirstSnapshot + documentsSecondSnapshot, - Objects.requireNonNull(searchResponseListener.result().getHits().getTotalHits()).value + Objects.requireNonNull(searchIndexResponseListener.result().getHits().getTotalHits()).value + ); + assertEquals( + documentsFirstSnapshot + documentsSecondSnapshot, + Objects.requireNonNull(searchAliasResponseListener.result().getHits().getTotalHits()).value ); assertThat(deleteSnapshotStepListener.result().isAcknowledged(), is(true)); assertThat(restoreSnapshotResponseListener.result().getRestoreInfo().failedShards(), is(0)); @@ -1520,6 +1536,22 @@ private StepListener createRepoAndIndex(String repoName, St return createIndexResponseStepListener; } + private StepListener createRepoAndIndexAndAlias(String repoName, String index, int shards, String alias) { + final StepListener createAliasListener = new StepListener<>(); + + continueOrDie( + createRepoAndIndex(repoName, index, shards), + acknowledgedResponse -> client().admin() + .indices() + .aliases( + new IndicesAliasesRequest().addAliasAction(IndicesAliasesRequest.AliasActions.add().index(index).alias(alias)), + createAliasListener + ) + ); + + return createAliasListener; + } + private void clearDisruptionsAndAwaitSync() { testClusterNodes.clearNetworkDisruptions(); stabilize(); @@ -2171,6 +2203,30 @@ public void onFailure(final Exception e) { indexNameExpressionResolver ) ); + final MetadataDeleteIndexService metadataDeleteIndexService = new MetadataDeleteIndexService( + settings, + clusterService, + allocationService + ); + final MetadataIndexAliasesService metadataIndexAliasesService = new MetadataIndexAliasesService( + clusterService, + indicesService, + new AliasValidator(), + metadataDeleteIndexService, + namedXContentRegistry + ); + actions.put( + IndicesAliasesAction.INSTANCE, + new TransportIndicesAliasesAction( + transportService, + clusterService, + threadPool, + metadataIndexAliasesService, + actionFilters, + indexNameExpressionResolver, + new RequestValidators<>(Collections.emptyList()) + ) + ); final MappingUpdatedAction mappingUpdatedAction = new MappingUpdatedAction(settings, clusterSettings, clusterService); mappingUpdatedAction.setClient(client); final TransportShardBulkAction transportShardBulkAction = new TransportShardBulkAction( @@ -2337,7 +2393,7 @@ public void onFailure(final Exception e) { transportService, clusterService, threadPool, - new MetadataDeleteIndexService(settings, clusterService, allocationService), + metadataDeleteIndexService, actionFilters, indexNameExpressionResolver, new DestructiveOperations(settings, clusterSettings) From bb1359f224158e94523ad1a2ecf5429244a8144f Mon Sep 17 00:00:00 2001 From: Gaurav Bafna <85113518+gbbafna@users.noreply.github.com> Date: Wed, 23 Oct 2024 14:09:10 +0530 Subject: [PATCH 02/17] Disallow snapshot deletion while a v2 snapshot is in progress (#16430) --------- Signed-off-by: Gaurav Bafna --- .../snapshots/ConcurrentSnapshotsV2IT.java | 5 ++--- .../snapshots/SnapshotsService.java | 21 +++++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsV2IT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsV2IT.java index 78497cac41d46..ab5b22c69b517 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsV2IT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsV2IT.java @@ -529,14 +529,13 @@ public void testDeleteWhileV2CreateOngoing() throws Exception { awaitNumberOfSnapshotsInProgress(1); ActionFuture a = startDeleteSnapshot(repoName, "snapshot-v1"); + expectThrows(ConcurrentSnapshotExecutionException.class, a::actionGet); unblockNode(repoName, clusterManagerName); CreateSnapshotResponse csr = snapshotFuture.actionGet(); assertTrue(csr.getSnapshotInfo().getPinnedTimestamp() != 0); - assertTrue(a.actionGet().isAcknowledged()); List snapInfo = client().admin().cluster().prepareGetSnapshots(repoName).get().getSnapshots(); - assertEquals(1, snapInfo.size()); - assertThat(snapInfo, contains(csr.getSnapshotInfo())); + assertEquals(2, snapInfo.size()); } @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/16205") diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java index ff1395c600ac0..0972f5dad0fa2 100644 --- a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java @@ -619,10 +619,8 @@ public void onResponse(RepositoryData repositoryData) { } cleanOrphanTimestamp(repositoryName, repositoryData); logger.info("created snapshot-v2 [{}] in repository [{}]", repositoryName, snapshotName); + leaveRepoLoop(repositoryName); listener.onResponse(snapshotInfo); - // For snapshot-v2, we don't allow concurrent snapshots . But meanwhile non-v2 snapshot operations - // can get queued . This is triggering them. - runNextQueuedOperation(repositoryData, repositoryName, true); } @Override @@ -1021,10 +1019,8 @@ public void onResponse(RepositoryData repositoryData) { return; } logger.info("snapshot-v2 clone [{}] completed successfully", snapshot); + leaveRepoLoop(repositoryName); listener.onResponse(null); - // For snapshot-v2, we don't allow concurrent snapshots . But meanwhile non-v2 snapshot operations - // can get queued . This is triggering them. - runNextQueuedOperation(repositoryData, repositoryName, true); } @Override @@ -2564,6 +2560,19 @@ public void deleteSnapshots(final DeleteSnapshotRequest request, final ActionLis public ClusterState execute(ClusterState currentState) throws Exception { final SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE, SnapshotsInProgress.EMPTY); final List snapshotEntries = findInProgressSnapshots(snapshots, snapshotNames, repoName); + boolean isSnapshotV2 = SHALLOW_SNAPSHOT_V2.get(repository.getMetadata().settings()); + boolean remoteStoreIndexShallowCopy = remoteStoreShallowCopyEnabled(repository); + List entriesForThisRepo = snapshots.entries() + .stream() + .filter(entry -> Objects.equals(entry.repository(), repoName)) + .collect(Collectors.toList()); + if (isSnapshotV2 && remoteStoreIndexShallowCopy && entriesForThisRepo.isEmpty() == false) { + throw new ConcurrentSnapshotExecutionException( + repoName, + String.join(",", snapshotNames), + "cannot delete snapshots in v2 repo while a snapshot is in progress" + ); + } final List snapshotIds = matchingSnapshotIds( snapshotEntries.stream().map(e -> e.snapshot().getSnapshotId()).collect(Collectors.toList()), repositoryData, From 15607b1ec44a8428f66c5d26c46b35fe1fc1bc50 Mon Sep 17 00:00:00 2001 From: SwethaGuptha <156877431+SwethaGuptha@users.noreply.github.com> Date: Wed, 23 Oct 2024 17:15:33 +0530 Subject: [PATCH 03/17] Downgrade version to 2.18.0 for ser/de of new ClusterStatsRequest metric params. (#16441) Signed-off-by: Swetha Guptha Co-authored-by: Swetha Guptha --- .../test/java/org/opensearch/upgrades/ClusterStatsIT.java | 4 ++-- .../action/admin/cluster/stats/ClusterStatsRequest.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/ClusterStatsIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/ClusterStatsIT.java index 1c5f35db8ec46..004bbd56bc526 100644 --- a/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/ClusterStatsIT.java +++ b/qa/rolling-upgrade/src/test/java/org/opensearch/upgrades/ClusterStatsIT.java @@ -26,8 +26,8 @@ public class ClusterStatsIT extends AbstractRollingTestCase { public void testClusterStats() throws IOException { Response response = client().performRequest(new Request("GET", "/_cluster/stats")); validateClusterStatsWithFilterResponse(response, nodeStatsMetrics, indicesStatsMetrics); - if (AbstractRollingTestCase.UPGRADE_FROM_VERSION.onOrAfter(Version.V_3_0_0) || ( - CLUSTER_TYPE == ClusterType.UPGRADED && Version.CURRENT.onOrAfter(Version.V_3_0_0))) { + if (AbstractRollingTestCase.UPGRADE_FROM_VERSION.onOrAfter(Version.V_2_18_0) || ( + CLUSTER_TYPE == ClusterType.UPGRADED && Version.CURRENT.onOrAfter(Version.V_2_18_0))) { response = client().performRequest(new Request("GET", "/_cluster/stats/os/nodes/_all")); validateClusterStatsWithFilterResponse(response, List.of("os"), Collections.emptyList()); response = client().performRequest(new Request("GET", "/_cluster/stats/indices/mappings/nodes/_all")); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java index 1c929881b898b..19b3033c9745b 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java @@ -59,7 +59,7 @@ public ClusterStatsRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_16_0)) { useAggregatedNodeLevelResponses = in.readOptionalBoolean(); } - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + if (in.getVersion().onOrAfter(Version.V_2_18_0)) { computeAllMetrics = in.readOptionalBoolean(); final long longMetricsFlags = in.readLong(); for (Metric metric : Metric.values()) { @@ -135,7 +135,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_16_0)) { out.writeOptionalBoolean(useAggregatedNodeLevelResponses); } - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + if (out.getVersion().onOrAfter(Version.V_2_18_0)) { out.writeOptionalBoolean(computeAllMetrics); long longMetricFlags = 0; for (Metric metric : requestedMetrics) { @@ -154,7 +154,7 @@ public void writeTo(StreamOutput out) throws IOException { * An enumeration of the "core" sections of metrics that may be requested * from the cluster stats endpoint. */ - @PublicApi(since = "3.0.0") + @PublicApi(since = "2.18.0") public enum Metric { OS("os", 0), JVM("jvm", 1), @@ -192,7 +192,7 @@ public int getIndex() { * * When no value is provided for param index_metric, default filter is set to _all. */ - @PublicApi(since = "3.0.0") + @PublicApi(since = "2.18.0") public enum IndexMetric { // Metrics computed from ShardStats SHARDS("shards", 0), From 5941a7e69dee8901b46f530551611a91b7de48c2 Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Wed, 23 Oct 2024 20:41:34 +0800 Subject: [PATCH 04/17] Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation (#16294) * Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation Signed-off-by: Gao Binlong * Update skip version in rest yaml test file Signed-off-by: Gao Binlong * Fix test failure Signed-off-by: Gao Binlong --------- Signed-off-by: Gao Binlong --- CHANGELOG.md | 1 + .../40_number_of_routing_shards.yml | 41 +++++++++++++++++++ .../cluster/metadata/IndexMetadata.java | 3 +- .../metadata/MetadataCreateIndexService.java | 7 +--- .../MetadataCreateIndexServiceTests.java | 37 +++++++++++++++++ 5 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 332dad2a7370a..eb50efffcd52e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Streaming bulk request hangs ([#16158](https://github.com/opensearch-project/OpenSearch/pull/16158)) - Fix warnings from SLF4J on startup when repository-s3 is installed ([#16194](https://github.com/opensearch-project/OpenSearch/pull/16194)) - Fix protobuf-java leak through client library dependencies ([#16254](https://github.com/opensearch-project/OpenSearch/pull/16254)) +- Fix get index settings API doesn't show `number_of_routing_shards` setting when it was explicitly set ([#16294](https://github.com/opensearch-project/OpenSearch/pull/16294)) - Fix multi-search with template doesn't return status code ([#16265](https://github.com/opensearch-project/OpenSearch/pull/16265)) - [Streaming Indexing] Fix intermittent 'The bulk request must be terminated by a newline [\n]' failures [#16337](https://github.com/opensearch-project/OpenSearch/pull/16337)) - Fix wrong default value when setting `index.number_of_routing_shards` to null on index creation ([#16331](https://github.com/opensearch-project/OpenSearch/pull/16331)) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml new file mode 100644 index 0000000000000..3fb392d6db134 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml @@ -0,0 +1,41 @@ +--- +setup: + - do: + indices.create: + body: + settings: + index: + number_of_routing_shards: 4 + number_of_shards: 2 + number_of_replicas: 1 + index: test-index + + - do: + indices.create: + body: + settings: + index: + number_of_shards: 2 + number_of_replicas: 1 + index: test-index1 + +--- +Test retrieval of number_routing_shards settings: + - skip: + version: " - 2.99.99" + reason: "introduced in 3.0.0" # TODO: change it to 2.18.0 after backport to 2.x branch + - do: + indices.get_settings: + flat_settings: true + index: test-index + # show `index.number_of_routing_shards` if it was explicitly set when creating + - match: + test-index.settings.index\.number_of_routing_shards: "4" + + - do: + indices.get_settings: + flat_settings: true + index: test-index1 + # do not show `index.number_of_routing_shards` if it was not explicitly set when creating + - match: + test-index1.settings.index\.number_of_routing_shards: null diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index 713f8c9fc332c..c8ea5442a0dd0 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -300,7 +300,8 @@ public Iterator> settings() { } }, - Property.IndexScope + Property.IndexScope, + Property.NotCopyableOnResize ); /** diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index abda5dad25e4e..11df35527eea7 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -626,14 +626,9 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata( final boolean isHiddenAfterTemplates = IndexMetadata.INDEX_HIDDEN_SETTING.get(aggregatedIndexSettings); final boolean isSystem = validateDotIndex(request.index(), isHiddenAfterTemplates); - // remove the setting it's temporary and is only relevant once we create the index - final Settings.Builder settingsBuilder = Settings.builder().put(aggregatedIndexSettings); - settingsBuilder.remove(IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey()); - final Settings indexSettings = settingsBuilder.build(); - final IndexMetadata.Builder tmpImdBuilder = IndexMetadata.builder(request.index()); tmpImdBuilder.setRoutingNumShards(routingNumShards); - tmpImdBuilder.settings(indexSettings); + tmpImdBuilder.settings(aggregatedIndexSettings); tmpImdBuilder.system(isSystem); addRemoteStoreCustomMetadata(tmpImdBuilder, true); diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 1fdd038053eb6..0bb9ec28a1efc 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -136,6 +136,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singleton; import static java.util.Collections.singletonList; +import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING; import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING; import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING; import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK; @@ -1821,6 +1822,42 @@ private void validateRemoteCustomData(Map customData, String exp assertEquals(expectedValue, customData.get(expectedKey)); } + public void testNumberOfRoutingShardsShowsInIndexSettings() { + withTemporaryClusterService(((clusterService, threadPool) -> { + MetadataCreateIndexService checkerService = new MetadataCreateIndexService( + Settings.EMPTY, + clusterService, + indicesServices, + null, + null, + createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService), + null, + null, + threadPool, + null, + new SystemIndices(Collections.emptyMap()), + false, + new AwarenessReplicaBalance(Settings.EMPTY, clusterService.getClusterSettings()), + DefaultRemoteStoreSettings.INSTANCE, + repositoriesServiceSupplier + ); + final int routingNumberOfShards = 4; + Settings indexSettings = Settings.builder() + .put("index.version.created", Version.CURRENT) + .put(INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 2) + .put(INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0) + .put(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey(), routingNumberOfShards) + .build(); + CreateIndexClusterStateUpdateRequest request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); + IndexMetadata indexMetadata = checkerService.buildAndValidateTemporaryIndexMetadata( + indexSettings, + request, + routingNumberOfShards + ); + assertEquals(INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.get(indexMetadata.getSettings()).intValue(), routingNumberOfShards); + })); + } + public void testGetIndexNumberOfRoutingShardsWithNullSourceIndex() { Settings indexSettings = Settings.builder() .put("index.version.created", Version.CURRENT) From 9a476b667244ca08530fc207d67af0add4821c43 Mon Sep 17 00:00:00 2001 From: gargharsh3134 <51459091+gargharsh3134@users.noreply.github.com> Date: Wed, 23 Oct 2024 19:50:11 +0530 Subject: [PATCH 05/17] Avoid making further transport calls if paginationStrategy outputs empty entities (#16444) Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsAction.java | 10 ++++++++ .../indices/stats/IndicesStatsResponse.java | 5 ++++ .../rest/action/cat/RestIndicesAction.java | 24 +++++++++++++------ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 3dc8c38152a16..7b36b7a10f4f2 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -108,6 +108,12 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { : paginationStrategy.getRequestedEntities() ); catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken()); + // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats. + if (shouldSkipIndicesStatsRequest(paginationStrategy)) { + catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse()); + cancellableListener.onResponse(catShardsResponse); + return; + } IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); indicesStatsRequest.setShouldCancelOnTimeout(true); indicesStatsRequest.all(); @@ -159,4 +165,8 @@ private void validateRequestLimit( } } } + + private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy) { + return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty(); + } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java index ae989573b39ea..a5409e076730d 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsResponse.java @@ -45,6 +45,7 @@ import org.opensearch.core.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -230,4 +231,8 @@ static final class Fields { public String toString() { return Strings.toString(MediaTypeRegistry.JSON, this, true, false); } + + public static IndicesStatsResponse getEmptyResponse() { + return new IndicesStatsResponse(new ShardStats[0], 0, 0, 0, Collections.emptyList()); + } } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java index b566ba9bbb8e4..ea73c474e90b8 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java @@ -212,13 +212,19 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { groupedListener.onResponse(getSettingsResponse); groupedListener.onResponse(clusterStateResponse); - sendIndicesStatsRequest( - indicesToBeQueried, - subRequestIndicesOptions, - includeUnloadedSegments, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); + // For paginated queries, if strategy outputs no indices to be returned, + // avoid fetching indices stats. + if (shouldSkipIndicesStatsRequest(paginationStrategy)) { + groupedListener.onResponse(IndicesStatsResponse.getEmptyResponse()); + } else { + sendIndicesStatsRequest( + indicesToBeQueried, + subRequestIndicesOptions, + includeUnloadedSegments, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + } sendClusterHealthRequest( indicesToBeQueried, @@ -1093,4 +1099,8 @@ public Tuple next() { }; } + private boolean shouldSkipIndicesStatsRequest(IndexPaginationStrategy paginationStrategy) { + return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty(); + } + } From 8eccbb5f78a7ffa18bedf3126134d6d877c42e17 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 23 Oct 2024 10:57:54 -0400 Subject: [PATCH 06/17] Add log message if SSL dual mode is enabled (#16437) * Add log message about dual mode enabled Signed-off-by: Craig Perkins * Add log message about dual mode enabled Signed-off-by: Craig Perkins --------- Signed-off-by: Craig Perkins --- .../opensearch/transport/netty4/ssl/SecureNetty4Transport.java | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java b/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java index e51ed5663502f..90a9194d3cfd7 100644 --- a/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java @@ -146,6 +146,7 @@ protected void initChannel(Channel ch) throws Exception { .map(SecureTransportSettingsProvider.SecureTransportParameters::dualModeEnabled) .orElse(false); if (dualModeEnabled) { + logger.info("SSL Dual mode enabled, using port unification handler"); final ChannelHandler portUnificationHandler = new DualModeSslHandler( settings, secureTransportSettingsProvider, From 66f01107d74d9c19d8646d1cf94cc79fe2350967 Mon Sep 17 00:00:00 2001 From: inpink <108166692+inpink@users.noreply.github.com> Date: Thu, 24 Oct 2024 00:54:06 +0900 Subject: [PATCH 07/17] Fix flaky test in `testApproximateRangeWithSizeOverDefault` by adjusting totalHits assertion logic (#15807) (#16434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated the test to account for Lucene's behavior where `IndexSearcher.search()` may return `GREATER_THAN_OR_EQUAL_TO` for totalHits when the number of matches exceeds 1000. - Added logic to check if `totalHits.relation` is `EQUAL_TO`. If so, assert that the count is exactly 11000. Otherwise, ensure the count is at least 11000 and within the allowed upper limit (`maxHits`). - This change prevents intermittent test failures caused by Lucene’s performance optimizations. Signed-off-by: inpink --- CHANGELOG.md | 1 + .../ApproximatePointRangeQueryTests.java | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb50efffcd52e..5062ffd830f41 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/), - Fix missing fields in task index mapping to ensure proper task result storage ([#16201](https://github.com/opensearch-project/OpenSearch/pull/16201)) - Fix typo super->sb in method toString() of RemoteStoreNodeAttribute ([#15362](https://github.com/opensearch-project/OpenSearch/pull/15362)) - [Workload Management] Fixing Create/Update QueryGroup TransportActions to execute from non-cluster manager nodes ([16422](https://github.com/opensearch-project/OpenSearch/pull/16422)) +- Fix flaky test in `testApproximateRangeWithSizeOverDefault` by adjusting totalHits assertion logic ([#16434](https://github.com/opensearch-project/OpenSearch/pull/16434#pullrequestreview-2386999409)) ### Security diff --git a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java index 9c022aade5dc6..4919cbc599892 100644 --- a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java +++ b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java @@ -21,6 +21,7 @@ import org.apache.lucene.search.SortField; import org.apache.lucene.search.TopDocs; import org.apache.lucene.search.TotalHits; +import org.apache.lucene.search.TotalHits.Relation; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; import org.opensearch.search.internal.SearchContext; @@ -175,6 +176,7 @@ public void testApproximateRangeWithSizeOverDefault() throws IOException { try { long lower = 0; long upper = 12000; + long maxHits = 12001; Query approximateQuery = new ApproximatePointRangeQuery( "point", pack(lower).bytes, @@ -188,7 +190,13 @@ protected String toString(int dimension, byte[] value) { }; IndexSearcher searcher = new IndexSearcher(reader); TopDocs topDocs = searcher.search(approximateQuery, 11000); - assertEquals(topDocs.totalHits, new TotalHits(11000, TotalHits.Relation.EQUAL_TO)); + + if (topDocs.totalHits.relation == Relation.EQUAL_TO) { + assertEquals(topDocs.totalHits.value, 11000); + } else { + assertTrue(11000 <= topDocs.totalHits.value); + assertTrue(maxHits >= topDocs.totalHits.value); + } } catch (IOException e) { throw new RuntimeException(e); } @@ -226,7 +234,7 @@ protected String toString(int dimension, byte[] value) { } }; Query query = LongPoint.newRangeQuery("point", lower, upper); - ; + IndexSearcher searcher = new IndexSearcher(reader); TopDocs topDocs = searcher.search(approximateQuery, 10); TopDocs topDocs1 = searcher.search(query, 10); @@ -235,7 +243,6 @@ protected String toString(int dimension, byte[] value) { assertNotEquals(topDocs.totalHits, topDocs1.totalHits); assertEquals(topDocs.totalHits, new TotalHits(10, TotalHits.Relation.EQUAL_TO)); assertEquals(topDocs1.totalHits, new TotalHits(101, TotalHits.Relation.EQUAL_TO)); - } catch (IOException e) { throw new RuntimeException(e); } @@ -278,7 +285,7 @@ protected String toString(int dimension, byte[] value) { } }; Query query = LongPoint.newRangeQuery("point", lower, upper); - ; + IndexSearcher searcher = new IndexSearcher(reader); Sort sort = new Sort(new SortField("point", SortField.Type.LONG)); TopDocs topDocs = searcher.search(approximateQuery, 10, sort); From 9dd1a59847ad8c2716d002716521ac40afc69355 Mon Sep 17 00:00:00 2001 From: Vinay Krishna Pudyodu Date: Wed, 23 Oct 2024 12:16:57 -0700 Subject: [PATCH 08/17] Add support for restoring from snapshot with search replicas (#16111) * Add support for restoring from snapshot with search replicas Signed-off-by: Vinay Krishna Pudyodu * Fixed PR comments Signed-off-by: Vinay Krishna Pudyodu * Fixed the issues in gradle precommit Signed-off-by: Vinay Krishna Pudyodu * Fixed flaky tests Signed-off-by: Vinay Krishna Pudyodu * Fixed precommit failure Signed-off-by: Vinay Krishna Pudyodu * Fixed the codecoverage for RestoreService Signed-off-by: Vinay Krishna Pudyodu * Fixed precommit failure Signed-off-by: Vinay Krishna Pudyodu * removing unwanted tests Signed-off-by: Vinay Krishna Pudyodu * Update CHANGELOG.md Co-authored-by: Marc Handalian Signed-off-by: Vinay Krishna Pudyodu --------- Signed-off-by: Vinay Krishna Pudyodu Signed-off-by: Vinay Krishna Pudyodu Co-authored-by: Vinay Krishna Pudyodu Co-authored-by: Marc Handalian --- CHANGELOG.md | 2 + .../replication/SearchReplicaRestoreIT.java | 226 ++++++++++++++++++ .../cluster/routing/IndexRoutingTable.java | 11 + .../opensearch/snapshots/RestoreService.java | 36 +++ .../SearchOnlyReplicaRestoreTests.java | 59 +++++ .../snapshots/RestoreServiceTests.java | 37 +++ .../AbstractSnapshotIntegTestCase.java | 22 ++ 7 files changed, 393 insertions(+) create mode 100644 server/src/internalClusterTest/java/org/opensearch/indices/replication/SearchReplicaRestoreIT.java create mode 100644 server/src/test/java/org/opensearch/cluster/routing/SearchOnlyReplicaRestoreTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 5062ffd830f41..e682c1b226f4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add support for renaming aliases during snapshot restore ([#16292](https://github.com/opensearch-project/OpenSearch/pull/16292)) - Add method to return dynamic SecureTransportParameters from SecureTransportSettingsProvider interface ([#16387](https://github.com/opensearch-project/OpenSearch/pull/16387)) - URI path filtering support in cluster stats API ([#15938](https://github.com/opensearch-project/OpenSearch/pull/15938)) +- [Star Tree - Search] Add support for metric aggregations with/without term query ([15289](https://github.com/opensearch-project/OpenSearch/pull/15289)) +- Add support for restoring from snapshot with search replicas ([#16111](https://github.com/opensearch-project/OpenSearch/pull/16111)) ### Dependencies - Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https://github.com/opensearch-project/OpenSearch/pull/15578)) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SearchReplicaRestoreIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SearchReplicaRestoreIT.java new file mode 100644 index 0000000000000..352332b962c92 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SearchReplicaRestoreIT.java @@ -0,0 +1,226 @@ +/* + * 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. + */ + +package org.opensearch.indices.replication; + +import org.opensearch.action.search.SearchResponse; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.indices.replication.common.ReplicationType; +import org.opensearch.snapshots.AbstractSnapshotIntegTestCase; +import org.opensearch.snapshots.SnapshotRestoreException; +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.util.List; + +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class SearchReplicaRestoreIT extends AbstractSnapshotIntegTestCase { + + private static final String INDEX_NAME = "test-idx-1"; + private static final String RESTORED_INDEX_NAME = INDEX_NAME + "-restored"; + private static final String REPOSITORY_NAME = "test-repo"; + private static final String SNAPSHOT_NAME = "test-snapshot"; + private static final String FS_REPOSITORY_TYPE = "fs"; + private static final int DOC_COUNT = 10; + + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL, true).build(); + } + + public void testSearchReplicaRestore_WhenSnapshotOnDocRep_RestoreOnDocRepWithSearchReplica() throws Exception { + bootstrapIndexWithOutSearchReplicas(ReplicationType.DOCUMENT); + createRepoAndSnapshot(REPOSITORY_NAME, FS_REPOSITORY_TYPE, SNAPSHOT_NAME, INDEX_NAME); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> restoreSnapshot( + REPOSITORY_NAME, + SNAPSHOT_NAME, + INDEX_NAME, + RESTORED_INDEX_NAME, + Settings.builder() + .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT) + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .build() + ) + ); + assertTrue(exception.getMessage().contains(getSnapshotExceptionMessage(ReplicationType.DOCUMENT, ReplicationType.DOCUMENT))); + } + + public void testSearchReplicaRestore_WhenSnapshotOnDocRep_RestoreOnSegRepWithSearchReplica() throws Exception { + bootstrapIndexWithOutSearchReplicas(ReplicationType.DOCUMENT); + createRepoAndSnapshot(REPOSITORY_NAME, FS_REPOSITORY_TYPE, SNAPSHOT_NAME, INDEX_NAME); + + restoreSnapshot( + REPOSITORY_NAME, + SNAPSHOT_NAME, + INDEX_NAME, + RESTORED_INDEX_NAME, + Settings.builder() + .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .build() + ); + ensureYellowAndNoInitializingShards(RESTORED_INDEX_NAME); + internalCluster().startDataOnlyNode(); + ensureGreen(RESTORED_INDEX_NAME); + assertEquals(1, getNumberOfSearchReplicas(RESTORED_INDEX_NAME)); + + SearchResponse resp = client().prepareSearch(RESTORED_INDEX_NAME).setQuery(QueryBuilders.matchAllQuery()).get(); + assertHitCount(resp, DOC_COUNT); + } + + public void testSearchReplicaRestore_WhenSnapshotOnSegRep_RestoreOnDocRepWithSearchReplica() throws Exception { + bootstrapIndexWithOutSearchReplicas(ReplicationType.SEGMENT); + createRepoAndSnapshot(REPOSITORY_NAME, FS_REPOSITORY_TYPE, SNAPSHOT_NAME, INDEX_NAME); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> restoreSnapshot( + REPOSITORY_NAME, + SNAPSHOT_NAME, + INDEX_NAME, + RESTORED_INDEX_NAME, + Settings.builder() + .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT) + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .build() + ) + ); + assertTrue(exception.getMessage().contains(getSnapshotExceptionMessage(ReplicationType.SEGMENT, ReplicationType.DOCUMENT))); + } + + public void testSearchReplicaRestore_WhenSnapshotOnSegRep_RestoreOnSegRepWithSearchReplica() throws Exception { + bootstrapIndexWithOutSearchReplicas(ReplicationType.SEGMENT); + createRepoAndSnapshot(REPOSITORY_NAME, FS_REPOSITORY_TYPE, SNAPSHOT_NAME, INDEX_NAME); + + restoreSnapshot( + REPOSITORY_NAME, + SNAPSHOT_NAME, + INDEX_NAME, + RESTORED_INDEX_NAME, + Settings.builder().put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1).build() + ); + ensureYellowAndNoInitializingShards(RESTORED_INDEX_NAME); + internalCluster().startDataOnlyNode(); + ensureGreen(RESTORED_INDEX_NAME); + assertEquals(1, getNumberOfSearchReplicas(RESTORED_INDEX_NAME)); + + SearchResponse resp = client().prepareSearch(RESTORED_INDEX_NAME).setQuery(QueryBuilders.matchAllQuery()).get(); + assertHitCount(resp, DOC_COUNT); + } + + public void testSearchReplicaRestore_WhenSnapshotOnSegRepWithSearchReplica_RestoreOnDocRep() throws Exception { + bootstrapIndexWithSearchReplicas(); + createRepoAndSnapshot(REPOSITORY_NAME, FS_REPOSITORY_TYPE, SNAPSHOT_NAME, INDEX_NAME); + + SnapshotRestoreException exception = expectThrows( + SnapshotRestoreException.class, + () -> restoreSnapshot( + REPOSITORY_NAME, + SNAPSHOT_NAME, + INDEX_NAME, + RESTORED_INDEX_NAME, + Settings.builder().put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT).build() + ) + ); + assertTrue(exception.getMessage().contains(getSnapshotExceptionMessage(ReplicationType.SEGMENT, ReplicationType.DOCUMENT))); + } + + public void testSearchReplicaRestore_WhenSnapshotOnSegRepWithSearchReplica_RestoreOnDocRepWithNoSearchReplica() throws Exception { + bootstrapIndexWithSearchReplicas(); + createRepoAndSnapshot(REPOSITORY_NAME, FS_REPOSITORY_TYPE, SNAPSHOT_NAME, INDEX_NAME); + + restoreSnapshot( + REPOSITORY_NAME, + SNAPSHOT_NAME, + INDEX_NAME, + RESTORED_INDEX_NAME, + Settings.builder() + .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT) + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 0) + .build() + ); + ensureGreen(RESTORED_INDEX_NAME); + assertEquals(0, getNumberOfSearchReplicas(RESTORED_INDEX_NAME)); + + SearchResponse resp = client().prepareSearch(RESTORED_INDEX_NAME).setQuery(QueryBuilders.matchAllQuery()).get(); + assertHitCount(resp, DOC_COUNT); + } + + private void bootstrapIndexWithOutSearchReplicas(ReplicationType replicationType) throws InterruptedException { + startCluster(2); + + Settings settings = Settings.builder() + .put(super.indexSettings()) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 0) + .put(IndexMetadata.SETTING_REPLICATION_TYPE, replicationType) + .build(); + + createIndex(INDEX_NAME, settings); + indexRandomDocs(INDEX_NAME, DOC_COUNT); + refresh(INDEX_NAME); + ensureGreen(INDEX_NAME); + } + + private void bootstrapIndexWithSearchReplicas() throws InterruptedException { + startCluster(3); + + Settings settings = Settings.builder() + .put(super.indexSettings()) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .build(); + + createIndex(INDEX_NAME, settings); + ensureGreen(INDEX_NAME); + for (int i = 0; i < DOC_COUNT; i++) { + client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); + } + flushAndRefresh(INDEX_NAME); + } + + private void startCluster(int numOfNodes) { + internalCluster().startClusterManagerOnlyNode(); + internalCluster().startDataOnlyNodes(numOfNodes); + } + + private void createRepoAndSnapshot(String repositoryName, String repositoryType, String snapshotName, String indexName) { + createRepository(repositoryName, repositoryType, randomRepoPath().toAbsolutePath()); + createSnapshot(repositoryName, snapshotName, List.of(indexName)); + assertAcked(client().admin().indices().prepareDelete(INDEX_NAME)); + assertFalse("index [" + INDEX_NAME + "] should have been deleted", indexExists(INDEX_NAME)); + } + + private String getSnapshotExceptionMessage(ReplicationType snapshotReplicationType, ReplicationType restoreReplicationType) { + return "snapshot was created with [index.replication.type] as [" + + snapshotReplicationType + + "]. " + + "To restore with [index.replication.type] as [" + + restoreReplicationType + + "], " + + "[index.number_of_search_only_replicas] must be set to [0]"; + } + + private int getNumberOfSearchReplicas(String index) { + Metadata metadata = client().admin().cluster().prepareState().get().getState().metadata(); + return Integer.valueOf(metadata.index(index).getSettings().get(SETTING_NUMBER_OF_SEARCH_REPLICAS)); + } +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/IndexRoutingTable.java b/server/src/main/java/org/opensearch/cluster/routing/IndexRoutingTable.java index 9cc3bb21e2d12..b4592659bb70f 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/IndexRoutingTable.java +++ b/server/src/main/java/org/opensearch/cluster/routing/IndexRoutingTable.java @@ -573,6 +573,17 @@ private Builder initializeAsRestore( ); } } + for (int i = 0; i < indexMetadata.getNumberOfSearchOnlyReplicas(); i++) { + indexShardRoutingBuilder.addShard( + ShardRouting.newUnassigned( + shardId, + false, + true, + PeerRecoverySource.INSTANCE, // TODO: Update to remote store if enabled + unassignedInfo + ) + ); + } shards.put(shardNumber, indexShardRoutingBuilder.build()); } return this; diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 88eff93e51b38..4b5bd951f80a0 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -92,6 +92,7 @@ import org.opensearch.index.store.remote.filecache.FileCacheStats; import org.opensearch.indices.IndicesService; import org.opensearch.indices.ShardLimitValidator; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.repositories.IndexId; @@ -120,10 +121,12 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_HISTORY_UUID; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_UPGRADED; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; @@ -401,6 +404,13 @@ public ClusterState execute(ClusterState currentState) { overrideSettingsInternal, ignoreSettingsInternal ); + + validateReplicationTypeRestoreSettings( + snapshot, + metadata.index(index).getSettings().get(SETTING_REPLICATION_TYPE), + snapshotIndexMetadata + ); + if (isRemoteSnapshot) { snapshotIndexMetadata = addSnapshotToIndexSettings(snapshotIndexMetadata, snapshot, snapshotIndexId); } @@ -1303,6 +1313,32 @@ private static void validateSnapshotRestorable(final String repository, final Sn } } + // Visible for testing + static void validateReplicationTypeRestoreSettings(Snapshot snapshot, String snapshotReplicationType, IndexMetadata updatedMetadata) { + int restoreNumberOfSearchReplicas = updatedMetadata.getSettings().getAsInt(SETTING_NUMBER_OF_SEARCH_REPLICAS, 0); + + if (restoreNumberOfSearchReplicas > 0 + && ReplicationType.DOCUMENT.toString().equals(updatedMetadata.getSettings().get(SETTING_REPLICATION_TYPE))) { + throw new SnapshotRestoreException( + snapshot, + "snapshot was created with [" + + SETTING_REPLICATION_TYPE + + "]" + + " as [" + + snapshotReplicationType + + "]." + + " To restore with [" + + SETTING_REPLICATION_TYPE + + "]" + + " as [" + + ReplicationType.DOCUMENT + + "], [" + + SETTING_NUMBER_OF_SEARCH_REPLICAS + + "] must be set to [0]" + ); + } + } + public static boolean failed(SnapshotInfo snapshot, String index) { for (SnapshotShardFailure failure : snapshot.shardFailures()) { if (index.equals(failure.index())) { diff --git a/server/src/test/java/org/opensearch/cluster/routing/SearchOnlyReplicaRestoreTests.java b/server/src/test/java/org/opensearch/cluster/routing/SearchOnlyReplicaRestoreTests.java new file mode 100644 index 0000000000000..d0effe9484533 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/routing/SearchOnlyReplicaRestoreTests.java @@ -0,0 +1,59 @@ +/* + * 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. + */ + +package org.opensearch.cluster.routing; + +import org.opensearch.Version; +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.common.UUIDs; +import org.opensearch.common.settings.Settings; +import org.opensearch.repositories.IndexId; +import org.opensearch.snapshots.Snapshot; +import org.opensearch.snapshots.SnapshotId; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.HashSet; + +public class SearchOnlyReplicaRestoreTests extends OpenSearchTestCase { + + public void testSearchOnlyReplicasRestored() { + Metadata metadata = Metadata.builder() + .put( + IndexMetadata.builder("test") + .settings(settings(Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(1) + .numberOfSearchReplicas(1) + ) + .build(); + + IndexMetadata indexMetadata = metadata.index("test"); + RecoverySource.SnapshotRecoverySource snapshotRecoverySource = new RecoverySource.SnapshotRecoverySource( + UUIDs.randomBase64UUID(), + new Snapshot("rep1", new SnapshotId("snp1", UUIDs.randomBase64UUID())), + Version.CURRENT, + new IndexId("test", UUIDs.randomBase64UUID(random())) + ); + + RoutingTable routingTable = RoutingTable.builder().addAsNewRestore(indexMetadata, snapshotRecoverySource, new HashSet<>()).build(); + + ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metadata(metadata) + .routingTable(routingTable) + .build(); + + IndexShardRoutingTable indexShardRoutingTable = clusterState.routingTable().index("test").shard(0); + + assertEquals(1, clusterState.routingTable().index("test").shards().size()); + assertEquals(3, indexShardRoutingTable.getShards().size()); + assertEquals(1, indexShardRoutingTable.searchOnlyReplicas().size()); + } +} diff --git a/server/src/test/java/org/opensearch/snapshots/RestoreServiceTests.java b/server/src/test/java/org/opensearch/snapshots/RestoreServiceTests.java index b56f1b47edc33..3629f324a62e4 100644 --- a/server/src/test/java/org/opensearch/snapshots/RestoreServiceTests.java +++ b/server/src/test/java/org/opensearch/snapshots/RestoreServiceTests.java @@ -36,13 +36,17 @@ import org.opensearch.cluster.metadata.DataStream; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.common.settings.Settings; import org.opensearch.core.index.Index; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchTestCase; import java.util.Collections; import java.util.List; import static org.opensearch.cluster.DataStreamTestHelper.createTimestampField; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -122,4 +126,37 @@ public void testPrefixNotChanged() { assertEquals(renamedDataStreamName, renamedDataStream.getName()); assertEquals(Collections.singletonList(renamedIndex), renamedDataStream.getIndices()); } + + public void testValidateReplicationTypeRestoreSettings_WhenSnapshotIsDocument_RestoreToDocument() { + SnapshotId snapshotId = new SnapshotId("snapshotId", "123"); + Snapshot snapshot = new Snapshot("testRepo", snapshotId); + IndexMetadata indexMetadata = mock(IndexMetadata.class); + Settings settings = Settings.builder() + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT.toString()) + .build(); + when(indexMetadata.getSettings()).thenReturn(settings); + + assertThrows( + SnapshotRestoreException.class, + () -> RestoreService.validateReplicationTypeRestoreSettings(snapshot, ReplicationType.DOCUMENT.toString(), indexMetadata) + ); + + } + + public void testValidateReplicationTypeRestoreSettings_WhenSnapshotIsSegment_RestoreToDocument() { + SnapshotId snapshotId = new SnapshotId("snapshotId", "123"); + Snapshot snapshot = new Snapshot("testRepo", snapshotId); + IndexMetadata indexMetadata = mock(IndexMetadata.class); + Settings settings = Settings.builder() + .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1) + .put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT.toString()) + .build(); + when(indexMetadata.getSettings()).thenReturn(settings); + + assertThrows( + SnapshotRestoreException.class, + () -> RestoreService.validateReplicationTypeRestoreSettings(snapshot, ReplicationType.SEGMENT.toString(), indexMetadata) + ); + } } diff --git a/test/framework/src/main/java/org/opensearch/snapshots/AbstractSnapshotIntegTestCase.java b/test/framework/src/main/java/org/opensearch/snapshots/AbstractSnapshotIntegTestCase.java index 0bfa70a771f65..9bf725766069c 100644 --- a/test/framework/src/main/java/org/opensearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -33,6 +33,7 @@ import org.opensearch.Version; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; +import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequestBuilder; import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.index.IndexRequestBuilder; import org.opensearch.action.search.SearchRequest; @@ -61,6 +62,7 @@ import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.compress.CompressorRegistry; +import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentBuilder; @@ -506,6 +508,26 @@ protected SnapshotInfo createSnapshot(String repositoryName, String snapshot, Li return snapshotInfo; } + protected void restoreSnapshot( + String repositoryName, + String snapshotName, + String indexName, + String restoredIndexName, + Settings indexSettings + ) { + logger.info("--> restoring snapshot [{}] of {} in [{}] to [{}]", snapshotName, indexName, repositoryName, restoredIndexName); + RestoreSnapshotRequestBuilder builder = client().admin() + .cluster() + .prepareRestoreSnapshot(repositoryName, snapshotName) + .setWaitForCompletion(false) + .setRenamePattern(indexName) + .setRenameReplacement(restoredIndexName); + if (indexSettings != null) { + builder.setIndexSettings(indexSettings); + } + assertEquals(builder.get().status(), RestStatus.ACCEPTED); + } + protected void createIndexWithRandomDocs(String indexName, int docCount) throws InterruptedException { createIndex(indexName); ensureGreen(); From 119abaff95a7f54affea6c844fad7ce3c8360155 Mon Sep 17 00:00:00 2001 From: gargharsh3134 <51459091+gargharsh3134@users.noreply.github.com> Date: Thu, 24 Oct 2024 11:00:55 +0530 Subject: [PATCH 09/17] Update version to 2.18.0 for CatShards request and response (#16455) * Update version to 2.18.0 for CatShards request and response Signed-off-by: Harsh Garg * Retry Build Signed-off-by: Harsh Garg * Fix CatShardsRequestTests Signed-off-by: Harsh Garg * Retry Build Signed-off-by: Harsh Garg --------- Signed-off-by: Harsh Garg Co-authored-by: Harsh Garg --- .../action/admin/cluster/shards/CatShardsRequest.java | 4 ++-- .../action/admin/cluster/shards/CatShardsResponse.java | 4 ++-- .../java/org/opensearch/action/pagination/PageParams.java | 2 +- .../action/admin/cluster/shards/CatShardsRequestTests.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java index 0319fa103138f..9511a45423e63 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsRequest.java @@ -37,7 +37,7 @@ public CatShardsRequest() {} public CatShardsRequest(StreamInput in) throws IOException { super(in); - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + if (in.getVersion().onOrAfter(Version.V_2_18_0)) { indices = in.readStringArray(); cancelAfterTimeInterval = in.readOptionalTimeValue(); if (in.readBoolean()) { @@ -50,7 +50,7 @@ public CatShardsRequest(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + if (out.getVersion().onOrAfter(Version.V_2_18_0)) { if (indices == null) { out.writeVInt(0); } else { diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java index c2499ab190ded..dee0b0f2d45c0 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/CatShardsResponse.java @@ -38,7 +38,7 @@ public CatShardsResponse() {} public CatShardsResponse(StreamInput in) throws IOException { super(in); indicesStatsResponse = new IndicesStatsResponse(in); - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + if (in.getVersion().onOrAfter(Version.V_2_18_0)) { nodes = DiscoveryNodes.readFrom(in, null); responseShards = in.readList(ShardRouting::new); if (in.readBoolean()) { @@ -50,7 +50,7 @@ public CatShardsResponse(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { indicesStatsResponse.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + if (out.getVersion().onOrAfter(Version.V_2_18_0)) { nodes.writeToWithAttribute(out); out.writeList(responseShards); out.writeBoolean(pageToken != null); diff --git a/server/src/main/java/org/opensearch/action/pagination/PageParams.java b/server/src/main/java/org/opensearch/action/pagination/PageParams.java index 03de1aa465a15..6459893a8505f 100644 --- a/server/src/main/java/org/opensearch/action/pagination/PageParams.java +++ b/server/src/main/java/org/opensearch/action/pagination/PageParams.java @@ -20,7 +20,7 @@ * * Class specific to paginated queries, which will contain common query params required by a paginated API. */ -@PublicApi(since = "3.0.0") +@PublicApi(since = "2.18.0") public class PageParams implements Writeable { public static final String PARAM_SORT = "sort"; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java index f4215f54c1e21..e161342c3c609 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/shards/CatShardsRequestTests.java @@ -95,7 +95,7 @@ public void testSerializationWithOlderVersionsParametersNotSerialized() throws E catShardsRequest.setCancelAfterTimeInterval(TimeValue.timeValueMillis(randomIntBetween(1, 5))); catShardsRequest.setIndices(new String[2]); - Version version = VersionUtils.getPreviousVersion(Version.CURRENT); + Version version = VersionUtils.getPreviousVersion(Version.V_2_18_0); try (BytesStreamOutput out = new BytesStreamOutput()) { out.setVersion(version); catShardsRequest.writeTo(out); From 94987937d77f4aac65e5e57a56a3ab5f45b73bc5 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Thu, 24 Oct 2024 08:51:01 -0400 Subject: [PATCH 10/17] Update 2.x to 2.19.0 (since 2.18.0 was cut) (#16466) Signed-off-by: Andriy Redko --- .ci/bwcVersions | 1 + libs/core/src/main/java/org/opensearch/Version.java | 1 + 2 files changed, 2 insertions(+) diff --git a/.ci/bwcVersions b/.ci/bwcVersions index 5587b8e5784c5..17bb5a7df9b21 100644 --- a/.ci/bwcVersions +++ b/.ci/bwcVersions @@ -41,3 +41,4 @@ BWC_VERSION: - "2.17.1" - "2.17.2" - "2.18.0" + - "2.19.0" \ No newline at end of file diff --git a/libs/core/src/main/java/org/opensearch/Version.java b/libs/core/src/main/java/org/opensearch/Version.java index 6b1f6898eec9b..4d685e3bc654a 100644 --- a/libs/core/src/main/java/org/opensearch/Version.java +++ b/libs/core/src/main/java/org/opensearch/Version.java @@ -113,6 +113,7 @@ public class Version implements Comparable, ToXContentFragment { public static final Version V_2_17_2 = new Version(2170299, org.apache.lucene.util.Version.LUCENE_9_11_1); public static final Version V_2_18_0 = new Version(2180099, org.apache.lucene.util.Version.LUCENE_9_12_0); public static final Version V_3_0_0 = new Version(3000099, org.apache.lucene.util.Version.LUCENE_9_12_0); + public static final Version V_2_19_0 = new Version(2190099, org.apache.lucene.util.Version.LUCENE_9_12_0); public static final Version CURRENT = V_3_0_0; public static Version fromId(int id) { From 936cdb9043fb183cabe208fd7f7fc44a44b8f842 Mon Sep 17 00:00:00 2001 From: "Spencer G. Jones" Date: Thu, 24 Oct 2024 11:07:57 -0700 Subject: [PATCH 11/17] Change to correct version since this has been backported (#16472) Signed-off-by: Spencer G. Jones --- .../snapshots/restore/RestoreSnapshotRequest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 42c64e04268e3..84bc87a5cb1ba 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -166,11 +166,10 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_2_17_0)) { sourceRemoteTranslogRepository = in.readOptionalString(); } - // TODO: change to V_2_18_0 once this is backported into that version - if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.getVersion().onOrAfter(Version.V_2_18_0)) { renameAliasPattern = in.readOptionalString(); } - if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.getVersion().onOrAfter(Version.V_2_18_0)) { renameAliasReplacement = in.readOptionalString(); } } @@ -200,11 +199,10 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_17_0)) { out.writeOptionalString(sourceRemoteTranslogRepository); } - // TODO: change to V_2_18_0 once this is backported into that version - if (out.getVersion().onOrAfter(Version.CURRENT)) { + if (out.getVersion().onOrAfter(Version.V_2_18_0)) { out.writeOptionalString(renameAliasPattern); } - if (out.getVersion().onOrAfter(Version.CURRENT)) { + if (out.getVersion().onOrAfter(Version.V_2_18_0)) { out.writeOptionalString(renameAliasReplacement); } } From 4ad1be3825f8a47ff94fdb3cee63db78c2c142a7 Mon Sep 17 00:00:00 2001 From: Sooraj Sinha <81695996+soosinha@users.noreply.github.com> Date: Fri, 25 Oct 2024 11:08:05 +0530 Subject: [PATCH 12/17] Revert uploading of manifest using min codec version (#16403) Signed-off-by: Sooraj Sinha --- CHANGELOG.md | 1 + .../PublicationTransportHandler.java | 1 - .../opensearch/gateway/GatewayMetaState.java | 14 +-- .../remote/RemoteClusterStateService.java | 12 +-- .../gateway/remote/RemoteManifestManager.java | 5 +- .../coordination/CoordinationStateTests.java | 6 +- .../GatewayMetaStatePersistedStateTests.java | 36 +++---- .../RemoteClusterStateServiceTests.java | 97 ++++++------------- 8 files changed, 57 insertions(+), 115 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e682c1b226f4a..f5dc69a9ec290 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix typo super->sb in method toString() of RemoteStoreNodeAttribute ([#15362](https://github.com/opensearch-project/OpenSearch/pull/15362)) - [Workload Management] Fixing Create/Update QueryGroup TransportActions to execute from non-cluster manager nodes ([16422](https://github.com/opensearch-project/OpenSearch/pull/16422)) - Fix flaky test in `testApproximateRangeWithSizeOverDefault` by adjusting totalHits assertion logic ([#16434](https://github.com/opensearch-project/OpenSearch/pull/16434#pullrequestreview-2386999409)) +- Revert changes to upload remote state manifest using minimum codec version([#16403](https://github.com/opensearch-project/OpenSearch/pull/16403)) ### Security diff --git a/server/src/main/java/org/opensearch/cluster/coordination/PublicationTransportHandler.java b/server/src/main/java/org/opensearch/cluster/coordination/PublicationTransportHandler.java index d30efde52bffb..c4cb484cda693 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/PublicationTransportHandler.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/PublicationTransportHandler.java @@ -367,7 +367,6 @@ public PublicationContext newPublicationContext( } private boolean validateRemotePublicationConfiguredOnAllNodes(DiscoveryNodes discoveryNodes) { - assert ClusterMetadataManifest.getCodecForVersion(discoveryNodes.getMinNodeVersion()) >= ClusterMetadataManifest.CODEC_V0; for (DiscoveryNode node : discoveryNodes.getNodes().values()) { // if a node is non-remote then created local publication context if (node.isRemoteStatePublicationEnabled() == false) { diff --git a/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java b/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java index f5da6df2689bd..1e2f5612ca002 100644 --- a/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java +++ b/server/src/main/java/org/opensearch/gateway/GatewayMetaState.java @@ -753,12 +753,8 @@ public void setLastAcceptedState(ClusterState clusterState) { } try { final RemoteClusterStateManifestInfo manifestDetails; - // Decide the codec version - int codecVersion = ClusterMetadataManifest.getCodecForVersion(clusterState.nodes().getMinNodeVersion()); - assert codecVersion >= 0 : codecVersion; - logger.info("codec version is {}", codecVersion); - if (shouldWriteFullClusterState(clusterState, codecVersion)) { + if (shouldWriteFullClusterState(clusterState)) { final Optional latestManifest = remoteClusterStateService.getLatestClusterMetadataManifest( clusterState.getClusterName().value(), clusterState.metadata().clusterUUID() @@ -775,7 +771,7 @@ public void setLastAcceptedState(ClusterState clusterState) { clusterState.metadata().clusterUUID() ); } - manifestDetails = remoteClusterStateService.writeFullMetadata(clusterState, previousClusterUUID, codecVersion); + manifestDetails = remoteClusterStateService.writeFullMetadata(clusterState, previousClusterUUID); } else { assert verifyManifestAndClusterState(lastAcceptedManifest, lastAcceptedState) == true : "Previous manifest and previous ClusterState are not in sync"; @@ -820,13 +816,11 @@ private boolean verifyManifestAndClusterState(ClusterMetadataManifest manifest, return true; } - private boolean shouldWriteFullClusterState(ClusterState clusterState, int codecVersion) { - assert lastAcceptedManifest == null || lastAcceptedManifest.getCodecVersion() <= codecVersion; + private boolean shouldWriteFullClusterState(ClusterState clusterState) { if (lastAcceptedState == null || lastAcceptedManifest == null || (remoteClusterStateService.isRemotePublicationEnabled() == false && lastAcceptedState.term() != clusterState.term()) - || lastAcceptedManifest.getOpensearchVersion() != Version.CURRENT - || lastAcceptedManifest.getCodecVersion() != codecVersion) { + || lastAcceptedManifest.getOpensearchVersion() != Version.CURRENT) { return true; } return false; diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java index 0cd2025b98783..dc41189afc3cb 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -301,8 +301,7 @@ public RemoteClusterStateService( * @return A manifest object which contains the details of uploaded entity metadata. */ @Nullable - public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterState, String previousClusterUUID, int codecVersion) - throws IOException { + public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterState, String previousClusterUUID) throws IOException { final long startTimeNanos = relativeTimeNanosSupplier.getAsLong(); if (clusterState.nodes().isLocalNodeElectedClusterManager() == false) { logger.error("Local node is not elected cluster manager. Exiting"); @@ -342,8 +341,7 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat !remoteClusterStateValidationMode.equals(RemoteClusterStateValidationMode.NONE) ? new ClusterStateChecksum(clusterState, threadpool) : null, - false, - codecVersion + false ); final long durationMillis = TimeValue.nsecToMSec(relativeTimeNanosSupplier.getAsLong() - startTimeNanos); @@ -551,8 +549,7 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( !remoteClusterStateValidationMode.equals(RemoteClusterStateValidationMode.NONE) ? new ClusterStateChecksum(clusterState, threadpool) : null, - false, - previousManifest.getCodecVersion() + false ); final long durationMillis = TimeValue.nsecToMSec(relativeTimeNanosSupplier.getAsLong() - startTimeNanos); @@ -1024,8 +1021,7 @@ public RemoteClusterStateManifestInfo markLastStateAsCommitted( !remoteClusterStateValidationMode.equals(RemoteClusterStateValidationMode.NONE) ? new ClusterStateChecksum(clusterState, threadpool) : null, - true, - previousManifest.getCodecVersion() + true ); if (!previousManifest.isClusterUUIDCommitted() && committedManifestDetails.getClusterMetadataManifest().isClusterUUIDCommitted()) { remoteClusterStateCleanupManager.deleteStaleClusterUUIDs(clusterState, committedManifestDetails.getClusterMetadataManifest()); diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java index b243269fe323e..20e14ff805ca8 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java @@ -100,8 +100,7 @@ RemoteClusterStateManifestInfo uploadManifest( String previousClusterUUID, ClusterStateDiffManifest clusterDiffManifest, ClusterStateChecksum clusterStateChecksum, - boolean committed, - int codecVersion + boolean committed ) { synchronized (this) { ClusterMetadataManifest.Builder manifestBuilder = ClusterMetadataManifest.builder(); @@ -112,7 +111,7 @@ RemoteClusterStateManifestInfo uploadManifest( .opensearchVersion(Version.CURRENT) .nodeId(nodeId) .committed(committed) - .codecVersion(codecVersion) + .codecVersion(ClusterMetadataManifest.MANIFEST_CURRENT_CODEC_VERSION) .indices(uploadedMetadataResult.uploadedIndexMetadata) .previousClusterUUID(previousClusterUUID) .clusterUUIDCommitted(clusterState.metadata().clusterUUIDCommitted()) diff --git a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java index b5d16e7be849f..f707198efb073 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/CoordinationStateTests.java @@ -67,7 +67,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; -import static org.opensearch.gateway.remote.ClusterMetadataManifest.MANIFEST_CURRENT_CODEC_VERSION; import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX; @@ -962,7 +961,7 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep .previousClusterUUID(randomAlphaOfLength(10)) .clusterUUIDCommitted(true) .build(); - when(remoteClusterStateService.writeFullMetadata(clusterState, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION)).thenReturn( + when(remoteClusterStateService.writeFullMetadata(clusterState, previousClusterUUID)).thenReturn( new RemoteClusterStateManifestInfo(manifest, "path/to/manifest") ); @@ -975,8 +974,7 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep final CoordinationState coordinationState = createCoordinationState(persistedStateRegistry, node1, remoteStateSettings()); coordinationState.handlePrePublish(clusterState); - Mockito.verify(remoteClusterStateService, Mockito.times(1)) - .writeFullMetadata(clusterState, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION); + Mockito.verify(remoteClusterStateService, Mockito.times(1)).writeFullMetadata(clusterState, previousClusterUUID); assertThat(persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE).getLastAcceptedState(), equalTo(clusterState)); when(remoteClusterStateService.markLastStateAsCommitted(any(), any(), eq(false))).thenReturn( diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index efdb3076f419c..955ea82e219e8 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -759,7 +759,7 @@ public void testRemotePersistedState() throws IOException { final RemoteClusterStateService remoteClusterStateService = Mockito.mock(RemoteClusterStateService.class); final ClusterMetadataManifest manifest = ClusterMetadataManifest.builder().clusterTerm(1L).stateVersion(5L).build(); final String previousClusterUUID = "prev-cluster-uuid"; - Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any(), eq(MANIFEST_CURRENT_CODEC_VERSION))) + Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any())) .thenReturn(new RemoteClusterStateManifestInfo(manifest, "path/to/manifest")); Mockito.when(remoteClusterStateService.writeIncrementalMetadata(Mockito.any(), Mockito.any(), Mockito.any())) @@ -777,7 +777,7 @@ public void testRemotePersistedState() throws IOException { ); remotePersistedState.setLastAcceptedState(clusterState); - Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION); + Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState, previousClusterUUID); assertThat(remotePersistedState.getLastAcceptedState(), equalTo(clusterState)); assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm)); @@ -789,8 +789,7 @@ public void testRemotePersistedState() throws IOException { ); remotePersistedState.setLastAcceptedState(secondClusterState); - Mockito.verify(remoteClusterStateService, times(1)) - .writeFullMetadata(secondClusterState, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION); + Mockito.verify(remoteClusterStateService, times(1)).writeFullMetadata(secondClusterState, previousClusterUUID); assertThat(remotePersistedState.getLastAcceptedState(), equalTo(secondClusterState)); assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm)); @@ -820,9 +819,9 @@ public void testRemotePersistedStateWithDifferentNodeConfiguration() throws IOEx .clusterTerm(1L) .stateVersion(5L) .codecVersion(CODEC_V1) - .opensearchVersion(Version.CURRENT) + .opensearchVersion(Version.V_2_15_0) .build(); - Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any(), eq(CODEC_V1))) + Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any())) .thenReturn(new RemoteClusterStateManifestInfo(manifest, "path/to/manifest2")); CoordinationState.PersistedState remotePersistedState = new RemotePersistedState(remoteClusterStateService, previousClusterUUID); @@ -833,7 +832,7 @@ public void testRemotePersistedStateWithDifferentNodeConfiguration() throws IOEx ); remotePersistedState.setLastAcceptedState(clusterState1); - Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState1, previousClusterUUID, CODEC_V1); + Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState1, previousClusterUUID); ClusterState clusterState2 = createClusterState( randomNonNegativeLong(), @@ -846,10 +845,10 @@ public void testRemotePersistedStateWithDifferentNodeConfiguration() throws IOEx .codecVersion(MANIFEST_CURRENT_CODEC_VERSION) .opensearchVersion(Version.CURRENT) .build(); - Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any(), eq(MANIFEST_CURRENT_CODEC_VERSION))) + Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any())) .thenReturn(new RemoteClusterStateManifestInfo(manifest2, "path/to/manifest")); remotePersistedState.setLastAcceptedState(clusterState2); - Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState2, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION); + Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterState2, previousClusterUUID); ClusterState clusterState3 = createClusterState( randomNonNegativeLong(), @@ -889,8 +888,7 @@ public void testRemotePersistentState_FollowerNode() throws IOException { remotePersistedState.setLastAcceptedState(clusterState); remotePersistedState.setLastAcceptedManifest(manifest); - Mockito.verify(remoteClusterStateService, never()) - .writeFullMetadata(clusterState, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION); + Mockito.verify(remoteClusterStateService, never()).writeFullMetadata(clusterState, previousClusterUUID); assertEquals(clusterState, remotePersistedState.getLastAcceptedState()); assertEquals(clusterTerm, remotePersistedState.getCurrentTerm()); @@ -906,8 +904,7 @@ public void testRemotePersistentState_FollowerNode() throws IOException { ); remotePersistedState.setLastAcceptedState(secondClusterState); - Mockito.verify(remoteClusterStateService, never()) - .writeFullMetadata(secondClusterState, previousClusterUUID, MANIFEST_CURRENT_CODEC_VERSION); + Mockito.verify(remoteClusterStateService, never()).writeFullMetadata(secondClusterState, previousClusterUUID); assertEquals(secondClusterState, remotePersistedState.getLastAcceptedState()); assertEquals(clusterTerm, remotePersistedState.getCurrentTerm()); @@ -940,7 +937,7 @@ public void testRemotePersistedStateNotCommitted() throws IOException { .build(); Mockito.when(remoteClusterStateService.getLatestClusterMetadataManifest(Mockito.any(), Mockito.any())) .thenReturn(Optional.of(manifest)); - Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any(), eq(MANIFEST_CURRENT_CODEC_VERSION))) + Mockito.when(remoteClusterStateService.writeFullMetadata(Mockito.any(), Mockito.any())) .thenReturn(new RemoteClusterStateManifestInfo(manifest, "path/to/manifest")); Mockito.when(remoteClusterStateService.writeIncrementalMetadata(Mockito.any(), Mockito.any(), Mockito.any())) @@ -966,17 +963,14 @@ public void testRemotePersistedStateNotCommitted() throws IOException { remotePersistedState.setLastAcceptedState(clusterState); ArgumentCaptor previousClusterUUIDCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor clusterStateCaptor = ArgumentCaptor.forClass(ClusterState.class); - Mockito.verify(remoteClusterStateService) - .writeFullMetadata(clusterStateCaptor.capture(), previousClusterUUIDCaptor.capture(), eq(MANIFEST_CURRENT_CODEC_VERSION)); + Mockito.verify(remoteClusterStateService).writeFullMetadata(clusterStateCaptor.capture(), previousClusterUUIDCaptor.capture()); assertEquals(previousClusterUUID, previousClusterUUIDCaptor.getValue()); } public void testRemotePersistedStateExceptionOnFullStateUpload() throws IOException { final RemoteClusterStateService remoteClusterStateService = Mockito.mock(RemoteClusterStateService.class); final String previousClusterUUID = "prev-cluster-uuid"; - Mockito.doThrow(IOException.class) - .when(remoteClusterStateService) - .writeFullMetadata(Mockito.any(), Mockito.any(), eq(MANIFEST_CURRENT_CODEC_VERSION)); + Mockito.doThrow(IOException.class).when(remoteClusterStateService).writeFullMetadata(Mockito.any(), Mockito.any()); CoordinationState.PersistedState remotePersistedState = new RemotePersistedState(remoteClusterStateService, previousClusterUUID); @@ -994,9 +988,7 @@ public void testRemotePersistedStateFailureStats() throws IOException { RemoteUploadStats remoteStateStats = new RemoteUploadStats(); final RemoteClusterStateService remoteClusterStateService = Mockito.mock(RemoteClusterStateService.class); final String previousClusterUUID = "prev-cluster-uuid"; - Mockito.doThrow(IOException.class) - .when(remoteClusterStateService) - .writeFullMetadata(Mockito.any(), Mockito.any(), eq(MANIFEST_CURRENT_CODEC_VERSION)); + Mockito.doThrow(IOException.class).when(remoteClusterStateService).writeFullMetadata(Mockito.any(), Mockito.any()); when(remoteClusterStateService.getUploadStats()).thenReturn(remoteStateStats); doAnswer((i) -> { remoteStateStats.stateFailed(); diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java index dffbb9d82545a..448b9cc9d78ac 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -280,8 +280,7 @@ public void testFailWriteFullMetadataNonClusterManagerNode() throws IOException final ClusterState clusterState = generateClusterStateWithOneIndex().build(); final RemoteClusterStateManifestInfo manifestDetails = remoteClusterStateService.writeFullMetadata( clusterState, - randomAlphaOfLength(10), - MANIFEST_CURRENT_CODEC_VERSION + randomAlphaOfLength(10) ); Assert.assertThat(manifestDetails, nullValue()); } @@ -327,11 +326,8 @@ public void testWriteFullMetadataSuccess() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); mockBlobStoreObjects(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); List indices = List.of(uploadedIndexMetadata); @@ -401,11 +397,8 @@ public void testWriteFullMetadataSuccessPublicationEnabled() throws IOException .build(); mockBlobStoreObjects(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); List indices = List.of(uploadedIndexMetadata); @@ -453,11 +446,8 @@ public void testWriteFullMetadataInParallelSuccess() throws IOException { }).when(container).asyncBlobUpload(writeContextArgumentCaptor.capture(), actionListenerArgumentCaptor.capture()); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); List indices = List.of(uploadedIndexMetadata); @@ -534,7 +524,7 @@ public void run() { remoteClusterStateService.start(); assertThrows( RemoteStateTransferException.class, - () -> remoteClusterStateService.writeFullMetadata(clusterState, randomAlphaOfLength(10), MANIFEST_CURRENT_CODEC_VERSION) + () -> remoteClusterStateService.writeFullMetadata(clusterState, randomAlphaOfLength(10)) ); } @@ -578,7 +568,7 @@ public void testTimeoutWhileWritingManifestFile() throws IOException { ).thenReturn(new RemoteClusterStateUtils.UploadedMetadataResults()); RemoteStateTransferException ex = expectThrows( RemoteStateTransferException.class, - () -> spiedService.writeFullMetadata(clusterState, randomAlphaOfLength(10), MANIFEST_CURRENT_CODEC_VERSION) + () -> spiedService.writeFullMetadata(clusterState, randomAlphaOfLength(10)) ); assertTrue(ex.getMessage().contains("Timed out waiting for transfer")); } @@ -600,7 +590,7 @@ public void testWriteFullMetadataInParallelFailureForIndexMetadata() throws IOEx remoteClusterStateService.start(); assertThrows( RemoteStateTransferException.class, - () -> remoteClusterStateService.writeFullMetadata(clusterState, randomAlphaOfLength(10), MANIFEST_CURRENT_CODEC_VERSION) + () -> remoteClusterStateService.writeFullMetadata(clusterState, randomAlphaOfLength(10)) ); assertEquals(0, remoteClusterStateService.getUploadStats().getSuccessCount()); } @@ -1850,7 +1840,7 @@ private void verifyCodecMigrationManifest(int previousCodec) throws IOException // global metadata is updated assertThat(manifestAfterUpdate.hasMetadataAttributesFiles(), is(true)); // During incremental update, codec version will not change. - assertThat(manifestAfterUpdate.getCodecVersion(), is(previousCodec)); + assertThat(manifestAfterUpdate.getCodecVersion(), is(MANIFEST_CURRENT_CODEC_VERSION)); } public void testWriteIncrementalGlobalMetadataFromCodecV0Success() throws IOException { @@ -1885,7 +1875,7 @@ private void verifyWriteIncrementalGlobalMetadataFromOlderCodecSuccess(ClusterMe ).getClusterMetadataManifest(); final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() - .codecVersion(previousManifest.getCodecVersion()) + .codecVersion(MANIFEST_CURRENT_CODEC_VERSION) .indices(Collections.emptyList()) .clusterTerm(1L) .stateVersion(1L) @@ -2074,11 +2064,8 @@ public void testCustomMetadataDeletedUpdatedAndAdded() throws IOException { // Initial cluster state with index. final ClusterState initialClusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest initialManifest = remoteClusterStateService.writeFullMetadata( - initialClusterState, - "_na_", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest initialManifest = remoteClusterStateService.writeFullMetadata(initialClusterState, "_na_") + .getClusterMetadataManifest(); ClusterState clusterState1 = ClusterState.builder(initialClusterState) .metadata( @@ -2156,11 +2143,8 @@ public void testIndexMetadataDeletedUpdatedAndAdded() throws IOException { // Initial cluster state with index. final ClusterState initialClusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest initialManifest = remoteClusterStateService.writeFullMetadata( - initialClusterState, - "_na_", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest initialManifest = remoteClusterStateService.writeFullMetadata(initialClusterState, "_na_") + .getClusterMetadataManifest(); String initialIndex = "test-index"; Index index1 = new Index("test-index-1", "index-uuid-1"); Index index2 = new Index("test-index-2", "index-uuid-2"); @@ -2238,11 +2222,8 @@ private void verifyMetadataAttributeOnlyUpdated( // Initial cluster state with index. final ClusterState initialClusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest initialManifest = remoteClusterStateService.writeFullMetadata( - initialClusterState, - "_na_", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest initialManifest = remoteClusterStateService.writeFullMetadata(initialClusterState, "_na_") + .getClusterMetadataManifest(); ClusterState newClusterState = clusterStateUpdater.apply(initialClusterState); @@ -2255,11 +2236,8 @@ private void verifyMetadataAttributeOnlyUpdated( initialManifest ).getClusterMetadataManifest(); } else { - manifestAfterMetadataUpdate = remoteClusterStateService.writeFullMetadata( - newClusterState, - initialClusterState.stateUUID(), - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + manifestAfterMetadataUpdate = remoteClusterStateService.writeFullMetadata(newClusterState, initialClusterState.stateUUID()) + .getClusterMetadataManifest(); } assertions.accept(initialManifest, manifestAfterMetadataUpdate); @@ -2742,11 +2720,8 @@ public void testRemoteStateUploadStats() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); mockBlobStoreObjects(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); assertTrue(remoteClusterStateService.getUploadStats() != null); assertEquals(1, remoteClusterStateService.getUploadStats().getSuccessCount()); @@ -2801,11 +2776,8 @@ public void testWriteFullMetadataSuccessWithRoutingTable() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( "test-index", @@ -2854,11 +2826,8 @@ public void testWriteFullMetadataInParallelSuccessWithRoutingTable() throws IOEx when((blobStoreRepository.basePath())).thenReturn(BlobPath.cleanPath().add("base-path")); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( @@ -3108,11 +3077,8 @@ public void testWriteFullMetadataSuccessWithChecksumValidationEnabled() throws I final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( "test-index", @@ -3152,11 +3118,8 @@ public void testWriteFullMetadataSuccessWithChecksumValidationModeNone() throws final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); - final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata( - clusterState, - "prev-cluster-uuid", - MANIFEST_CURRENT_CODEC_VERSION - ).getClusterMetadataManifest(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeFullMetadata(clusterState, "prev-cluster-uuid") + .getClusterMetadataManifest(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( "test-index", From bb45f0343553944627b18a3fd8fa0b79d0991b69 Mon Sep 17 00:00:00 2001 From: Arpit-Bandejiya Date: Fri, 25 Oct 2024 17:46:59 +0530 Subject: [PATCH 13/17] Add Setting to adjust the primary constraint weights (#16471) Add Setting to adjust the primary constraint weights (#16471) Signed-off-by: Arpit Bandejiya --- CHANGELOG.md | 1 + .../allocation/AllocationConstraints.java | 4 +-- .../routing/allocation/Constraint.java | 13 ++++--- .../routing/allocation/ConstraintTypes.java | 10 ++++++ .../allocation/RebalanceConstraints.java | 4 +-- .../allocator/BalancedShardsAllocator.java | 35 ++++++++++++++++--- .../common/settings/ClusterSettings.java | 1 + .../AllocationConstraintsTests.java | 28 ++++++++------- 8 files changed, 71 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5dc69a9ec290..d6e76d946866b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - URI path filtering support in cluster stats API ([#15938](https://github.com/opensearch-project/OpenSearch/pull/15938)) - [Star Tree - Search] Add support for metric aggregations with/without term query ([15289](https://github.com/opensearch-project/OpenSearch/pull/15289)) - Add support for restoring from snapshot with search replicas ([#16111](https://github.com/opensearch-project/OpenSearch/pull/16111)) +- Add Setting to adjust the primary constraint weights ([#16471](https://github.com/opensearch-project/OpenSearch/pull/16471)) ### Dependencies - Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https://github.com/opensearch-project/OpenSearch/pull/15578)) diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationConstraints.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationConstraints.java index 6702db4b43e91..4b71347e4071b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationConstraints.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationConstraints.java @@ -39,8 +39,8 @@ public void updateAllocationConstraint(String constraint, boolean enable) { this.constraints.get(constraint).setEnable(enable); } - public long weight(ShardsBalancer balancer, BalancedShardsAllocator.ModelNode node, String index) { - Constraint.ConstraintParams params = new Constraint.ConstraintParams(balancer, node, index); + public long weight(ShardsBalancer balancer, BalancedShardsAllocator.ModelNode node, String index, long primaryThresholdWeight) { + Constraint.ConstraintParams params = new Constraint.ConstraintParams(balancer, node, index, primaryThresholdWeight); return params.weight(constraints); } } diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/Constraint.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/Constraint.java index e9c3c0afcbe88..ce0bb70d7d0b7 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/Constraint.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/Constraint.java @@ -14,7 +14,7 @@ import java.util.Map; import java.util.function.Predicate; -import static org.opensearch.cluster.routing.allocation.ConstraintTypes.CONSTRAINT_WEIGHT; +import static org.opensearch.cluster.routing.allocation.ConstraintTypes.predicateKeyToWeightMap; /** * Defines a constraint useful to de-prioritize certain nodes as target of unassigned shards used in {@link AllocationConstraints} or @@ -44,11 +44,13 @@ static class ConstraintParams { private ShardsBalancer balancer; private BalancedShardsAllocator.ModelNode node; private String index; + private long PrimaryConstraintThreshold; - ConstraintParams(ShardsBalancer balancer, BalancedShardsAllocator.ModelNode node, String index) { + ConstraintParams(ShardsBalancer balancer, BalancedShardsAllocator.ModelNode node, String index, long primaryConstraintThreshold) { this.balancer = balancer; this.node = node; this.index = index; + this.PrimaryConstraintThreshold = primaryConstraintThreshold; } public ShardsBalancer getBalancer() { @@ -75,9 +77,12 @@ public String getIndex() { */ public long weight(Map constraints) { long totalConstraintWeight = 0; - for (Constraint constraint : constraints.values()) { + for (Map.Entry entry : constraints.entrySet()) { + String key = entry.getKey(); + Constraint constraint = entry.getValue(); if (constraint.test(this)) { - totalConstraintWeight += CONSTRAINT_WEIGHT; + double weight = predicateKeyToWeightMap(key, PrimaryConstraintThreshold); + totalConstraintWeight += weight; } } return totalConstraintWeight; diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/ConstraintTypes.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/ConstraintTypes.java index 28ad199218884..ff40556a4894b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/ConstraintTypes.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/ConstraintTypes.java @@ -86,4 +86,14 @@ public static Predicate isPrimaryShardsPerNodeBreac return primaryShardCount >= allowedPrimaryShardCount; }; } + + public static long predicateKeyToWeightMap(String key, long primaryConstraintWeight) { + switch (key) { + case CLUSTER_PRIMARY_SHARD_BALANCE_CONSTRAINT_ID: + case CLUSTER_PRIMARY_SHARD_REBALANCE_CONSTRAINT_ID: + return primaryConstraintWeight; + default: + return CONSTRAINT_WEIGHT; + } + } } diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/RebalanceConstraints.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/RebalanceConstraints.java index 2c2138af18abc..803ef04ce1675 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/RebalanceConstraints.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/RebalanceConstraints.java @@ -42,8 +42,8 @@ public void updateRebalanceConstraint(String constraint, boolean enable) { this.constraints.get(constraint).setEnable(enable); } - public long weight(ShardsBalancer balancer, BalancedShardsAllocator.ModelNode node, String index) { - Constraint.ConstraintParams params = new Constraint.ConstraintParams(balancer, node, index); + public long weight(ShardsBalancer balancer, BalancedShardsAllocator.ModelNode node, String index, long primaryConstraintThreshold) { + Constraint.ConstraintParams params = new Constraint.ConstraintParams(balancer, node, index, primaryConstraintThreshold); return params.weight(constraints); } } diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 785636fa7ff2a..cfbb4d34c3a38 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -139,6 +139,14 @@ public class BalancedShardsAllocator implements ShardsAllocator { Property.NodeScope ); + public static final Setting PRIMARY_CONSTRAINT_THRESHOLD_SETTING = Setting.longSetting( + "cluster.routing.allocation.primary_constraint.threshold", + 10, + 0, + Property.Dynamic, + Property.NodeScope + ); + /** * This setting governs whether primary shards balance is desired during allocation. This is used by {@link ConstraintTypes#isPerIndexPrimaryShardsPerNodeBreached()} * and {@link ConstraintTypes#isPrimaryShardsPerNodeBreached} which is used during unassigned shard allocation @@ -201,6 +209,7 @@ public class BalancedShardsAllocator implements ShardsAllocator { private volatile float shardBalanceFactor; private volatile WeightFunction weightFunction; private volatile float threshold; + private volatile long primaryConstraintThreshold; private volatile boolean ignoreThrottleInRestore; private volatile TimeValue allocatorTimeout; @@ -219,6 +228,7 @@ public BalancedShardsAllocator(Settings settings, ClusterSettings clusterSetting setIgnoreThrottleInRestore(IGNORE_THROTTLE_FOR_REMOTE_RESTORE.get(settings)); updateWeightFunction(); setThreshold(THRESHOLD_SETTING.get(settings)); + setPrimaryConstraintThresholdSetting(PRIMARY_CONSTRAINT_THRESHOLD_SETTING.get(settings)); setPreferPrimaryShardBalance(PREFER_PRIMARY_SHARD_BALANCE.get(settings)); setPreferPrimaryShardRebalance(PREFER_PRIMARY_SHARD_REBALANCE.get(settings)); setShardMovementStrategy(SHARD_MOVEMENT_STRATEGY_SETTING.get(settings)); @@ -231,6 +241,7 @@ public BalancedShardsAllocator(Settings settings, ClusterSettings clusterSetting clusterSettings.addSettingsUpdateConsumer(PRIMARY_SHARD_REBALANCE_BUFFER, this::updatePreferPrimaryShardBalanceBuffer); clusterSettings.addSettingsUpdateConsumer(PREFER_PRIMARY_SHARD_REBALANCE, this::setPreferPrimaryShardRebalance); clusterSettings.addSettingsUpdateConsumer(THRESHOLD_SETTING, this::setThreshold); + clusterSettings.addSettingsUpdateConsumer(PRIMARY_CONSTRAINT_THRESHOLD_SETTING, this::setPrimaryConstraintThresholdSetting); clusterSettings.addSettingsUpdateConsumer(IGNORE_THROTTLE_FOR_REMOTE_RESTORE, this::setIgnoreThrottleInRestore); clusterSettings.addSettingsUpdateConsumer(ALLOCATOR_TIMEOUT_SETTING, this::setAllocatorTimeout); } @@ -294,7 +305,12 @@ private void updatePreferPrimaryShardBalanceBuffer(float preferPrimaryShardBalan } private void updateWeightFunction() { - weightFunction = new WeightFunction(this.indexBalanceFactor, this.shardBalanceFactor, this.preferPrimaryShardRebalanceBuffer); + weightFunction = new WeightFunction( + this.indexBalanceFactor, + this.shardBalanceFactor, + this.preferPrimaryShardRebalanceBuffer, + this.primaryConstraintThreshold + ); } /** @@ -317,6 +333,11 @@ private void setThreshold(float threshold) { this.threshold = threshold; } + private void setPrimaryConstraintThresholdSetting(long threshold) { + this.primaryConstraintThreshold = threshold; + this.weightFunction.updatePrimaryConstraintThreshold(threshold); + } + private void setAllocatorTimeout(TimeValue allocatorTimeout) { this.allocatorTimeout = allocatorTimeout; } @@ -489,10 +510,11 @@ static class WeightFunction { private final float shardBalance; private final float theta0; private final float theta1; + private long primaryConstraintThreshold; private AllocationConstraints constraints; private RebalanceConstraints rebalanceConstraints; - WeightFunction(float indexBalance, float shardBalance, float preferPrimaryBalanceBuffer) { + WeightFunction(float indexBalance, float shardBalance, float preferPrimaryBalanceBuffer, long primaryConstraintThreshold) { float sum = indexBalance + shardBalance; if (sum <= 0.0f) { throw new IllegalArgumentException("Balance factors must sum to a value > 0 but was: " + sum); @@ -501,6 +523,7 @@ static class WeightFunction { theta1 = indexBalance / sum; this.indexBalance = indexBalance; this.shardBalance = shardBalance; + this.primaryConstraintThreshold = primaryConstraintThreshold; RebalanceParameter rebalanceParameter = new RebalanceParameter(preferPrimaryBalanceBuffer); this.constraints = new AllocationConstraints(); this.rebalanceConstraints = new RebalanceConstraints(rebalanceParameter); @@ -510,12 +533,12 @@ static class WeightFunction { public float weightWithAllocationConstraints(ShardsBalancer balancer, ModelNode node, String index) { float balancerWeight = weight(balancer, node, index); - return balancerWeight + constraints.weight(balancer, node, index); + return balancerWeight + constraints.weight(balancer, node, index, primaryConstraintThreshold); } public float weightWithRebalanceConstraints(ShardsBalancer balancer, ModelNode node, String index) { float balancerWeight = weight(balancer, node, index); - return balancerWeight + rebalanceConstraints.weight(balancer, node, index); + return balancerWeight + rebalanceConstraints.weight(balancer, node, index, primaryConstraintThreshold); } float weight(ShardsBalancer balancer, ModelNode node, String index) { @@ -531,6 +554,10 @@ void updateAllocationConstraint(String constraint, boolean enable) { void updateRebalanceConstraint(String constraint, boolean add) { this.rebalanceConstraints.updateRebalanceConstraint(constraint, add); } + + void updatePrimaryConstraintThreshold(long primaryConstraintThreshold) { + this.primaryConstraintThreshold = primaryConstraintThreshold; + } } /** diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index f769f8729c25b..c1f4e52706465 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -276,6 +276,7 @@ public void apply(Settings value, Settings current, Settings previous) { BalancedShardsAllocator.THRESHOLD_SETTING, BalancedShardsAllocator.IGNORE_THROTTLE_FOR_REMOTE_RESTORE, BalancedShardsAllocator.ALLOCATOR_TIMEOUT_SETTING, + BalancedShardsAllocator.PRIMARY_CONSTRAINT_THRESHOLD_SETTING, BreakerSettings.CIRCUIT_BREAKER_LIMIT_SETTING, BreakerSettings.CIRCUIT_BREAKER_OVERHEAD_SETTING, BreakerSettings.CIRCUIT_BREAKER_TYPE, diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationConstraintsTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationConstraintsTests.java index 4c9fcd1650664..23d0b708441f8 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationConstraintsTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationConstraintsTests.java @@ -25,6 +25,8 @@ public class AllocationConstraintsTests extends OpenSearchAllocationTestCase { + long constraintWeight = 20L; + public void testSettings() { Settings.Builder settings = Settings.builder(); ClusterSettings service = new ClusterSettings(Settings.builder().build(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); @@ -69,7 +71,7 @@ public void testIndexShardsPerNodeConstraint() { when(node.getNodeId()).thenReturn("test-node"); long expectedWeight = (shardCount >= avgShardsPerNode) ? CONSTRAINT_WEIGHT : 0; - assertEquals(expectedWeight, constraints.weight(balancer, node, "index")); + assertEquals(expectedWeight, constraints.weight(balancer, node, "index", constraintWeight)); } @@ -91,14 +93,14 @@ public void testPerIndexPrimaryShardsConstraint() { when(node.numPrimaryShards(anyString())).thenReturn(perIndexPrimaryShardCount); when(node.getNodeId()).thenReturn("test-node"); - assertEquals(0, constraints.weight(balancer, node, indexName)); + assertEquals(0, constraints.weight(balancer, node, indexName, constraintWeight)); perIndexPrimaryShardCount = 2; when(node.numPrimaryShards(anyString())).thenReturn(perIndexPrimaryShardCount); - assertEquals(CONSTRAINT_WEIGHT, constraints.weight(balancer, node, indexName)); + assertEquals(CONSTRAINT_WEIGHT, constraints.weight(balancer, node, indexName, constraintWeight)); constraints.updateAllocationConstraint(INDEX_PRIMARY_SHARD_BALANCE_CONSTRAINT_ID, false); - assertEquals(0, constraints.weight(balancer, node, indexName)); + assertEquals(0, constraints.weight(balancer, node, indexName, constraintWeight)); } /** @@ -118,14 +120,14 @@ public void testGlobalPrimaryShardsConstraint() { when(node.numPrimaryShards()).thenReturn(primaryShardCount); when(node.getNodeId()).thenReturn("test-node"); - assertEquals(0, constraints.weight(balancer, node, indexName)); + assertEquals(0, constraints.weight(balancer, node, indexName, constraintWeight)); primaryShardCount = 3; when(node.numPrimaryShards()).thenReturn(primaryShardCount); - assertEquals(CONSTRAINT_WEIGHT, constraints.weight(balancer, node, indexName)); + assertEquals(constraintWeight, constraints.weight(balancer, node, indexName, constraintWeight)); constraints.updateAllocationConstraint(CLUSTER_PRIMARY_SHARD_BALANCE_CONSTRAINT_ID, false); - assertEquals(0, constraints.weight(balancer, node, indexName)); + assertEquals(0, constraints.weight(balancer, node, indexName, constraintWeight)); } /** @@ -150,22 +152,22 @@ public void testPrimaryShardsConstraints() { when(node.numPrimaryShards()).thenReturn(primaryShardCount); when(node.getNodeId()).thenReturn("test-node"); - assertEquals(0, constraints.weight(balancer, node, indexName)); + assertEquals(0, constraints.weight(balancer, node, indexName, constraintWeight)); // breaching global primary shard count but not per index primary shard count primaryShardCount = 5; when(node.numPrimaryShards()).thenReturn(primaryShardCount); - assertEquals(CONSTRAINT_WEIGHT, constraints.weight(balancer, node, indexName)); + assertEquals(constraintWeight, constraints.weight(balancer, node, indexName, constraintWeight)); // when per index primary shard count constraint is also breached perIndexPrimaryShardCount = 3; when(node.numPrimaryShards(indexName)).thenReturn(perIndexPrimaryShardCount); - assertEquals(2 * CONSTRAINT_WEIGHT, constraints.weight(balancer, node, indexName)); + assertEquals(CONSTRAINT_WEIGHT + constraintWeight, constraints.weight(balancer, node, indexName, constraintWeight)); // disable both constraints constraints.updateAllocationConstraint(INDEX_PRIMARY_SHARD_BALANCE_CONSTRAINT_ID, false); constraints.updateAllocationConstraint(CLUSTER_PRIMARY_SHARD_BALANCE_CONSTRAINT_ID, false); - assertEquals(0, constraints.weight(balancer, node, indexName)); + assertEquals(0, constraints.weight(balancer, node, indexName, constraintWeight)); } /** @@ -202,8 +204,8 @@ public void testAllConstraints() { long expectedWeight = (shardCount >= (int) Math.ceil(avgPerIndexShardsPerNode)) ? CONSTRAINT_WEIGHT : 0; expectedWeight += perIndexPrimaryShardCount > (int) Math.ceil(avgPerIndexPrimaryShardsPerNode) ? CONSTRAINT_WEIGHT : 0; - expectedWeight += primaryShardsPerNode >= (int) Math.ceil(avgPrimaryShardsPerNode) ? CONSTRAINT_WEIGHT : 0; - assertEquals(expectedWeight, constraints.weight(balancer, node, indexName)); + expectedWeight += primaryShardsPerNode >= (int) Math.ceil(avgPrimaryShardsPerNode) ? constraintWeight : 0; + assertEquals(expectedWeight, constraints.weight(balancer, node, indexName, constraintWeight)); } } From b2d537a01a368e11840582b656264f69cb06ed4d Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Sat, 26 Oct 2024 00:02:58 +0800 Subject: [PATCH 14/17] Update version check in yaml test file for the bug fix for get index settings API (#16480) Signed-off-by: Gao Binlong --- .../test/indices.get_settings/40_number_of_routing_shards.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml index 3fb392d6db134..3c86678a690b8 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_settings/40_number_of_routing_shards.yml @@ -22,8 +22,8 @@ setup: --- Test retrieval of number_routing_shards settings: - skip: - version: " - 2.99.99" - reason: "introduced in 3.0.0" # TODO: change it to 2.18.0 after backport to 2.x branch + version: " - 2.18.99" + reason: "introduced in 2.19.0" - do: indices.get_settings: flat_settings: true From 6f1b59e54bec41d40772f8571c7b65d4b523f8b1 Mon Sep 17 00:00:00 2001 From: Sumit Bansal Date: Sat, 26 Oct 2024 11:34:44 +0530 Subject: [PATCH 15/17] Add logic in master service to optimize performance and retain detailed logging for critical cluster operations. (#16421) Signed-off-by: Sumit Bansal Signed-off-by: shwetathareja Co-authored-by: shwetathareja --- CHANGELOG.md | 1 + .../cluster/service/MasterService.java | 32 ++++++++------ .../cluster/service/TaskBatcher.java | 34 +++++++++----- .../cluster/service/MasterServiceTests.java | 44 +++++++++---------- 4 files changed, 64 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e76d946866b..e20df483030d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - URI path filtering support in cluster stats API ([#15938](https://github.com/opensearch-project/OpenSearch/pull/15938)) - [Star Tree - Search] Add support for metric aggregations with/without term query ([15289](https://github.com/opensearch-project/OpenSearch/pull/15289)) - Add support for restoring from snapshot with search replicas ([#16111](https://github.com/opensearch-project/OpenSearch/pull/16111)) +- Add logic in master service to optimize performance and retain detailed logging for critical cluster operations. ([#14795](https://github.com/opensearch-project/OpenSearch/pull/14795)) - Add Setting to adjust the primary constraint weights ([#16471](https://github.com/opensearch-project/OpenSearch/pull/16471)) ### Dependencies diff --git a/server/src/main/java/org/opensearch/cluster/service/MasterService.java b/server/src/main/java/org/opensearch/cluster/service/MasterService.java index 713de8cdd0fda..455e7301a490d 100644 --- a/server/src/main/java/org/opensearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/MasterService.java @@ -299,33 +299,37 @@ public static boolean assertNotMasterUpdateThread(String reason) { } private void runTasks(TaskInputs taskInputs) { - final String longSummary = logger.isTraceEnabled() ? taskInputs.taskSummaryGenerator.apply(true) : ""; - final String shortSummary = taskInputs.taskSummaryGenerator.apply(false); + final String summary; + if (logger.isTraceEnabled()) { + summary = taskInputs.taskSummaryGenerator.apply(true); + } else { + summary = taskInputs.taskSummaryGenerator.apply(false); + } if (!lifecycle.started()) { - logger.debug("processing [{}]: ignoring, cluster-manager service not started", shortSummary); + logger.debug("processing [{}]: ignoring, cluster-manager service not started", summary); return; } if (logger.isTraceEnabled()) { - logger.trace("executing cluster state update for [{}]", longSummary); + logger.trace("executing cluster state update for [{}]", summary); } else { - logger.debug("executing cluster state update for [{}]", shortSummary); + logger.debug("executing cluster state update for [{}]", summary); } final ClusterState previousClusterState = state(); if (!previousClusterState.nodes().isLocalNodeElectedClusterManager() && taskInputs.runOnlyWhenClusterManager()) { - logger.debug("failing [{}]: local node is no longer cluster-manager", shortSummary); + logger.debug("failing [{}]: local node is no longer cluster-manager", summary); taskInputs.onNoLongerClusterManager(); return; } final long computationStartTime = threadPool.preciseRelativeTimeInNanos(); - final TaskOutputs taskOutputs = calculateTaskOutputs(taskInputs, previousClusterState, shortSummary); + final TaskOutputs taskOutputs = calculateTaskOutputs(taskInputs, previousClusterState, summary); taskOutputs.notifyFailedTasks(); final TimeValue computationTime = getTimeSince(computationStartTime); - logExecutionTime(computationTime, "compute cluster state update", shortSummary); + logExecutionTime(computationTime, "compute cluster state update", summary); clusterManagerMetrics.recordLatency( clusterManagerMetrics.clusterStateComputeHistogram, @@ -337,17 +341,17 @@ private void runTasks(TaskInputs taskInputs) { final long notificationStartTime = threadPool.preciseRelativeTimeInNanos(); taskOutputs.notifySuccessfulTasksOnUnchangedClusterState(); final TimeValue executionTime = getTimeSince(notificationStartTime); - logExecutionTime(executionTime, "notify listeners on unchanged cluster state", shortSummary); + logExecutionTime(executionTime, "notify listeners on unchanged cluster state", summary); } else { final ClusterState newClusterState = taskOutputs.newClusterState; if (logger.isTraceEnabled()) { - logger.trace("cluster state updated, source [{}]\n{}", longSummary, newClusterState); + logger.trace("cluster state updated, source [{}]\n{}", summary, newClusterState); } else { - logger.debug("cluster state updated, version [{}], source [{}]", newClusterState.version(), shortSummary); + logger.debug("cluster state updated, version [{}], source [{}]", newClusterState.version(), summary); } final long publicationStartTime = threadPool.preciseRelativeTimeInNanos(); try { - ClusterChangedEvent clusterChangedEvent = new ClusterChangedEvent(shortSummary, newClusterState, previousClusterState); + ClusterChangedEvent clusterChangedEvent = new ClusterChangedEvent(summary, newClusterState, previousClusterState); // new cluster state, notify all listeners final DiscoveryNodes.Delta nodesDelta = clusterChangedEvent.nodesDelta(); if (nodesDelta.hasChanges() && logger.isInfoEnabled()) { @@ -355,7 +359,7 @@ private void runTasks(TaskInputs taskInputs) { if (nodesDeltaSummary.length() > 0) { logger.info( "{}, term: {}, version: {}, delta: {}", - shortSummary, + summary, newClusterState.term(), newClusterState.version(), nodesDeltaSummary @@ -366,7 +370,7 @@ private void runTasks(TaskInputs taskInputs) { logger.debug("publishing cluster state version [{}]", newClusterState.version()); publish(clusterChangedEvent, taskOutputs, publicationStartTime); } catch (Exception e) { - handleException(shortSummary, publicationStartTime, newClusterState, e); + handleException(summary, publicationStartTime, newClusterState, e); } } } diff --git a/server/src/main/java/org/opensearch/cluster/service/TaskBatcher.java b/server/src/main/java/org/opensearch/cluster/service/TaskBatcher.java index 3513bfffb7157..ac54693b8ad1e 100644 --- a/server/src/main/java/org/opensearch/cluster/service/TaskBatcher.java +++ b/server/src/main/java/org/opensearch/cluster/service/TaskBatcher.java @@ -195,16 +195,12 @@ void runIfNotProcessed(BatchedTask updateTask) { if (toExecute.isEmpty() == false) { Function taskSummaryGenerator = (longSummaryRequired) -> { if (longSummaryRequired == null || !longSummaryRequired) { - return buildShortSummary(updateTask.batchingKey, toExecute.size()); + final List sampleTasks = toExecute.stream() + .limit(Math.min(1000, toExecute.size())) + .collect(Collectors.toList()); + return buildShortSummary(updateTask.batchingKey, toExecute.size(), getSummary(updateTask, sampleTasks)); } - final Map> processTasksBySource = new HashMap<>(); - for (final BatchedTask task : toExecute) { - processTasksBySource.computeIfAbsent(task.source, s -> new ArrayList<>()).add(task); - } - return processTasksBySource.entrySet().stream().map(entry -> { - String tasks = updateTask.describeTasks(entry.getValue()); - return tasks.isEmpty() ? entry.getKey() : entry.getKey() + "[" + tasks + "]"; - }).reduce((s1, s2) -> s1 + ", " + s2).orElse(""); + return getSummary(updateTask, toExecute); }; taskBatcherListener.onBeginProcessing(toExecute); run(updateTask.batchingKey, toExecute, taskSummaryGenerator); @@ -212,8 +208,24 @@ void runIfNotProcessed(BatchedTask updateTask) { } } - private String buildShortSummary(final Object batchingKey, final int taskCount) { - return "Tasks batched with key: " + batchingKey.toString().split("\\$")[0] + " and count: " + taskCount; + private String getSummary(final BatchedTask updateTask, final List toExecute) { + final Map> processTasksBySource = new HashMap<>(); + for (final BatchedTask task : toExecute) { + processTasksBySource.computeIfAbsent(task.source, s -> new ArrayList<>()).add(task); + } + return processTasksBySource.entrySet().stream().map(entry -> { + String tasks = updateTask.describeTasks(entry.getValue()); + return tasks.isEmpty() ? entry.getKey() : entry.getKey() + "[" + tasks + "]"; + }).reduce((s1, s2) -> s1 + ", " + s2).orElse(""); + } + + private String buildShortSummary(final Object batchingKey, final int taskCount, final String sampleTasks) { + return "Tasks batched with key: " + + batchingKey.toString().split("\\$")[0] + + ", count:" + + taskCount + + " and sample tasks: " + + sampleTasks; } /** diff --git a/server/src/test/java/org/opensearch/cluster/service/MasterServiceTests.java b/server/src/test/java/org/opensearch/cluster/service/MasterServiceTests.java index db9abe0310e40..bb9e34d93431f 100644 --- a/server/src/test/java/org/opensearch/cluster/service/MasterServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/service/MasterServiceTests.java @@ -391,7 +391,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test1 computation", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [1s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [1s] to compute cluster state update for [test1]" ) ); mockAppender.addExpectation( @@ -399,7 +399,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test1 notification", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [0s] to notify listeners on unchanged cluster state for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [0s] to notify listeners on unchanged cluster state for [test1]" ) ); @@ -416,7 +416,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test2 failure", MasterService.class.getCanonicalName(), Level.TRACE, - "failed to execute cluster state update (on version: [*], uuid: [*]) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]*" + "failed to execute cluster state update (on version: [*], uuid: [*]) for [test2]*" ) ); mockAppender.addExpectation( @@ -424,7 +424,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test2 computation", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [2s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [2s] to compute cluster state update for [test2]" ) ); mockAppender.addExpectation( @@ -432,7 +432,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test2 notification", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [0s] to notify listeners on unchanged cluster state for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [0s] to notify listeners on unchanged cluster state for [test2]" ) ); @@ -449,7 +449,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test3 computation", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [3s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [3s] to compute cluster state update for [test3]" ) ); mockAppender.addExpectation( @@ -457,7 +457,7 @@ public void testClusterStateUpdateLoggingWithTraceEnabled() throws Exception { "test3 notification", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [4s] to notify listeners on successful publication of cluster state (version: *, uuid: *) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [4s] to notify listeners on successful publication of cluster state (version: *, uuid: *) for [test3]" ) ); @@ -548,7 +548,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test1 start", MasterService.class.getCanonicalName(), Level.DEBUG, - "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test1]" ) ); mockAppender.addExpectation( @@ -556,7 +556,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test1 computation", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [1s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [1s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test1]" ) ); mockAppender.addExpectation( @@ -564,7 +564,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test1 notification", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [0s] to notify listeners on unchanged cluster state for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [0s] to notify listeners on unchanged cluster state for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test1]" ) ); @@ -573,7 +573,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test2 start", MasterService.class.getCanonicalName(), Level.DEBUG, - "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test2]" ) ); mockAppender.addExpectation( @@ -581,7 +581,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test2 failure", MasterService.class.getCanonicalName(), Level.DEBUG, - "failed to execute cluster state update (on version: [*], uuid: [*]) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]*" + "failed to execute cluster state update (on version: [*], uuid: [*]) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test2]*" ) ); mockAppender.addExpectation( @@ -589,7 +589,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test2 computation", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [2s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [2s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test2]" ) ); mockAppender.addExpectation( @@ -597,7 +597,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test2 notification", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [0s] to notify listeners on unchanged cluster state for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [0s] to notify listeners on unchanged cluster state for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test2]" ) ); @@ -606,7 +606,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test3 start", MasterService.class.getCanonicalName(), Level.DEBUG, - "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test3]" ) ); mockAppender.addExpectation( @@ -614,7 +614,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test3 computation", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [3s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [3s] to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test3]" ) ); mockAppender.addExpectation( @@ -622,7 +622,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test3 notification", MasterService.class.getCanonicalName(), Level.DEBUG, - "took [4s] to notify listeners on successful publication of cluster state (version: *, uuid: *) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "took [4s] to notify listeners on successful publication of cluster state (version: *, uuid: *) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test3]" ) ); @@ -631,7 +631,7 @@ public void testClusterStateUpdateLoggingWithDebugEnabled() throws Exception { "test4", MasterService.class.getCanonicalName(), Level.DEBUG, - "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "executing cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test4]" ) ); @@ -1238,7 +1238,7 @@ public void testLongClusterStateUpdateLogging() throws Exception { "test2", MasterService.class.getCanonicalName(), Level.WARN, - "*took [*], which is over [10s], to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "*took [*], which is over [10s], to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test2]" ) ); mockAppender.addExpectation( @@ -1246,7 +1246,7 @@ public void testLongClusterStateUpdateLogging() throws Exception { "test3", MasterService.class.getCanonicalName(), Level.WARN, - "*took [*], which is over [10s], to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "*took [*], which is over [10s], to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test3]" ) ); mockAppender.addExpectation( @@ -1254,7 +1254,7 @@ public void testLongClusterStateUpdateLogging() throws Exception { "test4", MasterService.class.getCanonicalName(), Level.WARN, - "*took [*], which is over [10s], to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]" + "*took [*], which is over [10s], to compute cluster state update for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test4]" ) ); mockAppender.addExpectation( @@ -1432,7 +1432,7 @@ public void testLongClusterStateUpdateLoggingForFailedPublication() throws Excep "test1 should log due to slow and failing publication", MasterService.class.getCanonicalName(), Level.WARN, - "took [*] and then failed to publish updated cluster state (version: *, uuid: *) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests and count: 1]:*" + "took [*] and then failed to publish updated cluster state (version: *, uuid: *) for [Tasks batched with key: org.opensearch.cluster.service.MasterServiceTests, count:1 and sample tasks: test1]:*" ) ); From 72559bf60c84a59908a0685db01d2ed5b1419111 Mon Sep 17 00:00:00 2001 From: rajiv-kv <157019998+rajiv-kv@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:46:04 +0530 Subject: [PATCH 16/17] create publication repos during join task execution (#16383) * create publication repos during join task Signed-off-by: Rajiv Kumar Vaidyanathan --- .../RemotePublicationConfigurationIT.java | 10 +- .../coordination/JoinTaskExecutor.java | 42 +++- .../remotestore/RemoteStoreNodeService.java | 2 +- .../coordination/JoinTaskExecutorTests.java | 233 +++++++++++++++++- 4 files changed, 272 insertions(+), 15 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemotePublicationConfigurationIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemotePublicationConfigurationIT.java index 98859f517aad4..57bf9eccbf5b4 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemotePublicationConfigurationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemotePublicationConfigurationIT.java @@ -14,6 +14,7 @@ import org.opensearch.remotemigration.MigrationBaseTestCase; import org.opensearch.remotestore.multipart.mocks.MockFsRepositoryPlugin; import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.repositories.fs.FsRepository; import org.opensearch.repositories.fs.ReloadableFsRepository; import org.opensearch.test.InternalSettingsPlugin; import org.opensearch.test.OpenSearchIntegTestCase; @@ -97,23 +98,26 @@ public Settings.Builder remotePublishConfiguredNodeSetting() { .put(stateRepoSettingsAttributeKeyPrefix + prefixModeVerificationSuffix, true) .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, ROUTING_TABLE_REPO_NAME) - .put(routingTableRepoTypeAttributeKey, ReloadableFsRepository.TYPE) + .put(routingTableRepoTypeAttributeKey, FsRepository.TYPE) .put(routingTableRepoSettingsAttributeKeyPrefix + "location", segmentRepoPath); return builder; } public Settings.Builder remoteWithRoutingTableNodeSetting() { // Remote Cluster with Routing table + return Settings.builder() .put( - buildRemoteStoreNodeAttributes( + remoteStoreClusterSettings( REPOSITORY_NAME, segmentRepoPath, + ReloadableFsRepository.TYPE, REPOSITORY_2_NAME, translogRepoPath, + ReloadableFsRepository.TYPE, REPOSITORY_NAME, segmentRepoPath, - false + ReloadableFsRepository.TYPE ) ) .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true); diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index 13033b670d44b..d597b51c32ccd 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -42,6 +42,7 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.metadata.RepositoriesMetadata; +import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.RerouteService; @@ -57,6 +58,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -185,11 +187,30 @@ public ClusterTasksResult execute(ClusterState currentState, List jo // for every set of node join task which we can optimize to not compute if cluster state already has // repository information. Optional remoteDN = currentNodes.getNodes().values().stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); - DiscoveryNode dn = remoteDN.orElseGet(() -> (currentNodes.getNodes().values()).stream().findFirst().get()); - RepositoriesMetadata repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( - dn, - currentState.getMetadata().custom(RepositoriesMetadata.TYPE) - ); + Optional remotePublicationDN = currentNodes.getNodes() + .values() + .stream() + .filter(DiscoveryNode::isRemoteStatePublicationEnabled) + .findFirst(); + RepositoriesMetadata existingRepositoriesMetadata = currentState.getMetadata().custom(RepositoriesMetadata.TYPE); + Map repositories = new LinkedHashMap<>(); + if (existingRepositoriesMetadata != null) { + existingRepositoriesMetadata.repositories().forEach(r -> repositories.putIfAbsent(r.name(), r)); + } + if (remoteDN.isPresent()) { + RepositoriesMetadata repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( + remoteDN.get(), + existingRepositoriesMetadata + ); + repositoriesMetadata.repositories().forEach(r -> repositories.putIfAbsent(r.name(), r)); + } + if (remotePublicationDN.isPresent()) { + RepositoriesMetadata repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( + remotePublicationDN.get(), + existingRepositoriesMetadata + ); + repositoriesMetadata.repositories().forEach(r -> repositories.putIfAbsent(r.name(), r)); + } assert nodesBuilder.isLocalNodeElectedClusterManager(); @@ -219,15 +240,16 @@ public ClusterTasksResult execute(ClusterState currentState, List jo ensureNodeCommissioned(node, currentState.metadata()); nodesBuilder.add(node); - if (remoteDN.isEmpty() && node.isRemoteStoreNode()) { + if ((remoteDN.isEmpty() && node.isRemoteStoreNode()) + || (remotePublicationDN.isEmpty() && node.isRemoteStatePublicationEnabled())) { // This is hit only on cases where we encounter first remote node logger.info("Updating system repository now for remote store"); - repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( + RepositoriesMetadata repositoriesMetadata = remoteStoreNodeService.updateRepositoriesMetadata( node, - currentState.getMetadata().custom(RepositoriesMetadata.TYPE) + existingRepositoriesMetadata ); + repositoriesMetadata.repositories().forEach(r -> repositories.putIfAbsent(r.name(), r)); } - nodesChanged = true; minClusterNodeVersion = Version.min(minClusterNodeVersion, node.getVersion()); maxClusterNodeVersion = Version.max(maxClusterNodeVersion, node.getVersion()); @@ -241,7 +263,7 @@ public ClusterTasksResult execute(ClusterState currentState, List jo } results.success(joinTask); } - + RepositoriesMetadata repositoriesMetadata = new RepositoriesMetadata(new ArrayList<>(repositories.values())); if (nodesChanged) { rerouteService.reroute( "post-join reroute", diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java index cc5d8b0e62e90..c1c041ce01198 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java @@ -168,7 +168,7 @@ public void createAndVerifyRepositories(DiscoveryNode localNode) { * node repository metadata an exception will be thrown and the node will not be allowed to join the cluster. */ public RepositoriesMetadata updateRepositoriesMetadata(DiscoveryNode joiningNode, RepositoriesMetadata existingRepositories) { - if (joiningNode.isRemoteStoreNode()) { + if (joiningNode.isRemoteStoreNode() || joiningNode.isRemoteStatePublicationEnabled()) { List updatedRepositoryMetadataList = new ArrayList<>(); List newRepositoryMetadataList = new RemoteStoreNodeAttribute(joiningNode).getRepositoriesMetadata() .repositories(); diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index d192a2556c36b..f6fb203bfe1a9 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -959,6 +959,198 @@ public void testUpdatesClusterStateWithMultiNodeClusterAndSameRepository() throw validateRepositoryMetadata(result.resultingState, clusterManagerNode, 2); } + public void testUpdatesRepoRemoteNodeJoinPublicationCluster() throws Exception { + final AllocationService allocationService = mock(AllocationService.class); + when(allocationService.adaptAutoExpandReplicas(any())).then(invocationOnMock -> invocationOnMock.getArguments()[0]); + final RerouteService rerouteService = (reason, priority, listener) -> listener.onResponse(null); + final RemoteStoreNodeService remoteStoreNodeService = new RemoteStoreNodeService( + new SetOnce<>(mock(RepositoriesService.class))::get, + null + ); + + final JoinTaskExecutor joinTaskExecutor = new JoinTaskExecutor( + Settings.EMPTY, + allocationService, + logger, + rerouteService, + remoteStoreNodeService + ); + + final DiscoveryNode clusterManagerNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remotePublicationNodeAttributes(), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) + .nodes( + DiscoveryNodes.builder() + .add(clusterManagerNode) + .localNodeId(clusterManagerNode.getId()) + .clusterManagerNodeId(clusterManagerNode.getId()) + ) + .build(); + + final ClusterStateTaskExecutor.ClusterTasksResult result = joinTaskExecutor.execute( + clusterState, + List.of(new JoinTaskExecutor.Task(clusterManagerNode, "elect leader")) + ); + assertThat(result.executionResults.entrySet(), hasSize(1)); + final ClusterStateTaskExecutor.TaskResult taskResult = result.executionResults.values().iterator().next(); + assertTrue(taskResult.isSuccess()); + validatePublicationRepositoryMetadata(result.resultingState, clusterManagerNode); + + final Settings settings = Settings.builder() + .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed") + .build(); + final Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + + ClusterState currentState = ClusterState.builder(result.resultingState).metadata(metadata).build(); + + final DiscoveryNode remoteStoreNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final ClusterStateTaskExecutor.ClusterTasksResult resultAfterRemoteNodeJoin = joinTaskExecutor.execute( + currentState, + List.of(new JoinTaskExecutor.Task(remoteStoreNode, "test")) + ); + assertThat(resultAfterRemoteNodeJoin.executionResults.entrySet(), hasSize(1)); + final ClusterStateTaskExecutor.TaskResult taskResult1 = resultAfterRemoteNodeJoin.executionResults.values().iterator().next(); + assertTrue(taskResult1.isSuccess()); + validateRepositoriesMetadata(resultAfterRemoteNodeJoin.resultingState, remoteStoreNode, clusterManagerNode); + } + + public void testUpdatesRepoPublicationNodeJoinRemoteCluster() throws Exception { + final AllocationService allocationService = mock(AllocationService.class); + when(allocationService.adaptAutoExpandReplicas(any())).then(invocationOnMock -> invocationOnMock.getArguments()[0]); + final RerouteService rerouteService = (reason, priority, listener) -> listener.onResponse(null); + final RemoteStoreNodeService remoteStoreNodeService = new RemoteStoreNodeService( + new SetOnce<>(mock(RepositoriesService.class))::get, + null + ); + + final JoinTaskExecutor joinTaskExecutor = new JoinTaskExecutor( + Settings.EMPTY, + allocationService, + logger, + rerouteService, + remoteStoreNodeService + ); + + final DiscoveryNode clusterManagerNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) + .nodes( + DiscoveryNodes.builder() + .add(clusterManagerNode) + .localNodeId(clusterManagerNode.getId()) + .clusterManagerNodeId(clusterManagerNode.getId()) + ) + .build(); + + final ClusterStateTaskExecutor.ClusterTasksResult result = joinTaskExecutor.execute( + clusterState, + List.of(new JoinTaskExecutor.Task(clusterManagerNode, "elect leader")) + ); + final Settings settings = Settings.builder() + .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed") + .build(); + final Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + + ClusterState currentState = ClusterState.builder(result.resultingState).metadata(metadata).build(); + + final DiscoveryNode remotePublicationNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remotePublicationNodeAttributes(), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final ClusterStateTaskExecutor.ClusterTasksResult resultAfterRemoteNodeJoin = joinTaskExecutor.execute( + currentState, + List.of(new JoinTaskExecutor.Task(remotePublicationNode, "test")) + ); + assertThat(resultAfterRemoteNodeJoin.executionResults.entrySet(), hasSize(1)); + final ClusterStateTaskExecutor.TaskResult taskResult1 = resultAfterRemoteNodeJoin.executionResults.values().iterator().next(); + assertTrue(taskResult1.isSuccess()); + validateRepositoriesMetadata(resultAfterRemoteNodeJoin.resultingState, clusterManagerNode, remotePublicationNode); + } + + public void testUpdatesClusterStateWithMultiplePublicationNodeJoin() throws Exception { + Map remoteStoreNodeAttributes = remotePublicationNodeAttributes(); + final AllocationService allocationService = mock(AllocationService.class); + when(allocationService.adaptAutoExpandReplicas(any())).then(invocationOnMock -> invocationOnMock.getArguments()[0]); + final RerouteService rerouteService = (reason, priority, listener) -> listener.onResponse(null); + final RemoteStoreNodeService remoteStoreNodeService = new RemoteStoreNodeService( + new SetOnce<>(mock(RepositoriesService.class))::get, + null + ); + + final JoinTaskExecutor joinTaskExecutor = new JoinTaskExecutor( + Settings.EMPTY, + allocationService, + logger, + rerouteService, + remoteStoreNodeService + ); + + final DiscoveryNode clusterManagerNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + List repositoriesMetadata = new ArrayList<>(); + + final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) + .nodes( + DiscoveryNodes.builder() + .add(clusterManagerNode) + .localNodeId(clusterManagerNode.getId()) + .clusterManagerNodeId(clusterManagerNode.getId()) + ) + .metadata(Metadata.builder().putCustom(RepositoriesMetadata.TYPE, new RepositoriesMetadata(repositoriesMetadata))) + .build(); + + final DiscoveryNode joiningNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final ClusterStateTaskExecutor.ClusterTasksResult result = joinTaskExecutor.execute( + clusterState, + List.of(new JoinTaskExecutor.Task(joiningNode, "test")) + ); + assertThat(result.executionResults.entrySet(), hasSize(1)); + final ClusterStateTaskExecutor.TaskResult taskResult = result.executionResults.values().iterator().next(); + assertTrue(taskResult.isSuccess()); + validatePublicationRepositoryMetadata(result.resultingState, clusterManagerNode); + } + public void testNodeJoinInMixedMode() { List versions = allOpenSearchVersions(); assert versions.size() >= 2 : "test requires at least two open search versions"; @@ -1191,7 +1383,9 @@ private void validateRepositoryMetadata(ClusterState updatedState, DiscoveryNode final RepositoriesMetadata repositoriesMetadata = updatedState.metadata().custom(RepositoriesMetadata.TYPE); assertTrue(repositoriesMetadata.repositories().size() == expectedRepositories); - if (repositoriesMetadata.repositories().size() == 2 || repositoriesMetadata.repositories().size() == 3) { + if (repositoriesMetadata.repositories().size() == 2 + || repositoriesMetadata.repositories().size() == 3 + || repositoriesMetadata.repositories().size() == 4) { final RepositoryMetadata segmentRepositoryMetadata = buildRepositoryMetadata(existingNode, SEGMENT_REPO); final RepositoryMetadata translogRepositoryMetadata = buildRepositoryMetadata(existingNode, TRANSLOG_REPO); for (RepositoryMetadata repositoryMetadata : repositoriesMetadata.repositories()) { @@ -1212,6 +1406,43 @@ private void validateRepositoryMetadata(ClusterState updatedState, DiscoveryNode } } + private void validatePublicationRepositoryMetadata(ClusterState updatedState, DiscoveryNode existingNode) throws Exception { + final RepositoriesMetadata repositoriesMetadata = updatedState.metadata().custom(RepositoriesMetadata.TYPE); + assertTrue(repositoriesMetadata.repositories().size() == 2); + final RepositoryMetadata clusterStateRepoMetadata = buildRepositoryMetadata(existingNode, CLUSTER_STATE_REPO); + final RepositoryMetadata routingTableRepoMetadata = buildRepositoryMetadata(existingNode, ROUTING_TABLE_REPO); + for (RepositoryMetadata repositoryMetadata : repositoriesMetadata.repositories()) { + if (repositoryMetadata.name().equals(clusterStateRepoMetadata.name())) { + assertTrue(clusterStateRepoMetadata.equalsIgnoreGenerations(repositoryMetadata)); + } else if (repositoryMetadata.name().equals(routingTableRepoMetadata.name())) { + assertTrue(routingTableRepoMetadata.equalsIgnoreGenerations(repositoryMetadata)); + } + } + } + + private void validateRepositoriesMetadata(ClusterState updatedState, DiscoveryNode remoteNode, DiscoveryNode publicationNode) + throws Exception { + + final RepositoriesMetadata repositoriesMetadata = updatedState.metadata().custom(RepositoriesMetadata.TYPE); + assertEquals(4, repositoriesMetadata.repositories().size()); + final RepositoryMetadata segmentRepositoryMetadata = buildRepositoryMetadata(remoteNode, SEGMENT_REPO); + final RepositoryMetadata translogRepositoryMetadata = buildRepositoryMetadata(remoteNode, TRANSLOG_REPO); + final RepositoryMetadata clusterStateRepositoryMetadata = buildRepositoryMetadata(remoteNode, CLUSTER_STATE_REPO); + final RepositoryMetadata routingTableRepositoryMetadata = buildRepositoryMetadata(publicationNode, ROUTING_TABLE_REPO); + + for (RepositoryMetadata repositoryMetadata : repositoriesMetadata.repositories()) { + if (repositoryMetadata.name().equals(segmentRepositoryMetadata.name())) { + assertTrue(segmentRepositoryMetadata.equalsIgnoreGenerations(repositoryMetadata)); + } else if (repositoryMetadata.name().equals(translogRepositoryMetadata.name())) { + assertTrue(translogRepositoryMetadata.equalsIgnoreGenerations(repositoryMetadata)); + } else if (repositoryMetadata.name().equals(clusterStateRepositoryMetadata.name())) { + assertTrue(clusterStateRepositoryMetadata.equalsIgnoreGenerations(repositoryMetadata)); + } else if (repositoryMetadata.name().equals(routingTableRepositoryMetadata.name())) { + assertTrue(repositoryMetadata.equalsIgnoreGenerations(routingTableRepositoryMetadata)); + } + } + } + private DiscoveryNode newDiscoveryNode(Map attributes) { return new DiscoveryNode( randomAlphaOfLength(10), From 0fcb3ab82e0e2373f79287dd035c3b5e2a5fa306 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 11:15:44 -0400 Subject: [PATCH 17/17] Bump ch.qos.logback:logback-core from 1.5.10 to 1.5.12 in /test/fixtures/hdfs-fixture (#16503) * Bump ch.qos.logback:logback-core in /test/fixtures/hdfs-fixture Bumps [ch.qos.logback:logback-core](https://github.com/qos-ch/logback) from 1.5.10 to 1.5.12. - [Commits](https://github.com/qos-ch/logback/compare/v_1.5.10...v_1.5.12) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-core dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * Update changelog Signed-off-by: dependabot[bot] --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] --- CHANGELOG.md | 2 +- test/fixtures/hdfs-fixture/build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e20df483030d6..c1e4f459d1422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `peter-evans/create-pull-request` from 6 to 7 ([#15863](https://github.com/opensearch-project/OpenSearch/pull/15863)) - Bump `com.nimbusds:oauth2-oidc-sdk` from 11.9.1 to 11.19.1 ([#15862](https://github.com/opensearch-project/OpenSearch/pull/15862)) - Bump `com.microsoft.azure:msal4j` from 1.17.0 to 1.17.2 ([#15945](https://github.com/opensearch-project/OpenSearch/pull/15945), [#16406](https://github.com/opensearch-project/OpenSearch/pull/16406)) -- Bump `ch.qos.logback:logback-core` from 1.5.6 to 1.5.10 ([#15946](https://github.com/opensearch-project/OpenSearch/pull/15946), [#16307](https://github.com/opensearch-project/OpenSearch/pull/16307)) +- Bump `ch.qos.logback:logback-core` from 1.5.6 to 1.5.12 ([#15946](https://github.com/opensearch-project/OpenSearch/pull/15946), [#16307](https://github.com/opensearch-project/OpenSearch/pull/16307), [#16503](https://github.com/opensearch-project/OpenSearch/pull/16503)) - Update protobuf from 3.25.4 to 3.25.5 ([#16011](https://github.com/opensearch-project/OpenSearch/pull/16011)) - Bump `org.roaringbitmap:RoaringBitmap` from 1.2.1 to 1.3.0 ([#16040](https://github.com/opensearch-project/OpenSearch/pull/16040)) - Bump `com.nimbusds:nimbus-jose-jwt` from 9.40 to 9.41.1 ([#16038](https://github.com/opensearch-project/OpenSearch/pull/16038)) diff --git a/test/fixtures/hdfs-fixture/build.gradle b/test/fixtures/hdfs-fixture/build.gradle index 2bd0268ca136b..d27273f357758 100644 --- a/test/fixtures/hdfs-fixture/build.gradle +++ b/test/fixtures/hdfs-fixture/build.gradle @@ -74,7 +74,7 @@ dependencies { api 'org.apache.zookeeper:zookeeper:3.9.2' api "org.apache.commons:commons-text:1.12.0" api "commons-net:commons-net:3.11.1" - api "ch.qos.logback:logback-core:1.5.10" + api "ch.qos.logback:logback-core:1.5.12" api "ch.qos.logback:logback-classic:1.2.13" api "org.jboss.xnio:xnio-nio:3.8.16.Final" api 'org.jline:jline:3.27.1'