Address PR comments

Signed-off-by: Sachin Kale <kalsac@amazon.com>
opensearch-project · Aug 14, 2024 · bd569be · bd569be
1 parent 296c86d
commit bd569be
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 18 deletions.
diff --git a/...rc/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsIT.java b/...rc/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsIT.java
@@ -9,16 +9,13 @@
 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;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
 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
@@ -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,18 +802,22 @@ 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,
                     threadPool,
                     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
@@ -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<Void> listener) {