RecoveryState clean up #9811
Closed
RecoveryState clean up #9811
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing 100% there is nothing to recover.
|
oooh nice I will review... |
| } | ||
| if (peerRecoveryState != null) { | ||
| RecoveryState recoveryState = indexShard.recoveryState(); | ||
| if (recoveryState == null) { |
There was a problem hiding this comment.
weird... this if has an empty body?
|
left some comments.... I think this should not go into 1.4 since it's pretty big though... |
bleskes
force-pushed
the
recovery_state_cleanup
branch
from
c5f43d1
to
f8d5cb1
Compare
|
@s1monw I pushed another round, with more simplifications. |
|
I had one minor comments - LGTM otherwise |
bleskes
added a commit
to bleskes/elasticsearch
that referenced
this pull request
Feb 25, 2015
To support the `_recovery` API, the recovery process keeps track of current progress in a class called RecoveryState. This class currently have some issues, mostly around concurrency (see elastic#6644 ). This PR cleans it up as well as other issues around it: - Make the Index subsection API cleaner: - remove redundant information - all calculation is done based on the underlying file map - clearer definition of what is what: total files, vs reused files (local files that match the source) vs recovered files (copied over). % based progress is reported based on recovered files only. - cleaned up json response to match other API (sadly this breaks the structure). We now properly report human values for dates and other units. - Add more robust unit testing - Detail flag was passed along as state (it's now a ToXContent param) - State lookup during reporting is now always done via the IndexShard , no more fall backs to many other classes. - Cleanup APIs around time and move the little computations to the state class as opposed to doing them out of the API I also improved error messages out of the REST testing infra for things I run into. Closes elastic#6644 Closes elastic#9811
clintongormley
added
the
:Distributed/Recovery
clintongormley
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To support the
_recoveryAPI, the recovery process keeps track of current progress in a class called RecoveryState. This class currently have some issues, mostly around concurrency (see #6644 ). This PR cleans it up as well as other issues around it:I also improved error messages out of the REST testing infra for things I run into.
Given the BWC nature of the change I'm on the fence whether this should go into 1.4.X - it does fix a concurrency issue and makes things consistent where they weren't.
Closes #6644