Skip to content

Commit eb66f62

Browse files
authored
Memoize isOnRemoteNode in index settings and refactor as well (#12994)
* Memoize isOnRemoteNode in index settings and rename it as well --------- Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>
1 parent 170ea7a commit eb66f62

File tree

10 files changed

+37
-24
lines changed

10 files changed

+37
-24
lines changed

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

+5-3
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,7 @@ public static IndexMergePolicy fromString(String text) {
763763

764764
private volatile String defaultSearchPipeline;
765765
private final boolean widenIndexSortType;
766+
private final boolean assignedOnRemoteNode;
766767

767768
/**
768769
* The maximum age of a retention lease before it is considered expired.
@@ -986,6 +987,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
986987
* Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them.
987988
*/
988989
widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0);
990+
assignedOnRemoteNode = RemoteStoreNodeAttribute.isRemoteDataAttributePresent(this.getNodeSettings());
989991

990992
setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING));
991993
setDocIdFuzzySetFalsePositiveProbability(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING));
@@ -1231,7 +1233,7 @@ public int getNumberOfReplicas() {
12311233
* proper index setting during the migration.
12321234
*/
12331235
public boolean isSegRepEnabledOrRemoteNode() {
1234-
return ReplicationType.SEGMENT.equals(replicationType) || isRemoteNode();
1236+
return ReplicationType.SEGMENT.equals(replicationType) || isAssignedOnRemoteNode();
12351237
}
12361238

12371239
public boolean isSegRepLocalEnabled() {
@@ -1249,8 +1251,8 @@ public boolean isRemoteStoreEnabled() {
12491251
return isRemoteStoreEnabled;
12501252
}
12511253

1252-
public boolean isRemoteNode() {
1253-
return RemoteStoreNodeAttribute.isRemoteDataAttributePresent(this.getNodeSettings());
1254+
public boolean isAssignedOnRemoteNode() {
1255+
return assignedOnRemoteNode;
12541256
}
12551257

12561258
/**

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ protected final void closeNoLock(String reason, CountDownLatch closedLatch) {
437437
during promotion.
438438
*/
439439
if (engineConfig.getIndexSettings().isRemoteStoreEnabled() == false
440-
&& engineConfig.getIndexSettings().isRemoteNode() == false) {
440+
&& engineConfig.getIndexSettings().isAssignedOnRemoteNode() == false) {
441441
latestSegmentInfos.counter = latestSegmentInfos.counter + SI_COUNTER_INCREMENT;
442442
latestSegmentInfos.changed();
443443
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public RemoteStoreStatsTrackerFactory(ClusterService clusterService, Settings se
6868

6969
@Override
7070
public void afterIndexShardCreated(IndexShard indexShard) {
71-
if (indexShard.indexSettings().isRemoteStoreEnabled() == false && indexShard.indexSettings().isRemoteNode() == false) {
71+
if (indexShard.indexSettings().isRemoteStoreEnabled() == false && indexShard.indexSettings().isAssignedOnRemoteNode() == false) {
7272
return;
7373
}
7474
ShardId shardId = indexShard.shardId();

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

+13-13
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ public IndexShard(
406406
logger,
407407
threadPool,
408408
this::getEngine,
409-
indexSettings.isRemoteNode(),
409+
indexSettings.isAssignedOnRemoteNode(),
410410
() -> getRemoteTranslogUploadBufferInterval(remoteStoreSettings::getClusterRemoteTranslogBufferInterval)
411411
);
412412
this.mapperService = mapperService;
@@ -1469,7 +1469,7 @@ public SegmentsStats segmentStats(boolean includeSegmentFileSizes, boolean inclu
14691469
SegmentsStats segmentsStats = getEngine().segmentsStats(includeSegmentFileSizes, includeUnloadedSegments);
14701470
segmentsStats.addBitsetMemoryInBytes(shardBitsetFilterCache.getMemorySizeInBytes());
14711471
// Populate remote_store stats only if the index is remote store backed
1472-
if (indexSettings().isRemoteNode()) {
1472+
if (indexSettings().isAssignedOnRemoteNode()) {
14731473
segmentsStats.addRemoteSegmentStats(
14741474
new RemoteSegmentStats(remoteStoreStatsTrackerFactory.getRemoteSegmentTransferTracker(shardId).stats())
14751475
);
@@ -1491,7 +1491,7 @@ public FieldDataStats fieldDataStats(String... fields) {
14911491
public TranslogStats translogStats() {
14921492
TranslogStats translogStats = getEngine().translogManager().getTranslogStats();
14931493
// Populate remote_store stats only if the index is remote store backed
1494-
if (indexSettings.isRemoteNode()) {
1494+
if (indexSettings.isAssignedOnRemoteNode()) {
14951495
translogStats.addRemoteTranslogStats(
14961496
new RemoteTranslogStats(remoteStoreStatsTrackerFactory.getRemoteTranslogTransferTracker(shardId).stats())
14971497
);
@@ -1530,7 +1530,7 @@ public void flush(FlushRequest request) {
15301530
* {@link org.opensearch.index.translog.TranslogDeletionPolicy} for details
15311531
*/
15321532
public void trimTranslog() {
1533-
if (indexSettings.isRemoteNode()) {
1533+
if (indexSettings.isAssignedOnRemoteNode()) {
15341534
return;
15351535
}
15361536
verifyNotClosed();
@@ -2050,7 +2050,7 @@ public void close(String reason, boolean flushEngine, boolean deleted) throws IO
20502050
ToDo : Fix this https://github.com/opensearch-project/OpenSearch/issues/8003
20512051
*/
20522052
public RemoteSegmentStoreDirectory getRemoteDirectory() {
2053-
assert indexSettings.isRemoteNode();
2053+
assert indexSettings.isAssignedOnRemoteNode();
20542054
assert remoteStore.directory() instanceof FilterDirectory : "Store.directory is not an instance of FilterDirectory";
20552055
FilterDirectory remoteStoreDirectory = (FilterDirectory) remoteStore.directory();
20562056
FilterDirectory byteSizeCachingStoreDirectory = (FilterDirectory) remoteStoreDirectory.getDelegate();
@@ -2063,7 +2063,7 @@ public RemoteSegmentStoreDirectory getRemoteDirectory() {
20632063
* is in sync with local
20642064
*/
20652065
public boolean isRemoteSegmentStoreInSync() {
2066-
assert indexSettings.isRemoteNode();
2066+
assert indexSettings.isAssignedOnRemoteNode();
20672067
try {
20682068
RemoteSegmentStoreDirectory directory = getRemoteDirectory();
20692069
if (directory.readLatestMetadataFile() != null) {
@@ -2102,7 +2102,7 @@ public void waitForRemoteStoreSync() {
21022102
Calls onProgress on seeing an increased file count on remote
21032103
*/
21042104
public void waitForRemoteStoreSync(Runnable onProgress) {
2105-
assert indexSettings.isRemoteNode();
2105+
assert indexSettings.isAssignedOnRemoteNode();
21062106
RemoteSegmentStoreDirectory directory = getRemoteDirectory();
21072107
int segmentUploadeCount = 0;
21082108
if (shardRouting.primary() == false) {
@@ -2277,7 +2277,7 @@ public long recoverLocallyAndFetchStartSeqNo(boolean localTranslog) {
22772277
* @return the starting sequence number from which the recovery should start.
22782278
*/
22792279
private long recoverLocallyUptoLastCommit() {
2280-
assert indexSettings.isRemoteNode() : "Remote translog store is not enabled";
2280+
assert indexSettings.isAssignedOnRemoteNode() : "Remote translog store is not enabled";
22812281
long seqNo;
22822282
validateLocalRecoveryState();
22832283

@@ -3540,7 +3540,7 @@ assert getLocalCheckpoint() == primaryContext.getCheckpointStates().get(allocati
35403540
}
35413541

35423542
private void postActivatePrimaryMode() {
3543-
if (indexSettings.isRemoteNode()) {
3543+
if (indexSettings.isAssignedOnRemoteNode()) {
35443544
// We make sure to upload translog (even if it does not contain any operations) to remote translog.
35453545
// This helps to get a consistent state in remote store where both remote segment store and remote
35463546
// translog contains data.
@@ -4010,7 +4010,7 @@ public boolean enableUploadToRemoteTranslog() {
40104010
}
40114011

40124012
private boolean hasOneRemoteSegmentSyncHappened() {
4013-
assert indexSettings.isRemoteNode();
4013+
assert indexSettings.isAssignedOnRemoteNode();
40144014
// We upload remote translog only after one remote segment upload in case of migration
40154015
RemoteSegmentStoreDirectory rd = getRemoteDirectory();
40164016
AtomicBoolean segment_n_uploaded = new AtomicBoolean(false);
@@ -4624,7 +4624,7 @@ public final boolean isSearchIdle() {
46244624
public final boolean isSearchIdleSupported() {
46254625
// If the index is remote store backed, then search idle is not supported. This is to ensure that async refresh
46264626
// task continues to upload to remote store periodically.
4627-
if (isRemoteTranslogEnabled() || indexSettings.isRemoteNode()) {
4627+
if (isRemoteTranslogEnabled() || indexSettings.isAssignedOnRemoteNode()) {
46284628
return false;
46294629
}
46304630
return indexSettings.isSegRepEnabledOrRemoteNode() == false || indexSettings.getNumberOfReplicas() == 0;
@@ -5263,9 +5263,9 @@ enum ShardMigrationState {
52635263
}
52645264

52655265
static ShardMigrationState getShardMigrationState(IndexSettings indexSettings, boolean shouldSeed) {
5266-
if (indexSettings.isRemoteNode() && indexSettings.isRemoteStoreEnabled()) {
5266+
if (indexSettings.isAssignedOnRemoteNode() && indexSettings.isRemoteStoreEnabled()) {
52675267
return REMOTE_NON_MIGRATING;
5268-
} else if (indexSettings.isRemoteNode()) {
5268+
} else if (indexSettings.isAssignedOnRemoteNode()) {
52695269
return shouldSeed ? REMOTE_MIGRATING_UNSEEDED : REMOTE_MIGRATING_SEEDED;
52705270
}
52715271
return ShardMigrationState.DOCREP_NON_MIGRATING;

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,7 @@ public void beforeClose() {
893893
* @throws IOException when there is an IO error committing.
894894
*/
895895
public void commitSegmentInfos(SegmentInfos latestSegmentInfos, long maxSeqNo, long processedCheckpoint) throws IOException {
896-
assert indexSettings.isSegRepEnabledOrRemoteNode() || indexSettings.isRemoteNode();
896+
assert indexSettings.isSegRepEnabledOrRemoteNode() || indexSettings.isAssignedOnRemoteNode();
897897
metadataLock.writeLock().lock();
898898
try {
899899
final Map<String, String> userData = new HashMap<>(latestSegmentInfos.getUserData());

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ TranslogWriter createWriter(
525525
tragedy,
526526
persistedSequenceNumberConsumer,
527527
bigArrays,
528-
indexSettings.isRemoteNode()
528+
indexSettings.isAssignedOnRemoteNode()
529529
);
530530
} catch (final IOException e) {
531531
throw new TranslogException(shardId, "failed to create new translog file", e);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,7 @@ private EngineFactory getEngineFactory(final IndexSettings idxSettings) {
933933
if (idxSettings.isRemoteSnapshot()) {
934934
return config -> new ReadOnlyEngine(config, new SeqNoStats(0, 0, 0), new TranslogStats(), true, Function.identity(), false);
935935
}
936-
if (idxSettings.isSegRepEnabledOrRemoteNode() || idxSettings.isRemoteNode()) {
936+
if (idxSettings.isSegRepEnabledOrRemoteNode() || idxSettings.isAssignedOnRemoteNode()) {
937937
return new NRTReplicationEngineFactory();
938938
}
939939
return new InternalEngineFactory();

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ private void doRecovery(final long recoveryId, final StartRecoveryRequest preExi
261261
}
262262
}
263263
final boolean hasRemoteTranslog = recoveryTarget.state().getPrimary() == false
264-
&& indexShard.indexSettings().isRemoteNode();
264+
&& indexShard.indexSettings().isAssignedOnRemoteNode();
265265
final boolean hasNoTranslog = indexShard.indexSettings().isRemoteSnapshot();
266266
final boolean verifyTranslog = (hasRemoteTranslog || hasNoTranslog || hasRemoteSegmentStore) == false;
267267
final long startingSeqNo = indexShard.recoverLocallyAndFetchStartSeqNo(!hasRemoteTranslog);

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

+11
Original file line numberDiff line numberDiff line change
@@ -1053,4 +1053,15 @@ public void testDefaultSearchPipeline() throws Exception {
10531053
settings.updateIndexMetadata(metadata);
10541054
assertEquals("foo", settings.getDefaultSearchPipeline());
10551055
}
1056+
1057+
public void testIsOnRemoteNode() {
1058+
Version version = VersionUtils.getPreviousVersion();
1059+
Settings theSettings = Settings.builder()
1060+
.put(IndexMetadata.SETTING_VERSION_CREATED, version)
1061+
.put(IndexMetadata.SETTING_INDEX_UUID, "0xdeadbeef")
1062+
.build();
1063+
Settings nodeSettings = Settings.builder().put("node.attr.remote_store.translog.repository", "my-repo-1").build();
1064+
IndexSettings settings = newIndexSettings(newIndexMeta("index", theSettings), nodeSettings);
1065+
assertTrue("Index should be on remote node", settings.isAssignedOnRemoteNode());
1066+
}
10561067
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ protected IndexShard newShard(
654654
RemoteStoreStatsTrackerFactory remoteStoreStatsTrackerFactory = null;
655655
RepositoriesService mockRepoSvc = mock(RepositoriesService.class);
656656

657-
if (indexSettings.isRemoteStoreEnabled() || indexSettings.isRemoteNode()) {
657+
if (indexSettings.isRemoteStoreEnabled() || indexSettings.isAssignedOnRemoteNode()) {
658658
String remoteStoreRepository = indexSettings.getRemoteStoreRepository();
659659
// remote path via setting a repository . This is a hack used for shards are created using reset .
660660
// since we can't get remote path from IndexShard directly, we are using repository to store it .
@@ -1498,7 +1498,7 @@ private SegmentReplicationTargetService prepareForReplication(
14981498

14991499
SegmentReplicationSourceFactory sourceFactory = null;
15001500
SegmentReplicationTargetService targetService;
1501-
if (primaryShard.indexSettings.isRemoteStoreEnabled() || primaryShard.indexSettings.isRemoteNode()) {
1501+
if (primaryShard.indexSettings.isRemoteStoreEnabled() || primaryShard.indexSettings.isAssignedOnRemoteNode()) {
15021502
RecoverySettings recoverySettings = new RecoverySettings(
15031503
Settings.EMPTY,
15041504
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)

0 commit comments

Comments
 (0)