Skip to content

Commit

Permalink
Allow index & cluster default refresh interval setting value to be -1
Browse files Browse the repository at this point in the history 
Signed-off-by: Ashish Singh <[email protected]>
  • Loading branch information
@ashking94
ashking94 committed Dec 1, 2023
1 parent 69cc2a1 commit c7f4531
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 24 deletions.
170 changes: 149 additions & 21 deletions ...ternalClusterTest/java/org/opensearch/cluster/metadata/ClusterIndexRefreshIntervalIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,33 +235,20 @@ public void testDefaultRefreshIntervalWithUpdateClusterAndIndexSettings() throws
}

public void testRefreshIntervalDisabled() throws ExecutionException, InterruptedException {
TimeValue clusterMinimumRefreshInterval = client().settings()
.getAsTime(IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.MINUS_ONE);
boolean createIndexSuccess = clusterMinimumRefreshInterval.equals(TimeValue.MINUS_ONE);
String clusterManagerName = internalCluster().getClusterManagerName();
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
Settings settings = Settings.builder()
.put(indexSettings())
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), IndexSettings.MINIMUM_REFRESH_INTERVAL)
.build();
if (createIndexSuccess) {
createIndex(INDEX_NAME, settings);
ensureYellowAndNoInitializingShards(INDEX_NAME);
ensureGreen(INDEX_NAME);
GetIndexResponse getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get();
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes));
String uuid = getIndexResponse.getSettings().get(INDEX_NAME).get(IndexMetadata.SETTING_INDEX_UUID);
IndexService indexService = indicesService.indexService(new Index(INDEX_NAME, uuid));
assertEquals(IndexSettings.MINIMUM_REFRESH_INTERVAL, indexService.getRefreshTaskInterval());
} else {
IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> createIndex(INDEX_NAME, settings));
assertEquals(
"invalid index.refresh_interval [-1]: cannot be smaller than cluster.minimum.index.refresh_interval ["
+ getMinRefreshIntervalForRefreshDisabled()
+ "]",
exception.getMessage()
);
}
createIndex(INDEX_NAME, settings);
ensureYellowAndNoInitializingShards(INDEX_NAME);
ensureGreen(INDEX_NAME);
GetIndexResponse getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get();
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes));
String uuid = getIndexResponse.getSettings().get(INDEX_NAME).get(IndexMetadata.SETTING_INDEX_UUID);
IndexService indexService = indicesService.indexService(new Index(INDEX_NAME, uuid));
assertEquals(IndexSettings.MINIMUM_REFRESH_INTERVAL, indexService.getRefreshTaskInterval());
}

protected TimeValue getMinRefreshIntervalForRefreshDisabled() {
Expand Down Expand Up @@ -366,6 +353,147 @@ public void testClusterMinimumChangeOnIndexWithCustomRefreshInterval() throws Ex
assertEquals(customRefreshInterval, indexService.getRefreshTaskInterval());
}

public void testClusterMinimumRefreshIntervalOfMinusOneFails() {
// This test checks that we can not set cluster minimum refresh interval as -1 (or -1ms).
String clusterManagerName = internalCluster().getClusterManagerName();
String refreshInterval = randomFrom("-1", "-1ms");
IllegalArgumentException ex = assertThrows(
IllegalArgumentException.class,
() -> client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), refreshInterval))
.get()
);
assertEquals(
"failed to parse value [" + refreshInterval + "] for setting [cluster.minimum.index.refresh_interval], must be >= [0ms]",
ex.getMessage()
);
}

public void testClusterMinimumRefreshIntervalOfZero() {
// This test checks that we can set the cluster minimum refresh interval as 0.
String clusterManagerName = internalCluster().getClusterManagerName();
client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "0"))
.get();
}

public void testDefaultRefreshIntervalOfMinusOneIrrespectiveOfMinimum() {
// This test checks that we are able to set the cluster default refresh interval to one regardless of what the
// minimum is set to. -1 corresponds to no period background refreshes.
String clusterManagerName = internalCluster().getClusterManagerName();
client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("0", "1ms", "1s", "10s"))
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms"))
)
.get();
}

public void testCreateIndexWithMinusOneRefreshInterval() throws ExecutionException, InterruptedException {
// This test checks that we are able to create index with -1 refresh interval using index settings and default interval both.
String clusterManagerName = internalCluster().getClusterManagerName();
client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s")
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s")
)
.get();

Settings indexSettings = Settings.builder()
.put(indexSettings())
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms"))
.build();
createIndex(INDEX_NAME, indexSettings);
ensureGreen(INDEX_NAME);

IndexService indexService = getIndexServiceFromRandomDataNode(INDEX_NAME);
assertEquals(-1, indexService.getRefreshTaskInterval().millis());

client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms")))
.get();
createIndex(OTHER_INDEX_NAME);
ensureGreen(OTHER_INDEX_NAME);
indexService = getIndexServiceFromRandomDataNode(OTHER_INDEX_NAME);
assertEquals(-1, indexService.getRefreshTaskInterval().millis());
}

public void testUpdateIndexWithMinusOneRefreshInterval() throws ExecutionException, InterruptedException {
// This test checks that we are able to update index with -1 refresh interval using index settings and default interval both.
String clusterManagerName = internalCluster().getClusterManagerName();
client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(
Settings.builder()
.put(CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s")
.put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s")
)
.get();

createIndex(INDEX_NAME);
ensureGreen(INDEX_NAME);
IndexService indexService = getIndexServiceFromRandomDataNode(INDEX_NAME);
assertEquals(10, indexService.getRefreshTaskInterval().seconds());

client(clusterManagerName).admin()
.indices()
.updateSettings(
new UpdateSettingsRequest(INDEX_NAME).settings(
Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms"))
)
)
.actionGet();
assertEquals(-1, indexService.getRefreshTaskInterval().millis());

client(clusterManagerName).admin()
.indices()
.updateSettings(
new UpdateSettingsRequest(INDEX_NAME).settings(
Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "100s")
)
)
.actionGet();
assertEquals(100, indexService.getRefreshTaskInterval().seconds());

client(clusterManagerName).admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), randomFrom("-1", "-1ms")))
.get();

client(clusterManagerName).admin()
.indices()
.updateSettings(
new UpdateSettingsRequest(INDEX_NAME).settings(
Settings.builder().putNull(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey())
)
)
.actionGet();
assertEquals(-1, indexService.getRefreshTaskInterval().millis());
}

private IndexService getIndexServiceFromRandomDataNode(String indexName) throws ExecutionException, InterruptedException {
String clusterManagerName = internalCluster().getClusterManagerName();
List<String> dataNodes = new ArrayList<>(internalCluster().getDataNodeNames());
GetIndexResponse getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get();
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes));
String uuid = getIndexResponse.getSettings().get(indexName).get(IndexMetadata.SETTING_INDEX_UUID);
return indicesService.indexService(new Index(indexName, uuid));
}

protected TimeValue getDefaultRefreshInterval() {
return IndexSettings.DEFAULT_REFRESH_INTERVAL;
}
Expand Down
8 changes: 7 additions & 1 deletion server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1497,10 +1497,16 @@ public static void validateTranslogRetentionSettings(Settings indexSettings) {
* @param clusterSettings cluster setting
*/
public static void validateRefreshIntervalSettings(Settings requestSettings, ClusterSettings clusterSettings) {
if (IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.exists(requestSettings) == false) {
if (IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.exists(requestSettings) == false
|| requestSettings.get(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey()) == null) {
return;
}
TimeValue requestRefreshInterval = IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.get(requestSettings);
// If the refresh interval supplied is -1, we allow the index to be created because -1 means no periodic refresh.
if (requestRefreshInterval.millis() < 0) {
assert requestRefreshInterval.millis() == -1;
return;
}
TimeValue clusterMinimumRefreshInterval = clusterSettings.get(IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING);
if (requestRefreshInterval.millis() < clusterMinimumRefreshInterval.millis()) {
throw new IllegalArgumentException(
Expand Down
7 changes: 5 additions & 2 deletions server/src/main/java/org/opensearch/indices/IndicesService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ public class IndicesService extends AbstractLifecycleComponent
*/
public static final Setting<TimeValue> CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING = Setting.timeSetting(
"cluster.minimum.index.refresh_interval",
IndexSettings.MINIMUM_REFRESH_INTERVAL,
IndexSettings.MINIMUM_REFRESH_INTERVAL,
TimeValue.ZERO,
TimeValue.ZERO,
new ClusterMinimumRefreshIntervalValidator(),
Property.NodeScope,
Property.Dynamic
Expand Down Expand Up @@ -1993,6 +1993,9 @@ public Iterator<Setting<?>> settings() {
* @param defaultRefreshInterval value of cluster default index refresh interval setting
*/
private static void validateRefreshIntervalSettings(TimeValue minimumRefreshInterval, TimeValue defaultRefreshInterval) {
if (defaultRefreshInterval.millis() < 0) {
return;
}
if (minimumRefreshInterval.compareTo(defaultRefreshInterval) > 0) {
throw new IllegalArgumentException(
"cluster minimum index refresh interval ["
Expand Down

0 comments on commit c7f4531

Please sign in to comment.