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

Extract all shard-level snapshot operation into dedicated SnapshotShardsService #11756

Merged
merged 1 commit into from Jun 23, 2015

Conversation

imotov
Copy link
Contributor

@imotov imotov commented Jun 18, 2015

Currently the SnapshotsService is concerned with both maintaining the global snapshot lifecycle on the master node as well as responsible for keeping track of individual shards on the data nodes. This refactoring separates two areas of concerns by moving all shard-level operations into a separate SnapshotShardsService.

@imotov imotov added v2.0.0-beta1 review :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Jun 18, 2015
*/
public static boolean completed(Collection<ShardSnapshotStatus> shards) {
for (ShardSnapshotStatus status : shards) {
if (!status.state().completed()) {
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 == false

@s1monw
Copy link
Contributor

s1monw commented Jun 22, 2015

left some style comments LGTM in general

@imotov
Copy link
Contributor Author

imotov commented Jun 23, 2015

@s1monw I pushed the changes, but I am not 100% sure I what you meant in some of your comments. Could you take another look?

@s1monw
Copy link
Contributor

s1monw commented Jun 23, 2015

left a tiny comment about naming LGTM - feel free to push

…rdsService

Currently the SnapshotsService is concerned with both maintaining the global snapshot lifecycle on the master node as well as responsible for keeping track of individual shards on the data nodes. This refactoring separates two areas of concerns by moving all shard-level operations into a separate SnapshotShardsService.

Closes elastic#11756
@imotov
Copy link
Contributor Author

imotov commented Jun 23, 2015

@s1monw, yes, that looks like a bug, I would like to backport the fix to 1.x and 1.6 branches. Could you review #11839.

imotov added a commit to imotov/elasticsearch that referenced this pull request Jun 26, 2015
…ages in the snapshot process

If the abort command is issued very early in shard snapshot lifecycle, it might not cancel this shard. This commit backports the change discovered and fixed as part of elastic#11756

Closes elastic#11839
imotov added a commit that referenced this pull request Jun 26, 2015
…ages in the snapshot process

If the abort command is issued very early in shard snapshot lifecycle, it might not cancel this shard. This commit backports the change discovered and fixed as part of #11756

Closes #11839
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
…ages in the snapshot process

If the abort command is issued very early in shard snapshot lifecycle, it might not cancel this shard. This commit backports the change discovered and fixed as part of elastic#11756

Closes elastic#11839
@imotov imotov deleted the extract-snapshotshardsservice branch May 1, 2020 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants