From bd569bebad373a7cfeba5a6a9aa16c8f89159064 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Wed, 14 Aug 2024 15:08:06 +0530 Subject: [PATCH] Address PR comments Signed-off-by: Sachin Kale --- .../remotestore/RemoteStorePinnedTimestampsIT.java | 8 -------- server/src/main/java/org/opensearch/node/Node.java | 13 ++++++++----- .../RemoteStorePinnedTimestampService.java | 6 +----- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsIT.java index 8dceceb82cdfb..0bb53309f7a78 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsIT.java @@ -9,7 +9,6 @@ package org.opensearch.remotestore; import org.opensearch.common.collect.Tuple; -import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; import org.opensearch.node.remotestore.RemoteStorePinnedTimestampService; @@ -17,8 +16,6 @@ import java.util.Set; -import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; - @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class RemoteStorePinnedTimestampsIT extends RemoteStoreBaseIntegTestCase { static final String INDEX_NAME = "remote-store-test-idx-1"; @@ -31,11 +28,6 @@ public void onResponse(Void unused) {} public void onFailure(Exception e) {} }; - @Override - protected Settings nodeSettings(int nodeOrdinal) { - return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true).build(); - } - public void testTimestampPinUnpin() throws Exception { prepareCluster(1, 1, INDEX_NAME, 0, 2); ensureGreen(INDEX_NAME); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 9c4627d5bef38..8324d3b219d3f 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -785,7 +785,6 @@ protected Node( final RemoteClusterStateService remoteClusterStateService; final RemoteClusterStateCleanupManager remoteClusterStateCleanupManager; final RemoteIndexPathUploader remoteIndexPathUploader; - final RemoteStorePinnedTimestampService remoteStorePinnedTimestampService; if (isRemoteStoreClusterStateEnabled(settings)) { remoteIndexPathUploader = new RemoteIndexPathUploader( threadPool, @@ -803,6 +802,14 @@ protected Node( List.of(remoteIndexPathUploader), namedWriteableRegistry ); + remoteClusterStateCleanupManager = remoteClusterStateService.getCleanupManager(); + } else { + remoteClusterStateService = null; + remoteIndexPathUploader = null; + remoteClusterStateCleanupManager = null; + } + final RemoteStorePinnedTimestampService remoteStorePinnedTimestampService; + if (isRemoteStoreAttributePresent(settings)) { remoteStorePinnedTimestampService = new RemoteStorePinnedTimestampService( repositoriesServiceReference::get, settings, @@ -810,11 +817,7 @@ protected Node( clusterService ); resourcesToClose.add(remoteStorePinnedTimestampService); - remoteClusterStateCleanupManager = remoteClusterStateService.getCleanupManager(); } else { - remoteClusterStateService = null; - remoteIndexPathUploader = null; - remoteClusterStateCleanupManager = null; remoteStorePinnedTimestampService = null; } diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java index 57cf8e5495529..8723e55a6e52f 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStorePinnedTimestampService.java @@ -40,8 +40,6 @@ import java.util.function.Supplier; import java.util.stream.Collectors; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled; - /** * Service for managing pinned timestamps in a remote store. * This service handles pinning and unpinning of timestamps, as well as periodic updates of the pinned timestamps set. @@ -100,9 +98,8 @@ public void start() { } private void validateRemoteStoreConfiguration() { - assert isRemoteStoreClusterStateEnabled(settings) : "Remote cluster state is not enabled"; final String remoteStoreRepo = settings.get( - Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY + Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY ); assert remoteStoreRepo != null : "Remote Cluster State repository is not configured"; final Repository repository = repositoriesService.get().repository(remoteStoreRepo); @@ -132,7 +129,6 @@ private void startAsyncUpdateTask() { * @param timestamp The timestamp to be pinned * @param pinningEntity The entity responsible for pinning the timestamp * @param listener A listener to be notified when the pinning operation completes - * @throws IOException If an I/O error occurs during the pinning process * @throws IllegalArgumentException If the timestamp is less than the current time minus one second */ public void pinTimestamp(long timestamp, String pinningEntity, ActionListener listener) {