Skip to content

Commit 9102895

Browse files
rgsriramrgsriram
committed
Added custom remote store segment path prefix for clusterless support
Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Fixed the breaking change Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Added all the tests Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Removed unwanted comments Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Removed unwanted comments Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Added necessary comments for the code made for this change in RemoteStorePathStrategyTests Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Refactored the code Signed-off-by: Sriram Ganesh <srignsh22@gmail.com> Refactored the code Signed-off-by: Sriram Ganesh <srignsh22@gmail.com>
1 parent 30519a3 commit 9102895

File tree

9 files changed

+293
-4
lines changed

9 files changed

+293
-4
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3737
- Add support for plugins to profile information ([#18656](https://github.com/opensearch-project/OpenSearch/pull/18656))
3838
- Add support for Combined Fields query ([#18724](https://github.com/opensearch-project/OpenSearch/pull/18724))
3939
- Added approximation support for range queries with now in date field ([#18511](https://github.com/opensearch-project/OpenSearch/pull/18511))
40+
- Add support for custom remote store segment path prefix to support clusterless configurations ([#18750](https://github.com/opensearch-project/OpenSearch/issues/18750))
4041

4142
### Changed
4243
- Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570))

‎server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,57 @@ public Iterator<Setting<?>> settings() {
458458
Property.Dynamic
459459
);
460460

461+
/**
462+
* Used to specify a custom path prefix for remote store segments. This allows injecting a unique identifier
463+
* (e.g., writer node ID) into the remote store path to support clusterless configurations where multiple
464+
* writers may write to the same shard.
465+
*/
466+
public static final Setting<String> INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX = Setting.simpleString(
467+
"index.remote_store.segment.path_prefix",
468+
"",
469+
new Setting.Validator<>() {
470+
471+
@Override
472+
public void validate(final String value) {}
473+
474+
@Override
475+
public void validate(final String value, final Map<Setting<?>, Object> settings) {
476+
// Only validate if the value is not null and not empty
477+
if (value != null && !value.trim().isEmpty()) {
478+
// Validate that remote store is enabled when this setting is used
479+
final Boolean isRemoteSegmentStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
480+
if (isRemoteSegmentStoreEnabled == null || isRemoteSegmentStoreEnabled == false) {
481+
throw new IllegalArgumentException(
482+
"Setting "
483+
+ INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey()
484+
+ " can only be set when "
485+
+ INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
486+
+ " is set to true"
487+
);
488+
}
489+
490+
// Validate that the path prefix doesn't contain invalid characters for file paths
491+
if (value.contains("/") || value.contains("\\") || value.contains(":")) {
492+
throw new IllegalArgumentException(
493+
"Setting "
494+
+ INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey()
495+
+ " cannot contain path separators (/ or \\) or drive specifiers (:)"
496+
);
497+
}
498+
}
499+
}
500+
501+
@Override
502+
public Iterator<Setting<?>> settings() {
503+
final List<Setting<?>> settings = Collections.singletonList(INDEX_REMOTE_STORE_ENABLED_SETTING);
504+
return settings.iterator();
505+
}
506+
},
507+
Property.IndexScope,
508+
Property.InternalIndex,
509+
Property.Dynamic
510+
);
511+
461512
private static void validateRemoteStoreSettingEnabled(final Map<Setting<?>, Object> settings, Setting<?> setting) {
462513
final Boolean isRemoteSegmentStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
463514
if (isRemoteSegmentStoreEnabled == false) {

‎server/src/main/java/org/opensearch/index/IndexService.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,8 @@ public synchronized IndexShard createShard(
689689
RemoteStoreNodeAttribute.getRemoteStoreSegmentRepo(this.indexSettings.getNodeSettings()),
690690
this.indexSettings.getUUID(),
691691
shardId,
692-
this.indexSettings.getRemoteStorePathStrategy()
692+
this.indexSettings.getRemoteStorePathStrategy(),
693+
this.indexSettings.getRemoteStoreSegmentPathPrefix()
693694
);
694695
}
695696
// When an instance of Store is created, a shardlock is created which is released on closing the instance of store.

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,7 @@ public static IndexMergePolicy fromString(String text) {
809809
private volatile TimeValue remoteTranslogUploadBufferInterval;
810810
private volatile String remoteStoreTranslogRepository;
811811
private volatile String remoteStoreRepository;
812+
private volatile String remoteStoreSegmentPathPrefix;
812813
private int remoteTranslogKeepExtraGen;
813814
private boolean autoForcemergeEnabled;
814815

@@ -1025,6 +1026,9 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
10251026
remoteTranslogUploadBufferInterval = INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(settings);
10261027
remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY);
10271028
this.remoteTranslogKeepExtraGen = INDEX_REMOTE_TRANSLOG_KEEP_EXTRA_GEN_SETTING.get(settings);
1029+
String rawPrefix = IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(settings);
1030+
// Only set the prefix if it's explicitly set and not empty
1031+
this.remoteStoreSegmentPathPrefix = (rawPrefix != null && !rawPrefix.trim().isEmpty()) ? rawPrefix : null;
10281032
this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings);
10291033
this.shouldCleanupUnreferencedFiles = INDEX_UNREFERENCED_FILE_CLEANUP.get(settings);
10301034
this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings);
@@ -1428,6 +1432,13 @@ public String getRemoteStoreTranslogRepository() {
14281432
return remoteStoreTranslogRepository;
14291433
}
14301434

1435+
/**
1436+
* Returns the custom path prefix for remote store segments, if set.
1437+
*/
1438+
public String getRemoteStoreSegmentPathPrefix() {
1439+
return remoteStoreSegmentPathPrefix;
1440+
}
1441+
14311442
/**
14321443
* Returns true if the index is writable warm index and has partial store locality.
14331444
*/

‎server/src/main/java/org/opensearch/index/remote/RemoteStorePathStrategy.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,14 @@ public static class ShardDataPathInput extends PathInput {
230230
private final String shardId;
231231
private final DataCategory dataCategory;
232232
private final DataType dataType;
233+
private final String writerNodeId;
233234

234235
public ShardDataPathInput(Builder builder) {
235236
super(builder);
236237
this.shardId = Objects.requireNonNull(builder.shardId);
237238
this.dataCategory = Objects.requireNonNull(builder.dataCategory);
238239
this.dataType = Objects.requireNonNull(builder.dataType);
240+
this.writerNodeId = builder.writerNodeId; // Can be null
239241
assert dataCategory.isSupportedDataType(dataType) : "category:"
240242
+ dataCategory
241243
+ " type:"
@@ -258,7 +260,12 @@ DataType dataType() {
258260

259261
@Override
260262
BlobPath fixedSubPath() {
261-
return super.fixedSubPath().add(shardId).add(dataCategory.getName()).add(dataType.getName());
263+
BlobPath path = super.fixedSubPath().add(shardId);
264+
// Only add writer node ID if it's explicitly set and not empty
265+
if (writerNodeId != null && !writerNodeId.trim().isEmpty()) {
266+
path = path.add(writerNodeId);
267+
}
268+
return path.add(dataCategory.getName()).add(dataType.getName());
262269
}
263270

264271
/**
@@ -279,6 +286,7 @@ public static class Builder extends PathInput.Builder<Builder> {
279286
private String shardId;
280287
private DataCategory dataCategory;
281288
private DataType dataType;
289+
private String writerNodeId;
282290

283291
public Builder shardId(String shardId) {
284292
this.shardId = shardId;
@@ -295,6 +303,11 @@ public Builder dataType(DataType dataType) {
295303
return this;
296304
}
297305

306+
public Builder writerNodeId(String writerNodeId) {
307+
this.writerNodeId = writerNodeId;
308+
return this;
309+
}
310+
298311
@Override
299312
protected Builder self() {
300313
return this;

‎server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryFactory.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ public Directory newDirectory(IndexSettings indexSettings, ShardPath path) throw
6363

6464
public Directory newDirectory(String repositoryName, String indexUUID, ShardId shardId, RemoteStorePathStrategy pathStrategy)
6565
throws IOException {
66+
return newDirectory(repositoryName, indexUUID, shardId, pathStrategy, null);
67+
}
68+
69+
public Directory newDirectory(
70+
String repositoryName,
71+
String indexUUID,
72+
ShardId shardId,
73+
RemoteStorePathStrategy pathStrategy,
74+
String writerNodeId
75+
) throws IOException {
6676
assert Objects.nonNull(pathStrategy);
6777
try (Repository repository = repositoriesService.get().repository(repositoryName)) {
6878

@@ -78,6 +88,7 @@ public Directory newDirectory(String repositoryName, String indexUUID, ShardId s
7888
.dataCategory(SEGMENTS)
7989
.dataType(DATA)
8090
.fixedPrefix(segmentsPathFixedPrefix)
91+
.writerNodeId(writerNodeId)
8192
.build();
8293
// Derive the path for data directory of SEGMENTS
8394
BlobPath dataPath = pathStrategy.generatePath(dataPathInput);
@@ -95,6 +106,7 @@ public Directory newDirectory(String repositoryName, String indexUUID, ShardId s
95106
.dataCategory(SEGMENTS)
96107
.dataType(METADATA)
97108
.fixedPrefix(segmentsPathFixedPrefix)
109+
.writerNodeId(writerNodeId)
98110
.build();
99111
// Derive the path for metadata directory of SEGMENTS
100112
BlobPath mdPath = pathStrategy.generatePath(mdPathInput);
@@ -107,7 +119,8 @@ public Directory newDirectory(String repositoryName, String indexUUID, ShardId s
107119
indexUUID,
108120
shardIdStr,
109121
pathStrategy,
110-
segmentsPathFixedPrefix
122+
segmentsPathFixedPrefix,
123+
writerNodeId
111124
);
112125

113126
return new RemoteSegmentStoreDirectory(dataDirectory, metadataDirectory, mdLockManager, threadPool, shardId);

‎server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerFactory.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public RemoteStoreLockManager newLockManager(
4444
String shardId,
4545
RemoteStorePathStrategy pathStrategy
4646
) {
47-
return newLockManager(repositoriesService.get(), repositoryName, indexUUID, shardId, pathStrategy, segmentsPathFixedPrefix);
47+
return newLockManager(repositoriesService.get(), repositoryName, indexUUID, shardId, pathStrategy, segmentsPathFixedPrefix, null);
4848
}
4949

5050
public static RemoteStoreMetadataLockManager newLockManager(
@@ -54,6 +54,18 @@ public static RemoteStoreMetadataLockManager newLockManager(
5454
String shardId,
5555
RemoteStorePathStrategy pathStrategy,
5656
String segmentsPathFixedPrefix
57+
) {
58+
return newLockManager(repositoriesService, repositoryName, indexUUID, shardId, pathStrategy, segmentsPathFixedPrefix, null);
59+
}
60+
61+
public static RemoteStoreMetadataLockManager newLockManager(
62+
RepositoriesService repositoriesService,
63+
String repositoryName,
64+
String indexUUID,
65+
String shardId,
66+
RemoteStorePathStrategy pathStrategy,
67+
String segmentsPathFixedPrefix,
68+
String writerNodeId
5769
) {
5870
try (Repository repository = repositoriesService.repository(repositoryName)) {
5971
assert repository instanceof BlobStoreRepository : "repository should be instance of BlobStoreRepository";
@@ -66,6 +78,7 @@ public static RemoteStoreMetadataLockManager newLockManager(
6678
.dataCategory(SEGMENTS)
6779
.dataType(LOCK_FILES)
6880
.fixedPrefix(segmentsPathFixedPrefix)
81+
.writerNodeId(writerNodeId)
6982
.build();
7083
BlobPath lockDirectoryPath = pathStrategy.generatePath(lockFilesPathInput);
7184
BlobContainer lockDirectoryBlobContainer = ((BlobStoreRepository) repository).blobStore().blobContainer(lockDirectoryPath);

‎server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,4 +606,91 @@ public void testIndicesMetadataDiffSystemFlagFlipped() {
606606
assertThat(indexMetadataAfterDiffApplied.getVersion(), equalTo(nextIndexMetadata.getVersion()));
607607
}
608608

609+
/**
610+
* Test validation for remote store segment path prefix setting
611+
*/
612+
public void testRemoteStoreSegmentPathPrefixValidation() {
613+
// Test empty value (should be allowed)
614+
final Settings emptySettings = Settings.builder()
615+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
616+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "")
617+
.build();
618+
619+
IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(emptySettings);
620+
621+
// Test whitespace-only value (should be allowed)
622+
final Settings whitespaceSettings = Settings.builder()
623+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
624+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), " ")
625+
.build();
626+
627+
IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(whitespaceSettings);
628+
629+
// Test valid value when remote store is enabled
630+
final Settings validSettings = Settings.builder()
631+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
632+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "writer-node-1")
633+
.build();
634+
635+
String value = IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(validSettings);
636+
assertEquals("writer-node-1", value);
637+
638+
// Test that setting throws exception when remote store is disabled
639+
final Settings disabledSettings = Settings.builder()
640+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), false)
641+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "writer-node-1")
642+
.build();
643+
644+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
645+
IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(disabledSettings);
646+
});
647+
assertTrue(e.getMessage().contains("can only be set when"));
648+
649+
// Test that setting throws exception when remote store is not set
650+
final Settings noRemoteStoreSettings = Settings.builder()
651+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "writer-node-1")
652+
.build();
653+
654+
e = expectThrows(
655+
IllegalArgumentException.class,
656+
() -> { IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(noRemoteStoreSettings); }
657+
);
658+
assertTrue(e.getMessage().contains("can only be set when"));
659+
660+
// Test that setting throws exception when path contains forward slash
661+
final Settings invalidPathSettings = Settings.builder()
662+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
663+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "writer/node")
664+
.build();
665+
666+
e = expectThrows(
667+
IllegalArgumentException.class,
668+
() -> { IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(invalidPathSettings); }
669+
);
670+
assertTrue(e.getMessage().contains("cannot contain path separators"));
671+
672+
// Test that setting throws exception when path contains backslash
673+
final Settings backslashSettings = Settings.builder()
674+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
675+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "writer\\node")
676+
.build();
677+
678+
e = expectThrows(
679+
IllegalArgumentException.class,
680+
() -> { IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(backslashSettings); }
681+
);
682+
assertTrue(e.getMessage().contains("cannot contain path separators"));
683+
684+
// Test that setting throws exception when path contains colon
685+
final Settings colonSettings = Settings.builder()
686+
.put(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
687+
.put(IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.getKey(), "writer:node")
688+
.build();
689+
690+
e = expectThrows(
691+
IllegalArgumentException.class,
692+
() -> { IndexMetadata.INDEX_REMOTE_STORE_SEGMENT_PATH_PREFIX.get(colonSettings); }
693+
);
694+
assertTrue(e.getMessage().contains("cannot contain path separators"));
695+
}
609696
}

0 commit comments

Comments
 (0)