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

Snapshot/Restore: Add ability to restore partial snapshots #6368

Merged
merged 1 commit into from Jul 1, 2014

Conversation

Projects
None yet
4 participants
@imotov
Copy link
Member

imotov commented Jun 1, 2014

Closes #5742

@imotov imotov added the review label Jun 1, 2014

@s1monw

View changes

...org/elasticsearch/action/admin/cluster/snapshots/restore/TransportRestoreSnapshotAction.java Outdated
@@ -82,6 +82,7 @@ protected void masterOperation(final RestoreSnapshotRequest request, ClusterStat
.renamePattern(request.renamePattern())
.renameReplacement(request.renameReplacement())
.includeGlobalState(request.includeGlobalState())

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

this one becomes too dangerous IMO. It's the only place we use this class and we risk to miss passing on a parameter. IMO we should remove all the setters and make this a struct like immutalbe class taking all the values here as ctor args. The information what they are is already there via the getter and the compiler should tell us if we have to pass on any further information!

@s1monw

View changes

src/main/java/org/elasticsearch/snapshots/RestoreService.java Outdated
throw new SnapshotRestoreException(snapshotId, "unsupported snapshot state [" + snapshot.state() + "]");
}
if (Version.CURRENT.before(snapshot.version())) {
throw new SnapshotRestoreException(snapshotId, "incompatible snapshot version [" + snapshot.version() + "]");

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

maybe you can add some more insight into why it is incompatible?

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Jun 12, 2014

added some comments... seems close

@s1monw s1monw removed the review label Jun 12, 2014

@imotov

This comment has been minimized.

Copy link
Member Author

imotov commented Jun 12, 2014

@s1monw - makes sense. Thanks! Fixed.

@s1monw s1monw added the review label Jun 27, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Jun 27, 2014

LGTM

@imotov imotov merged commit 1425e28 into elastic:master Jul 1, 2014

@jpountz jpountz removed the review label Jul 16, 2014

@clintongormley clintongormley changed the title Add ability to restore partial snapshots Snapshot/Restore: Add ability to restore partial snapshots Sep 8, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.