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

Refactor state format to use incremental state IDs #10316

Merged
merged 1 commit into from Mar 31, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Mar 30, 2015

Today there is a chance that the state version for shard, index or cluster
state goes backwards or is reset on a full restart etc. depending on
several factors not related to the state. To prevent any collisions
with already existing state files and to maintain write-once properties
this change introductes an incremental state ID instead of using the plain
state version. This also fixes a bug when the previous legacy state had a
greater version than the current state which causes an exception on node
startup or if left-over files are present.

this.format = format;
this.deleteOldFiles = deleteOldFiles;
this.prefix = prefix;
this.stateFilePattern = Pattern.compile(prefix + "(\\d+)(" + MetaDataStateFormat.STATE_FILE_EXTENSION + ")?");
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use Pattern.qoute for the prefix? just being paranoid..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@s1monw
Copy link
Contributor Author

s1monw commented Mar 30, 2015

@bleskes pushed a new commit

}
};
long maxId = -1;
// now clean up the old files
Copy link
Contributor

Choose a reason for hiding this comment

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

left over comment.

@bleskes
Copy link
Contributor

bleskes commented Mar 30, 2015

I left one comment regarding the use of versions in determining the counter. Another thing I was wondering about is how we deal with the scenario 1.5.0 can live us in - a higher id legacy file, with a lower id and non legacy file.

@s1monw
Copy link
Contributor Author

s1monw commented Mar 30, 2015

I left one comment regarding the use of versions in determining the counter. Another thing I was wondering about is how we deal with the scenario 1.5.0 can live us in - a higher id legacy file, with a lower id and non legacy file.

we don't deal with that at all. IMO this requires user interaction - no way to resolve it automatically and I don't think we should unless it's evident that there is a real problem here that happens regularly.

@bleskes
Copy link
Contributor

bleskes commented Mar 30, 2015

I don't think we should unless it's evident that there is a real problem here that happens regularly.

Fair enough. Let's wait and see.

@s1monw
Copy link
Contributor Author

s1monw commented Mar 31, 2015

@bleskes can you take another look?

* the given one.
*/
private static final class VersionAndLegacyPredicate implements Predicate<PathAndVersion> {
private final long version;
private static final class VersionAndLegacyPredicate implements Predicate<PathAndStateId> {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename this to StateIdAndLegacyPredicate?

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 good catch

@bleskes
Copy link
Contributor

bleskes commented Mar 31, 2015

LGTM

Today there is a chance that the state version for shard, index or cluster
state goes backwards or is reset on a full restart etc. depending on
several factors not related to the state. To prevent any collisions
with already existing state files and to maintain write-once properties
this change introductes an incremental state ID instead of using the plain
state version. This also fixes a bug when the previous legacy state had a
greater version than the current state which causes an exception on node
startup or if left-over files are present.

Closes elastic#10316
@s1monw s1monw merged commit 78d86bc into elastic:master Mar 31, 2015
@s1monw s1monw deleted the refactor_state_format branch March 31, 2015 12:27
s1monw added a commit to s1monw/elasticsearch that referenced this pull request Apr 1, 2015
Today there is a chance that the state version for shard, index or cluster
state goes backwards or is reset on a full restart etc. depending on
several factors not related to the state. To prevent any collisions
with already existing state files and to maintain write-once properties
this change introductes an incremental state ID instead of using the plain
state version. This also fixes a bug when the previous legacy state had a
greater version than the current state which causes an exception on node
startup or if left-over files are present.

Closes elastic#10316
s1monw added a commit to s1monw/elasticsearch that referenced this pull request Apr 1, 2015
Today there is a chance that the state version for shard, index or cluster
state goes backwards or is reset on a full restart etc. depending on
several factors not related to the state. To prevent any collisions
with already existing state files and to maintain write-once properties
this change introductes an incremental state ID instead of using the plain
state version. This also fixes a bug when the previous legacy state had a
greater version than the current state which causes an exception on node
startup or if left-over files are present.

Closes elastic#10316
@clintongormley clintongormley added resiliency :Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. labels Apr 9, 2015
@clintongormley clintongormley changed the title [STATE] Refactor state format to use incremental state IDs Refactor state format to use incremental state IDs May 29, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
Today there is a chance that the state version for shard, index or cluster
state goes backwards or is reset on a full restart etc. depending on
several factors not related to the state. To prevent any collisions
with already existing state files and to maintain write-once properties
this change introductes an incremental state ID instead of using the plain
state version. This also fixes a bug when the previous legacy state had a
greater version than the current state which causes an exception on node
startup or if left-over files are present.

Closes elastic#10316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. resiliency v1.5.1 v1.6.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants