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

[ILM] reduce time restriction on IndexLifecycleExplainResponse #35954

Merged
merged 1 commit into from
Nov 27, 2018

Conversation

talevy
Copy link
Contributor

@talevy talevy commented Nov 27, 2018

step times were set. The assumption was that these are always set.
Tests passed, which led me to believe this was true. There is a time
when shrunk indices have their step phase/action/step details set,
but with no time information (in the CopyExecutionStateStep).
Explain API fails for these

step times were set. The assumption was that these are always set.
Tests passed, which led me to believe this was true. There is a time
when shrunk indices have their step phase/action/step details set,
but with no time information (in the CopyExecutionStateStep).
Explain API fails for these
@talevy talevy added >bug blocker :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Nov 27, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@talevy
Copy link
Contributor Author

talevy commented Nov 27, 2018

I've opened this to discuss an alternative solution that the original commit in the PR does not address. That is to have CopyExecutionStateStep set phase_time,action_time,step_time. To be on the safe side, since this is very cosmetic, I went with just making the constructor more lenient

@gwbrown
Copy link
Contributor

gwbrown commented Nov 27, 2018

CopyExecutionStateStep should probably copy the times as well - the whole idea is to have the shrunken index completely replace the original, so it should take the times with it.

@talevy
Copy link
Contributor Author

talevy commented Nov 27, 2018

@gwbrown I agree. I also think a part of that change should add validation of null values for these relevant fields in the LifecycleExecutionState constructor. I am also not sure what the correct step_time. the best estimate is the time of the step from the original index.

@talevy
Copy link
Contributor Author

talevy commented Nov 27, 2018

test this please

@talevy talevy merged commit c2329cd into elastic:master Nov 27, 2018
@talevy talevy deleted the ilm-fix-time-strictness branch November 27, 2018 19:03
talevy added a commit that referenced this pull request Nov 27, 2018
step times were set. The assumption was that these are always set.
Tests passed, which led me to believe this was true. There is a time
when shrunk indices have their step phase/action/step details set,
but with no time information (in the CopyExecutionStateStep).
Explain API fails for these
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Nov 28, 2018
* master:
  DOCS Audit event attributes in new format (elastic#35510)
  Scripting: Actually add joda time back to whitelist (elastic#35965)
  [DOCS] fix HLRC ILM doc misreferenced tag
  Add realm information for Authenticate API (elastic#35648)
  [ILM] add HLRC docs to remove-policy-from-index (elastic#35759)
  [Rollup] Update serialization version after backport
  [Rollup] Add more diagnostic stats to job (elastic#35471)
  Build: Fix gradle build for Mac OS (elastic#35968)
  Adds deprecation logging to ScriptDocValues#getValues. (elastic#34279)
  [Monitoring] Make Exporters Async (elastic#35765)
  [ILM] reduce time restriction on IndexLifecycleExplainResponse (elastic#35954)
  Remove use of AbstractComponent in xpack (elastic#35394)
  Deprecate types in search and multi search templates. (elastic#35669)
  Remove fromXContent from IndexUpgradeInfoResponse (elastic#35934)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker >bug :Data Management/ILM+SLM Index and Snapshot lifecycle management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants