Skip to content

Commit 48edb4e

Browse files
ashking94ashking94
committed
Incorporate PR review comments
Signed-off-by: Ashish Singh <ssashish@amazon.com>
1 parent 683697c commit 48edb4e

File tree

6 files changed

+38
-41
lines changed

6 files changed

+38
-41
lines changed

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@
8888
import org.opensearch.index.mapper.MapperService;
8989
import org.opensearch.index.mapper.MapperService.MergeReason;
9090
import org.opensearch.index.query.QueryShardContext;
91-
import org.opensearch.index.remote.RemoteStoreBlobPathResolver;
92-
import org.opensearch.index.remote.RemoteStoreBlobPathType;
91+
import org.opensearch.index.remote.RemoteStorePathResolver;
92+
import org.opensearch.index.remote.RemoteStorePathType;
9393
import org.opensearch.index.shard.IndexSettingProvider;
9494
import org.opensearch.index.translog.Translog;
9595
import org.opensearch.indices.IndexCreationException;
@@ -170,7 +170,7 @@ public class MetadataCreateIndexService {
170170
private AwarenessReplicaBalance awarenessReplicaBalance;
171171

172172
@Nullable
173-
private final RemoteStoreBlobPathResolver remoteStoreBlobPathResolver;
173+
private final RemoteStorePathResolver remoteStorePathResolver;
174174

175175
public MetadataCreateIndexService(
176176
final Settings settings,
@@ -203,8 +203,8 @@ public MetadataCreateIndexService(
203203

204204
// Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction.
205205
createIndexTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_KEY, true);
206-
remoteStoreBlobPathResolver = isRemoteDataAttributePresent(settings)
207-
? new RemoteStoreBlobPathResolver(clusterService.getClusterSettings())
206+
remoteStorePathResolver = isRemoteDataAttributePresent(settings)
207+
? new RemoteStorePathResolver(clusterService.getClusterSettings())
208208
: null;
209209
}
210210

@@ -554,9 +554,9 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
554554
tmpImdBuilder.settings(indexSettings);
555555
tmpImdBuilder.system(isSystem);
556556

557-
if (remoteStoreBlobPathResolver != null) {
558-
String pathType = remoteStoreBlobPathResolver.resolveType().toString();
559-
tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, Map.of(RemoteStoreBlobPathType.NAME, pathType));
557+
if (remoteStorePathResolver != null) {
558+
String pathType = remoteStorePathResolver.resolveType().toString();
559+
tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, Map.of(RemoteStorePathType.NAME, pathType));
560560
}
561561

562562
// Set up everything, now locally create the index to see that things are ok, and apply

‎server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ public void apply(Settings value, Settings current, Settings previous) {
713713
IoBasedAdmissionControllerSettings.SEARCH_IO_USAGE_LIMIT,
714714
IoBasedAdmissionControllerSettings.INDEXING_IO_USAGE_LIMIT,
715715
IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING,
716-
IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING,
716+
IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING,
717717

718718
// Concurrent segment search settings
719719
SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING,

‎server/src/main/java/org/opensearch/index/remote/RemoteStoreBlobPathResolver.java ‎server/src/main/java/org/opensearch/index/remote/RemoteStorePathResolver.java

+5-7
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,19 @@
1212
import org.opensearch.indices.IndicesService;
1313

1414
/**
15-
* Determines the {@link RemoteStoreBlobPathType} at the time of index metadata creation.
15+
* Determines the {@link RemoteStorePathType} at the time of index metadata creation.
1616
*
1717
* @opensearch.internal
1818
*/
19-
public class RemoteStoreBlobPathResolver {
19+
public class RemoteStorePathResolver {
2020

2121
private final ClusterSettings clusterSettings;
2222

23-
public RemoteStoreBlobPathResolver(ClusterSettings clusterSettings) {
23+
public RemoteStorePathResolver(ClusterSettings clusterSettings) {
2424
this.clusterSettings = clusterSettings;
2525
}
2626

27-
public RemoteStoreBlobPathType resolveType() {
28-
return clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING)
29-
? RemoteStoreBlobPathType.HASHED_PREFIX
30-
: RemoteStoreBlobPathType.FIXED;
27+
public RemoteStorePathType resolveType() {
28+
return clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING);
3129
}
3230
}

‎server/src/main/java/org/opensearch/index/remote/RemoteStoreBlobPathType.java ‎server/src/main/java/org/opensearch/index/remote/RemoteStorePathType.java

+5-6
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,24 @@
88

99
package org.opensearch.index.remote;
1010

11+
import java.util.Locale;
12+
1113
/**
1214
* Enumerates the types of remote store paths resolution techniques supported by OpenSearch.
1315
* For more information, see <a href="https://github.com/opensearch-project/OpenSearch/issues/12567">Github issue #12567</a>.
1416
*
1517
* @opensearch.internal
1618
*/
17-
public enum RemoteStoreBlobPathType {
19+
public enum RemoteStorePathType {
1820

1921
FIXED,
2022
HASHED_PREFIX;
2123

22-
public static RemoteStoreBlobPathType parseString(String remoteStoreBlobPathType) {
24+
public static RemoteStorePathType parseString(String remoteStoreBlobPathType) {
2325
try {
24-
return RemoteStoreBlobPathType.valueOf(remoteStoreBlobPathType);
26+
return RemoteStorePathType.valueOf(remoteStoreBlobPathType.toUpperCase(Locale.ROOT));
2527
} catch (IllegalArgumentException e) {
2628
throw new IllegalArgumentException("Could not parse RemoteStorePathType for [" + remoteStoreBlobPathType + "]");
27-
} catch (NullPointerException npe) {
28-
// return a default value for null input
29-
return FIXED;
3029
}
3130
}
3231

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

+7-5
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@
123123
import org.opensearch.index.query.QueryRewriteContext;
124124
import org.opensearch.index.recovery.RecoveryStats;
125125
import org.opensearch.index.refresh.RefreshStats;
126+
import org.opensearch.index.remote.RemoteStorePathType;
126127
import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory;
127128
import org.opensearch.index.search.stats.SearchStats;
128129
import org.opensearch.index.seqno.RetentionLeaseStats;
@@ -314,12 +315,13 @@ public class IndicesService extends AbstractLifecycleComponent
314315
);
315316

316317
/**
317-
* This setting is used to enable the optimisation in prefix path which helps in achieving higher throughput and lesser
318-
* rate limiting by remote store providers. This setting is effective only for remote store enabled cluster.
318+
* This setting is used to set the remote store blob store path prefix strategy. This setting is effective only for
319+
* remote store enabled cluster.
319320
*/
320-
public static final Setting<Boolean> CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING = Setting.boolSetting(
321-
"cluster.remote_store.index.path.prefix.optimised",
322-
false,
321+
public static final Setting<RemoteStorePathType> CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING = new Setting<>(
322+
"cluster.remote_store.index.path.prefix.type",
323+
RemoteStorePathType.FIXED.toString(),
324+
RemoteStorePathType::parseString,
323325
Property.NodeScope,
324326
Property.Dynamic
325327
);

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

+12-14
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
import org.opensearch.index.IndexSettings;
7272
import org.opensearch.index.mapper.MapperService;
7373
import org.opensearch.index.query.QueryShardContext;
74-
import org.opensearch.index.remote.RemoteStoreBlobPathType;
74+
import org.opensearch.index.remote.RemoteStorePathType;
7575
import org.opensearch.index.translog.Translog;
7676
import org.opensearch.indices.IndexCreationException;
7777
import org.opensearch.indices.IndicesService;
@@ -1587,34 +1587,32 @@ public void testBuildIndexMetadata() {
15871587
*/
15881588
public void testRemoteCustomData() {
15891589
// Case 1 - Remote store is not enabled
1590-
IndexMetadata indexMetadata = testRemoteCustomData(false, randomBoolean());
1590+
IndexMetadata indexMetadata = testRemoteCustomData(false, randomFrom(RemoteStorePathType.values()));
15911591
assertNull(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY));
15921592

1593-
// Case 2 - cluster.remote_store.index.path.prefix.optimised=false (default value)
1594-
indexMetadata = testRemoteCustomData(true, false);
1593+
// Case 2 - cluster.remote_store.index.path.prefix.optimised=fixed (default value)
1594+
indexMetadata = testRemoteCustomData(true, RemoteStorePathType.FIXED);
15951595
validateRemoteCustomData(
15961596
indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY),
1597-
RemoteStoreBlobPathType.NAME,
1598-
RemoteStoreBlobPathType.FIXED.toString()
1597+
RemoteStorePathType.NAME,
1598+
RemoteStorePathType.FIXED.toString()
15991599
);
16001600

1601-
// Case 3 - cluster.remote_store.index.path.prefix.optimised=true
1602-
indexMetadata = testRemoteCustomData(true, true);
1601+
// Case 3 - cluster.remote_store.index.path.prefix.optimised=hashed_prefix
1602+
indexMetadata = testRemoteCustomData(true, RemoteStorePathType.HASHED_PREFIX);
16031603
validateRemoteCustomData(
16041604
indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY),
1605-
RemoteStoreBlobPathType.NAME,
1606-
RemoteStoreBlobPathType.HASHED_PREFIX.toString()
1605+
RemoteStorePathType.NAME,
1606+
RemoteStorePathType.HASHED_PREFIX.toString()
16071607
);
16081608
}
16091609

1610-
private IndexMetadata testRemoteCustomData(boolean remoteStoreEnabled, boolean optimisedPrefix) {
1610+
private IndexMetadata testRemoteCustomData(boolean remoteStoreEnabled, RemoteStorePathType remoteStorePathType) {
16111611
Settings.Builder settingsBuilder = Settings.builder();
16121612
if (remoteStoreEnabled) {
16131613
settingsBuilder.put(NODE_ATTRIBUTES.getKey() + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "test");
16141614
}
1615-
if (optimisedPrefix) {
1616-
settingsBuilder.put(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING.getKey(), true);
1617-
}
1615+
settingsBuilder.put(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), remoteStorePathType.toString());
16181616
Settings settings = settingsBuilder.build();
16191617

16201618
ClusterService clusterService = mock(ClusterService.class);

0 commit comments

Comments
 (0)