From b987c55724bafee85ff910f10b600de5ffa95797 Mon Sep 17 00:00:00 2001 From: Gaurav Bafna Date: Fri, 27 Oct 2023 22:43:03 +0530 Subject: [PATCH] PR comments and fixing clone IT Signed-off-by: Gaurav Bafna --- .../indices/create/RemoteCloneIndexIT.java | 25 +------------------ .../opensearch/index/shard/IndexShard.java | 6 ++--- .../opensearch/index/shard/StoreRecovery.java | 6 ++--- .../index/shard/IndexShardTests.java | 2 +- 4 files changed, 8 insertions(+), 31 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/RemoteCloneIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/RemoteCloneIndexIT.java index 4821673c65058..a081110e6c5a1 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/RemoteCloneIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/RemoteCloneIndexIT.java @@ -47,7 +47,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.index.query.TermsQueryBuilder; -import org.opensearch.index.seqno.SeqNoStats; import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase; import org.opensearch.test.VersionUtils; @@ -91,16 +90,12 @@ public void testCreateCloneIndex() { .setTransientSettings(Settings.builder().put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), "none")) .get(); try { - - final boolean createWithReplicas = randomBoolean(); assertAcked( client().admin() .indices() .prepareResizeIndex("source", "target") .setResizeType(ResizeType.CLONE) - .setSettings( - Settings.builder().put("index.number_of_replicas", createWithReplicas ? 1 : 0).putNull("index.blocks.write").build() - ) + .setSettings(Settings.builder().put("index.number_of_replicas", 0).putNull("index.blocks.write").build()) .get() ); ensureGreen(); @@ -108,27 +103,9 @@ public void testCreateCloneIndex() { final IndicesStatsResponse targetStats = client().admin().indices().prepareStats("target").get(); assertThat(targetStats.getIndex("target").getIndexShards().keySet().size(), equalTo(numPrimaryShards)); - for (int i = 0; i < numPrimaryShards; i++) { - final SeqNoStats sourceSeqNoStats = sourceStats.getIndex("source").getIndexShards().get(i).getAt(0).getSeqNoStats(); - final SeqNoStats targetSeqNoStats = targetStats.getIndex("target").getIndexShards().get(i).getAt(0).getSeqNoStats(); - assertEquals(sourceSeqNoStats.getMaxSeqNo(), targetSeqNoStats.getMaxSeqNo()); - assertEquals(targetSeqNoStats.getMaxSeqNo(), targetSeqNoStats.getLocalCheckpoint()); - } - final int size = docs > 0 ? 2 * docs : 1; assertHitCount(client().prepareSearch("target").setSize(size).setQuery(new TermsQueryBuilder("foo", "bar")).get(), docs); - if (createWithReplicas == false) { - // bump replicas - client().admin() - .indices() - .prepareUpdateSettings("target") - .setSettings(Settings.builder().put("index.number_of_replicas", 1)) - .get(); - ensureGreen(); - assertHitCount(client().prepareSearch("target").setSize(size).setQuery(new TermsQueryBuilder("foo", "bar")).get(), docs); - } - for (int i = docs; i < 2 * docs; i++) { client().prepareIndex("target").setSource("{\"foo\" : \"bar\", \"i\" : " + i + "}", MediaTypeRegistry.JSON).get(); } diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index faa9a6fce9095..b17ddeb722bd0 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -1998,10 +1998,10 @@ private RemoteSegmentStoreDirectory getRemoteDirectory() { } /** - Returns true iff it is able to verify that there is at least one - remote segment metadata uploaded + Returns true iff it is able to verify that remote segment store + is in sync with local */ - boolean isRemoteSync() { + boolean isRemoteSegmentStoreInSync() { assert indexSettings.isRemoteStoreEnabled(); try { RemoteSegmentStoreDirectory directory = getRemoteDirectory(); diff --git a/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java b/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java index 8c2dc669de4ed..e823401e5ef7e 100644 --- a/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java +++ b/server/src/main/java/org/opensearch/index/shard/StoreRecovery.java @@ -192,7 +192,7 @@ void recoverFromLocalShards( // copied segments - we will also see them in stats etc. indexShard.getEngine().forceMerge(false, -1, false, false, false, UUIDs.randomBase64UUID()); if (indexShard.isRemoteTranslogEnabled()) { - if (indexShard.isRemoteSync() == false) { + if (indexShard.isRemoteSegmentStoreInSync() == false) { throw new IndexShardRecoveryException( indexShard.shardId(), "failed to upload to remote", @@ -428,7 +428,7 @@ void recoverFromSnapshotAndRemoteStore( indexShard.getEngine().fillSeqNoGaps(indexShard.getPendingPrimaryTerm()); indexShard.finalizeRecovery(); if (indexShard.isRemoteTranslogEnabled()) { - if (indexShard.isRemoteSync() == false) { + if (indexShard.isRemoteSegmentStoreInSync() == false) { listener.onFailure(new IndexShardRestoreFailedException(shardId, "Failed to upload to remote segment store")); return; } @@ -713,7 +713,7 @@ private void restore( indexShard.getEngine().fillSeqNoGaps(indexShard.getPendingPrimaryTerm()); indexShard.finalizeRecovery(); if (indexShard.isRemoteTranslogEnabled()) { - if (indexShard.isRemoteSync() == false) { + if (indexShard.isRemoteSegmentStoreInSync() == false) { listener.onFailure(new IndexShardRestoreFailedException(shardId, "Failed to upload to remote segment store")); return; } diff --git a/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java b/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java index cf5d11c677085..dc2111fdcfc56 100644 --- a/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java @@ -2850,7 +2850,7 @@ public void testSyncSegmentsFromGivenRemoteSegmentStore() throws IOException { indexDoc(source, "_doc", "1"); indexDoc(source, "_doc", "2"); source.refresh("test"); - assertTrue("At lease one remote sync should have been completed", source.isRemoteSync()); + assertTrue("At lease one remote sync should have been completed", source.isRemoteSegmentStoreInSync()); assertDocs(source, "1", "2"); indexDoc(source, "_doc", "3"); source.refresh("test");