From 09e9ffae4f8ce69d60531659d7ff2770e0993672 Mon Sep 17 00:00:00 2001 From: Bukhtawar Khan Date: Sat, 2 Sep 2023 15:31:54 +0530 Subject: [PATCH] Empty replication leases on replica Signed-off-by: Bukhtawar Khan --- .../index/seqno/RetentionLeaseIT.java | 70 ++++++++++++++----- .../test/OpenSearchIntegTestCase.java | 7 ++ 2 files changed, 59 insertions(+), 18 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/index/seqno/RetentionLeaseIT.java b/server/src/internalClusterTest/java/org/opensearch/index/seqno/RetentionLeaseIT.java index 6163edada9f6e..a7b155684bab5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/seqno/RetentionLeaseIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/seqno/RetentionLeaseIT.java @@ -41,6 +41,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.util.CollectionUtils; import org.opensearch.core.index.shard.ShardId; import org.opensearch.index.IndexService; import org.opensearch.index.IndexSettings; @@ -70,12 +71,12 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.opensearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_NETWORK_SETTING; -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_NETWORK_SETTING; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST) public class RetentionLeaseIT extends OpenSearchIntegTestCase { @@ -141,13 +142,22 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { final Map retentionLeasesOnReplica = RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases( replica.getRetentionLeases() ); - assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); + if (isIndexRemoteStoreEnabled("index")) { + assertThat(retentionLeasesOnReplica, equalTo(Collections.EMPTY_MAP)); + } else { + assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); + } // check retention leases have been written on the replica - assertThat( - currentRetentionLeases, - equalTo(RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases())) - ); + if (isIndexRemoteStoreEnabled("index")) { + assertThat( + RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases()), equalTo(Collections.EMPTY_MAP) + ); + } else { + assertThat( + RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases()), equalTo(currentRetentionLeases) + ); + } } } } @@ -205,13 +215,22 @@ public void testRetentionLeaseSyncedOnRemove() throws Exception { final Map retentionLeasesOnReplica = RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases( replica.getRetentionLeases() ); - assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); + if (isIndexRemoteStoreEnabled("index")) { + assertThat(retentionLeasesOnReplica, equalTo(Collections.EMPTY_MAP)); + } else { + assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); + } // check retention leases have been written on the replica - assertThat( - currentRetentionLeases, - equalTo(RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases())) - ); + if (isIndexRemoteStoreEnabled("index")) { + assertThat( + RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases()), equalTo(Collections.EMPTY_MAP) + ); + } else { + assertThat( + RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases()), equalTo(currentRetentionLeases) + ); + } } } } @@ -352,7 +371,11 @@ public void testBackgroundRetentionLeaseSync() throws Exception { final String replicaShardNodeName = clusterService().state().nodes().get(replicaShardNodeId).getName(); final IndexShard replica = internalCluster().getInstance(IndicesService.class, replicaShardNodeName) .getShardOrNull(new ShardId(resolveIndex("index"), 0)); - assertThat(replica.getRetentionLeases(), equalTo(primary.getRetentionLeases())); + if(isIndexRemoteStoreEnabled("index")) { + assertThat(replica.getRetentionLeases(), equalTo(new RetentionLeases(primary.getOperationPrimaryTerm(), 0, new ArrayList<>()))); + } else { + assertThat(replica.getRetentionLeases(), equalTo(primary.getRetentionLeases())); + } } }); } @@ -444,13 +467,24 @@ public void testRetentionLeasesSyncOnRecovery() throws Exception { final Map retentionLeasesOnReplica = RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases( replica.getRetentionLeases() ); - assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); + if(isIndexRemoteStoreEnabled("index")) { + assertThat(retentionLeasesOnReplica, equalTo(Collections.EMPTY_MAP)); + } else { + assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); + } // check retention leases have been written on the replica; see RecoveryTarget#finalizeRecovery - assertThat( - currentRetentionLeases, - equalTo(RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases())) - ); + if(isIndexRemoteStoreEnabled("index")) { + assertThat( + RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases()), + equalTo(Collections.EMPTY_MAP) + ); + } else { + assertThat( + RetentionLeaseUtils.toMapExcludingPeerRecoveryRetentionLeases(replica.loadRetentionLeases()), + equalTo(currentRetentionLeases) + ); + } } } diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index e6caeee7dd714..d7dc898ba550c 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -64,6 +64,7 @@ import org.opensearch.action.admin.indices.segments.IndexShardSegments; import org.opensearch.action.admin.indices.segments.IndicesSegmentResponse; import org.opensearch.action.admin.indices.segments.ShardSegments; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; import org.opensearch.action.admin.indices.template.put.PutIndexTemplateRequestBuilder; import org.opensearch.action.bulk.BulkRequestBuilder; import org.opensearch.action.bulk.BulkResponse; @@ -203,6 +204,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; @@ -2598,4 +2600,9 @@ protected ClusterState getClusterState() { return client(internalCluster().getClusterManagerName()).admin().cluster().prepareState().get().getState(); } + protected boolean isIndexRemoteStoreEnabled(String index) throws Exception { + return client().admin().indices().getSettings(new GetSettingsRequest().indices("index")).get() + .getSetting("index", IndexMetadata.SETTING_REMOTE_STORE_ENABLED).equals(Boolean.TRUE.toString()); + } + }