Skip to content

Add lower bound on poll_interval#39593

Merged
AthenaEryma merged 1 commit intoelastic:masterfrom
tawAsh1:add-lowerbound
Mar 5, 2019
Merged

Add lower bound on poll_interval#39593
AthenaEryma merged 1 commit intoelastic:masterfrom
tawAsh1:add-lowerbound

Conversation

@tawAsh1
Copy link
Copy Markdown
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 DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. label Mar 3, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Mar 3, 2019

Pinging @elastic/es-core-features

@AthenaEryma
Copy link
Copy Markdown
Contributor

AthenaEryma 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.

@dakrone dakrone mentioned this pull request Mar 4, 2019
Closed
@tawAsh1
Copy link
Copy Markdown
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.

elasticsearch/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleInitialisationTests.java

Lines 404 to 427 in d79cd8c

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));
}
}

elasticsearch/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleServiceTests.java

Lines 92 to 95 in ecf6af4

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);

@AthenaEryma
Copy link
Copy Markdown
Contributor

AthenaEryma commented Mar 5, 2019

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

elasticsearch/x-pack/plugin/ilm/qa/multi-cluster/build.gradle

Line 25 in 89b80c6

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!

@AthenaEryma AthenaEryma merged commit dc613b0 into elastic:master Mar 5, 2019
@AthenaEryma AthenaEryma self-requested a review March 5, 2019 18:58
@AthenaEryma AthenaEryma mentioned this pull request Apr 10, 2019
Merged
@AthenaEryma AthenaEryma mentioned this pull request May 22, 2019
Closed
80 tasks
@jakelandis jakelandis mentioned this pull request Feb 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AthenaEryma AthenaEryma Awaiting requested review from AthenaEryma

Assignees

No one assigned

Labels

>breaking :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. >enhancement v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tawAsh1 @elasticmachine @AthenaEryma @jakelandis