Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lower bound on poll_interval #39593

Merged
merged 1 commit into from Mar 5, 2019
Merged

Conversation

tawAsh1
Copy link
Contributor

@tawAsh1 tawAsh1 commented Mar 1, 2019

#39163

Add lower bound on poll_interval

$ curl -H "Content-Type: application/json" -X PUT -d '{
  "persistent": {
    "indices.lifecycle.poll_interval": "1ms",
    "logger.org.elasticsearch.xpack.indexlifecycle": "TRACE"
  }
}' localhost:9200/_cluster/settings

{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"failed to parse value [1ms] for setting [indices.lifecycle.poll_interval], must be >= [1s]"}],"type":"illegal_argument_exception","reason":"failed to parse value [1ms] for setting [indices.lifecycle.poll_interval], must be >= [1s]"},"status":400}

@jakelandis jakelandis added the :Data Management/ILM+SLM Index and Snapshot lifecycle management label Mar 3, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@gwbrown
Copy link
Contributor

gwbrown commented Mar 4, 2019

@elasticmachine test this please

I think we may have to adjust some of the places we set this setting in our tests as well, so this test run may fail.

@tawAsh1
Copy link
Contributor Author

tawAsh1 commented Mar 5, 2019

There are tests which is related to this setting.
All tests set poll_interval to 1 sec or more.
Therefore, this change probably does not affect the test results.

public void testPollIntervalUpdate() throws Exception {
TimeValue pollInterval = TimeValue.timeValueSeconds(randomLongBetween(1, 5));
final String server_1 = internalCluster().startMasterOnlyNode(
Settings.builder().put(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, pollInterval.getStringRep()).build());
IndexLifecycleService indexLifecycleService = internalCluster().getInstance(IndexLifecycleService.class, server_1);
assertBusy(() -> {
assertNotNull(indexLifecycleService.getScheduler());
assertThat(indexLifecycleService.getScheduler().jobCount(), equalTo(1));
});
{
TimeValueSchedule schedule = (TimeValueSchedule) indexLifecycleService.getScheduledJob().getSchedule();
assertThat(schedule.getInterval(), equalTo(pollInterval));
}
// update the poll interval
TimeValue newPollInterval = TimeValue.timeValueHours(randomLongBetween(6, 1000));
Settings newIntervalSettings = Settings.builder().put(LifecycleSettings.LIFECYCLE_POLL_INTERVAL,
newPollInterval.getStringRep()).build();
assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(newIntervalSettings));
{
TimeValueSchedule schedule = (TimeValueSchedule) indexLifecycleService.getScheduledJob().getSchedule();
assertThat(schedule.getInterval(), equalTo(newPollInterval));
}
}

Settings settings = Settings.builder().put(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, "1s").build();
when(clusterService.getClusterSettings()).thenReturn(new ClusterSettings(settings,
Collections.singleton(LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING)));
when(clusterService.lifecycleState()).thenReturn(State.STARTED);

@gwbrown
Copy link
Contributor

gwbrown commented Mar 5, 2019

You're right! I was actually thinking of things like this:

setting 'indices.lifecycle.poll_interval', '1000ms'

Where the setting is defined in the Gradle setup for our tests - I thought some of them used poll intervals of <1s, but I was wrong. Thanks for your contribution @tawAsh1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants