Skip to content

Allow date math for naming newly-created snapshots (#7939) #30479

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
May 16, 2018
Merged

Allow date math for naming newly-created snapshots (#7939) #30479

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4055c34
Automatic snapshot naming (#7939)
May 8, 2018
4b20260
Automatic snapshot naming #7939
May 11, 2018
b168ec8
Automatic snapshot naming #7939
May 11, 2018
256c3c7
#7939 dropped unnecessary new lines in SnapshotBlocksIT
May 15, 2018
117495f
#7939 use simple get() instead of execute().actionGet()
May 15, 2018
ff244bd
#7939 fix test taking into account potential day rollover
May 15, 2018
4648145
#7939 doc adjusted
May 15, 2018
04404d0
Merge remote-tracking branch 'remotes/origin/master' into 7939
May 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/reference/modules/snapshots.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ PUT /_snapshot/my_backup/snapshot_2?wait_for_completion=true
// TEST[continued]

The list of indices that should be included into the snapshot can be specified using the `indices` parameter that
supports <<multi-index,multi index syntax>>. The snapshot request also supports the
supports <<multi-index,multi index syntax>>. It can be useful to use <<date-math-index-names,date math>> to name the
Copy link
Contributor

Choose a reason for hiding this comment

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

This links to docs on using date math for index names. This can be confusing. I think you need to clarify here that the same date math expression that is used for indices can be used for snapshot names. I think it would be better to put this info into a separate paragraph, together with an example. I wonder if we need to support date math not only for snapshot creation, but also for other operations.

snapshot according to the date that the snapshot made, e.g. `snapshot-2018.05.09`. The snapshot request also supports the
`ignore_unavailable` option. Setting it to `true` will cause indices that do not exist to be ignored during snapshot
creation. By default, when `ignore_unavailable` option is not set and an index is missing the snapshot request will fail.
By setting `include_global_state` to false it's possible to prevent the cluster global state to be stored as part of
Expand Down
7 changes: 4 additions & 3 deletions ...rg/elasticsearch/action/admin/cluster/snapshots/create/TransportCreateSnapshotAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ protected ClusterBlockException checkBlock(CreateSnapshotRequest request, Cluste

@Override
protected void masterOperation(final CreateSnapshotRequest request, ClusterState state, final ActionListener<CreateSnapshotResponse> listener) {
final String snapshotName = indexNameExpressionResolver.resolveDateMathExpression(request.snapshot());
SnapshotsService.SnapshotRequest snapshotRequest =
new SnapshotsService.SnapshotRequest(request.repository(), request.snapshot(), "create_snapshot [" + request.snapshot() + "]")
new SnapshotsService.SnapshotRequest(request.repository(), snapshotName, "create_snapshot [" + snapshotName + "]")
.indices(request.indices())
.indicesOptions(request.indicesOptions())
.partial(request.partial())
Expand All @@ -87,7 +88,7 @@ public void onResponse() {
@Override
public void onSnapshotCompletion(Snapshot snapshot, SnapshotInfo snapshotInfo) {
if (snapshot.getRepository().equals(request.repository()) &&
snapshot.getSnapshotId().getName().equals(request.snapshot())) {
snapshot.getSnapshotId().getName().equals(snapshotName)) {
listener.onResponse(new CreateSnapshotResponse(snapshotInfo));
snapshotsService.removeListener(this);
}
Expand All @@ -96,7 +97,7 @@ public void onSnapshotCompletion(Snapshot snapshot, SnapshotInfo snapshotInfo) {
@Override
public void onSnapshotFailure(Snapshot snapshot, Exception e) {
if (snapshot.getRepository().equals(request.repository()) &&
snapshot.getSnapshotId().getName().equals(request.snapshot())) {
snapshot.getSnapshotId().getName().equals(snapshotName)) {
listener.onFailure(e);
snapshotsService.removeListener(this);
}
Expand Down
27 changes: 27 additions & 0 deletions server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/SnapshotBlocksIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.RestStatus;
Expand Down Expand Up @@ -176,4 +177,30 @@ public void testSnapshotStatusWithBlocks() {
setClusterReadOnly(false);
}
}

public void testSnapshotWithDateMath() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this is exactly testing. There are no assertions that check that the date math functionality has worked in a meaningful way. Also why is this testing blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it tests that it is possible to create snapshot with name <snapshot-{now/d}> and it is resolved into smth like snapshot-2018.05.11 using nameExpressionResolver.resolveDateMathExpression(snapshotName) - I assume that nameExpressionResolver works.

Agree - it is worth to move test to DedicatedClusterSnapshotRestoreIT - is that ok ?

// This test checks that the Snapshot Status operation is never blocked, even if the cluster is read only.
try {
setClusterReadOnly(true);

final IndexNameExpressionResolver nameExpressionResolver = new IndexNameExpressionResolver(Settings.EMPTY);
final String snapshotName = "<snapshot-{now/d}>";
final String expression = nameExpressionResolver.resolveDateMathExpression(snapshotName);

Copy link
Contributor

Choose a reason for hiding this comment

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

can you revert this?

CreateSnapshotResponse snapshotResponse =
client().admin().cluster().prepareCreateSnapshot(REPOSITORY_NAME, snapshotName)
.setIncludeGlobalState(true)
.setWaitForCompletion(true)
.execute().actionGet();
assertThat(snapshotResponse.status(), equalTo(RestStatus.OK));

SnapshotsStatusResponse response = client().admin().cluster().prepareSnapshotStatus(REPOSITORY_NAME)
.setSnapshots(expression)
.execute().actionGet();
assertThat(response.getSnapshots(), hasSize(1));
assertThat(response.getSnapshots().get(0).getState().completed(), equalTo(true));
} finally {
setClusterReadOnly(false);
}
}
}