Memoize isOnRemoteNode in index settings and refactor as well (#12994) (

#13016)


Signed-off-by: Gaurav Bafna <[email protected]>
(cherry picked from commit eb66f62)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

server/src/main/java/org/opensearch/index/IndexSettings.java

@@ -759,6 +759,7 @@ public static IndexMergePolicy fromString(String text) {
 
     private volatile String defaultSearchPipeline;
     private final boolean widenIndexSortType;
+    private final boolean assignedOnRemoteNode;
 
     /**
      * The maximum age of a retention lease before it is considered expired.
@@ -981,6 +982,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
          * Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them.
          */
         widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0);
+        assignedOnRemoteNode = RemoteStoreNodeAttribute.isRemoteDataAttributePresent(this.getNodeSettings());
 
         setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING));
         setDocIdFuzzySetFalsePositiveProbability(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING));
@@ -1226,7 +1228,7 @@ public int getNumberOfReplicas() {
      * proper index setting during the migration.
      */
     public boolean isSegRepEnabledOrRemoteNode() {
-        return ReplicationType.SEGMENT.equals(replicationType) || isRemoteNode();
+        return ReplicationType.SEGMENT.equals(replicationType) || isAssignedOnRemoteNode();
     }
 
     public boolean isSegRepLocalEnabled() {
@@ -1244,8 +1246,8 @@ public boolean isRemoteStoreEnabled() {
         return isRemoteStoreEnabled;
     }
 
-    public boolean isRemoteNode() {
-        return RemoteStoreNodeAttribute.isRemoteDataAttributePresent(this.getNodeSettings());
+    public boolean isAssignedOnRemoteNode() {
+        return assignedOnRemoteNode;
     }
 
     /**

server/src/main/java/org/opensearch/index/engine/NRTReplicationEngine.java

@@ -489,7 +489,7 @@ protected final void closeNoLock(String reason, CountDownLatch closedLatch) {
                  during promotion.
                  */
                 if (engineConfig.getIndexSettings().isRemoteStoreEnabled() == false
-                    && engineConfig.getIndexSettings().isRemoteNode() == false) {
+                    && engineConfig.getIndexSettings().isAssignedOnRemoteNode() == false) {
                     latestSegmentInfos.counter = latestSegmentInfos.counter + SI_COUNTER_INCREMENT;
                     latestSegmentInfos.changed();
                 }

server/src/main/java/org/opensearch/index/remote/RemoteStoreStatsTrackerFactory.java

@@ -68,7 +68,7 @@ public RemoteStoreStatsTrackerFactory(ClusterService clusterService, Settings se
 
     @Override
     public void afterIndexShardCreated(IndexShard indexShard) {
-        if (indexShard.indexSettings().isRemoteStoreEnabled() == false && indexShard.indexSettings().isRemoteNode() == false) {
+        if (indexShard.indexSettings().isRemoteStoreEnabled() == false && indexShard.indexSettings().isAssignedOnRemoteNode() == false) {
             return;
         }
         ShardId shardId = indexShard.shardId();

server/src/main/java/org/opensearch/index/shard/IndexShard.java

@@ -408,7 +408,7 @@ public IndexShard(
             logger,
             threadPool,
             this::getEngine,
-            indexSettings.isRemoteNode(),
+            indexSettings.isAssignedOnRemoteNode(),
             () -> getRemoteTranslogUploadBufferInterval(remoteStoreSettings::getClusterRemoteTranslogBufferInterval)
         );
         this.mapperService = mapperService;
@@ -1476,7 +1476,7 @@ public SegmentsStats segmentStats(boolean includeSegmentFileSizes, boolean inclu
         SegmentsStats segmentsStats = getEngine().segmentsStats(includeSegmentFileSizes, includeUnloadedSegments);
         segmentsStats.addBitsetMemoryInBytes(shardBitsetFilterCache.getMemorySizeInBytes());
         // Populate remote_store stats only if the index is remote store backed
-        if (indexSettings().isRemoteNode()) {
+        if (indexSettings().isAssignedOnRemoteNode()) {
             segmentsStats.addRemoteSegmentStats(
                 new RemoteSegmentStats(remoteStoreStatsTrackerFactory.getRemoteSegmentTransferTracker(shardId).stats())
             );
@@ -1498,7 +1498,7 @@ public FieldDataStats fieldDataStats(String... fields) {
     public TranslogStats translogStats() {
         TranslogStats translogStats = getEngine().getTranslogStats();
         // Populate remote_store stats only if the index is remote store backed
-        if (indexSettings.isRemoteNode()) {
+        if (indexSettings.isAssignedOnRemoteNode()) {
             translogStats.addRemoteTranslogStats(
                 new RemoteTranslogStats(remoteStoreStatsTrackerFactory.getRemoteTranslogTransferTracker(shardId).stats())
             );
@@ -1537,7 +1537,7 @@ public void flush(FlushRequest request) {
      * {@link org.opensearch.index.translog.TranslogDeletionPolicy} for details
      */
     public void trimTranslog() {
-        if (indexSettings.isRemoteNode()) {
+        if (indexSettings.isAssignedOnRemoteNode()) {
             return;
         }
         verifyNotClosed();
@@ -2059,7 +2059,7 @@ public void close(String reason, boolean flushEngine, boolean deleted) throws IO
     ToDo : Fix this https://github.com/opensearch-project/OpenSearch/issues/8003
      */
     public RemoteSegmentStoreDirectory getRemoteDirectory() {
-        assert indexSettings.isRemoteNode();
+        assert indexSettings.isAssignedOnRemoteNode();
         assert remoteStore.directory() instanceof FilterDirectory : "Store.directory is not an instance of FilterDirectory";
         FilterDirectory remoteStoreDirectory = (FilterDirectory) remoteStore.directory();
         FilterDirectory byteSizeCachingStoreDirectory = (FilterDirectory) remoteStoreDirectory.getDelegate();
@@ -2072,7 +2072,7 @@ public RemoteSegmentStoreDirectory getRemoteDirectory() {
      * is in sync with local
      */
     public boolean isRemoteSegmentStoreInSync() {
-        assert indexSettings.isRemoteNode();
+        assert indexSettings.isAssignedOnRemoteNode();
         try {
             RemoteSegmentStoreDirectory directory = getRemoteDirectory();
             if (directory.readLatestMetadataFile() != null) {
@@ -2111,7 +2111,7 @@ public void waitForRemoteStoreSync() {
     Calls onProgress on seeing an increased file count on remote
     */
     public void waitForRemoteStoreSync(Runnable onProgress) {
-        assert indexSettings.isRemoteNode();
+        assert indexSettings.isAssignedOnRemoteNode();
         RemoteSegmentStoreDirectory directory = getRemoteDirectory();
         int segmentUploadeCount = 0;
         if (shardRouting.primary() == false) {
@@ -2285,7 +2285,7 @@ public long recoverLocallyAndFetchStartSeqNo(boolean localTranslog) {
      * @return the starting sequence number from which the recovery should start.
      */
     private long recoverLocallyUptoLastCommit() {
-        assert indexSettings.isRemoteNode() : "Remote translog store is not enabled";
+        assert indexSettings.isAssignedOnRemoteNode() : "Remote translog store is not enabled";
         long seqNo;
         validateLocalRecoveryState();
 
@@ -3555,7 +3555,7 @@ assert getLocalCheckpoint() == primaryContext.getCheckpointStates().get(allocati
     }
 
     private void postActivatePrimaryMode() {
-        if (indexSettings.isRemoteNode()) {
+        if (indexSettings.isAssignedOnRemoteNode()) {
             // We make sure to upload translog (even if it does not contain any operations) to remote translog.
             // This helps to get a consistent state in remote store where both remote segment store and remote
             // translog contains data.
@@ -4025,7 +4025,7 @@ public boolean enableUploadToRemoteTranslog() {
     }
 
     private boolean hasOneRemoteSegmentSyncHappened() {
-        assert indexSettings.isRemoteNode();
+        assert indexSettings.isAssignedOnRemoteNode();
         // We upload remote translog only after one remote segment upload in case of migration
         RemoteSegmentStoreDirectory rd = getRemoteDirectory();
         AtomicBoolean segment_n_uploaded = new AtomicBoolean(false);
@@ -4639,7 +4639,7 @@ public final boolean isSearchIdle() {
     public final boolean isSearchIdleSupported() {
         // If the index is remote store backed, then search idle is not supported. This is to ensure that async refresh
         // task continues to upload to remote store periodically.
-        if (isRemoteTranslogEnabled() || indexSettings.isRemoteNode()) {
+        if (isRemoteTranslogEnabled() || indexSettings.isAssignedOnRemoteNode()) {
             return false;
         }
         return indexSettings.isSegRepEnabledOrRemoteNode() == false || indexSettings.getNumberOfReplicas() == 0;
@@ -5269,9 +5269,9 @@ enum ShardMigrationState {
     }
 
     static ShardMigrationState getShardMigrationState(IndexSettings indexSettings, boolean shouldSeed) {
-        if (indexSettings.isRemoteNode() && indexSettings.isRemoteStoreEnabled()) {
+        if (indexSettings.isAssignedOnRemoteNode() && indexSettings.isRemoteStoreEnabled()) {
             return REMOTE_NON_MIGRATING;
-        } else if (indexSettings.isRemoteNode()) {
+        } else if (indexSettings.isAssignedOnRemoteNode()) {
             return shouldSeed ? REMOTE_MIGRATING_UNSEEDED : REMOTE_MIGRATING_SEEDED;
         }
         return ShardMigrationState.DOCREP_NON_MIGRATING;

server/src/main/java/org/opensearch/index/store/Store.java

@@ -893,7 +893,7 @@ public void beforeClose() {
      * @throws IOException when there is an IO error committing.
      */
     public void commitSegmentInfos(SegmentInfos latestSegmentInfos, long maxSeqNo, long processedCheckpoint) throws IOException {
-        assert indexSettings.isSegRepEnabledOrRemoteNode() || indexSettings.isRemoteNode();
+        assert indexSettings.isSegRepEnabledOrRemoteNode() || indexSettings.isAssignedOnRemoteNode();
         metadataLock.writeLock().lock();
         try {
             final Map<String, String> userData = new HashMap<>(latestSegmentInfos.getUserData());

server/src/main/java/org/opensearch/index/translog/Translog.java

@@ -525,7 +525,7 @@ TranslogWriter createWriter(
                 tragedy,
                 persistedSequenceNumberConsumer,
                 bigArrays,
-                indexSettings.isRemoteNode()
+                indexSettings.isAssignedOnRemoteNode()
             );
         } catch (final IOException e) {
             throw new TranslogException(shardId, "failed to create new translog file", e);

server/src/main/java/org/opensearch/indices/IndicesService.java

@@ -923,7 +923,7 @@ private EngineFactory getEngineFactory(final IndexSettings idxSettings) {
             if (idxSettings.isRemoteSnapshot()) {
                 return config -> new ReadOnlyEngine(config, new SeqNoStats(0, 0, 0), new TranslogStats(), true, Function.identity(), false);
             }
-            if (idxSettings.isSegRepEnabledOrRemoteNode() || idxSettings.isRemoteNode()) {
+            if (idxSettings.isSegRepEnabledOrRemoteNode() || idxSettings.isAssignedOnRemoteNode()) {
                 return new NRTReplicationEngineFactory();
             }
             return new InternalEngineFactory();

server/src/main/java/org/opensearch/indices/recovery/PeerRecoveryTargetService.java

@@ -262,7 +262,7 @@ private void doRecovery(final long recoveryId, final StartRecoveryRequest preExi
                         }
                     }
                     final boolean hasRemoteTranslog = recoveryTarget.state().getPrimary() == false
-                        && indexShard.indexSettings().isRemoteNode();
+                        && indexShard.indexSettings().isAssignedOnRemoteNode();
                     final boolean hasNoTranslog = indexShard.indexSettings().isRemoteSnapshot();
                     final boolean verifyTranslog = (hasRemoteTranslog || hasNoTranslog || hasRemoteSegmentStore) == false;
                     final long startingSeqNo = indexShard.recoverLocallyAndFetchStartSeqNo(!hasRemoteTranslog);

server/src/test/java/org/opensearch/index/IndexSettingsTests.java

@@ -1052,4 +1052,15 @@ public void testDefaultSearchPipeline() throws Exception {
         settings.updateIndexMetadata(metadata);
         assertEquals("foo", settings.getDefaultSearchPipeline());
     }
+
+    public void testIsOnRemoteNode() {
+        Version version = VersionUtils.getPreviousVersion();
+        Settings theSettings = Settings.builder()
+            .put(IndexMetadata.SETTING_VERSION_CREATED, version)
+            .put(IndexMetadata.SETTING_INDEX_UUID, "0xdeadbeef")
+            .build();
+        Settings nodeSettings = Settings.builder().put("node.attr.remote_store.translog.repository", "my-repo-1").build();
+        IndexSettings settings = newIndexSettings(newIndexMeta("index", theSettings), nodeSettings);
+        assertTrue("Index should be on remote node", settings.isAssignedOnRemoteNode());
+    }
 }

test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java

@@ -654,7 +654,7 @@ protected IndexShard newShard(
             RemoteStoreStatsTrackerFactory remoteStoreStatsTrackerFactory = null;
             RepositoriesService mockRepoSvc = mock(RepositoriesService.class);
 
-            if (indexSettings.isRemoteStoreEnabled() || indexSettings.isRemoteNode()) {
+            if (indexSettings.isRemoteStoreEnabled() || indexSettings.isAssignedOnRemoteNode()) {
                 String remoteStoreRepository = indexSettings.getRemoteStoreRepository();
                 // remote path via setting a repository . This is a hack used for shards are created using reset .
                 // since we can't get remote path from IndexShard directly, we are using repository to store it .
@@ -1498,7 +1498,7 @@ private SegmentReplicationTargetService prepareForReplication(
 
         SegmentReplicationSourceFactory sourceFactory = null;
         SegmentReplicationTargetService targetService;
-        if (primaryShard.indexSettings.isRemoteStoreEnabled() || primaryShard.indexSettings.isRemoteNode()) {
+        if (primaryShard.indexSettings.isRemoteStoreEnabled() || primaryShard.indexSettings.isAssignedOnRemoteNode()) {
             RecoverySettings recoverySettings = new RecoverySettings(
                 Settings.EMPTY,
                 new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)