Skip to content

Conversation

@talevy
Copy link
Contributor

@talevy talevy commented Dec 7, 2018

Tests were failing here because the trigger time overflowed to the point that it was negative.

Closes #35948.

@talevy talevy added >test Issues or PRs that are addressing/adding tests :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Dec 7, 2018
@talevy talevy requested review from colings86 and gwbrown December 7, 2018 18:23
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

public void setUpStartAndInterval() {
// start with random epoch between 1/1/1970 and 31/12/2035 so that start is not
// so large such that (start + interval) > Long.MAX
start = randomLongBetween(0, 2082672000000L);
Copy link
Contributor

@gwbrown gwbrown Dec 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something but I don't quite get the math here - this number + 10,000 days (from createRandomTimeValue) is way less than Long.MAX (2^63 - 1)? Are you just giving it plenty of headroom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, why not be more restrictive :). doing randomNonnegativeLong was too large of a range

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought I would leave a comment to explain why the magic numbers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, cool - I just read this the first time as saying it had to be 31/12/2035 (or less) to prevent an overflow, rather than that being somewhat arbitrary.

@talevy talevy merged commit ed7afd1 into elastic:master Dec 10, 2018
@talevy talevy deleted the fix-35948 branch December 10, 2018 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/ILM+SLM Index and Snapshot lifecycle management >test Issues or PRs that are addressing/adding tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants