-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Switch to write once mode for snapshot metadata files #8782
Switch to write once mode for snapshot metadata files #8782
Conversation
w00t! - I will review |
public void move(String source, String target) throws IOException { | ||
Path sourcePath = path.resolve(source); | ||
Path targetPath = path.resolve(target); | ||
Files.move(sourcePath, targetPath, new CopyOption[]{StandardCopyOption.ATOMIC_MOVE}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can just be
Files.move(sourcePath, targetPath, StandardCopyOption.ATOMIC_MOVE);
I really like it. I think we should have a small unittest in |
@@ -295,19 +304,19 @@ public ClusterState execute(ClusterState currentState) { | |||
SnapshotMetaData snapshots = metaData.custom(SnapshotMetaData.TYPE); | |||
ImmutableList.Builder<SnapshotMetaData.Entry> entries = ImmutableList.builder(); | |||
for (SnapshotMetaData.Entry entry : snapshots.entries()) { | |||
if (entry.snapshotId().equals(snapshot.snapshotId())) { | |||
if (entry.snapshotId().equals(entry.snapshotId())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that comparison really intended?
The more I dig into the snapshot/restore code, the more I like it :) LGTM, left two comments. |
This commit removes creation of in-progress snapshot file and makes creation of the final snapshot file atomic. Fixes elastic#8696
19338f5
to
0b024ad
Compare
This has been added in elasticsearch 2.0.0. See elastic/elasticsearch#8782 Wondering if this PR should be split in two parts: * one that clean the code and which is portable to 1.4, 1.x and master * one that simply add move() method for master branch @imotov WDYT? Closes elastic#49.
This has been added in elasticsearch 2.0.0. See elastic/elasticsearch#8782 Wondering if this PR should be split in two parts: * one that clean the code and which is portable to 1.4, 1.x and master * one that simply add move() method for master branch @imotov WDYT? Closes #49.
Together with elastic#8782 it should help in the situations simliar to elastic#8887 by adding an ability to get information about currently running snapshot without accessing the repository itself. Closes elastic#8887
This commit removes creation of in-progress snapshot file and makes creation of the final snapshot file atomic.
Fixes #8696