diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java index d573a7a17e322..d35e9a2d19a7a 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java @@ -464,7 +464,7 @@ public static boolean okayToDeleteSnapshots(ClusterState state) { // Cannot delete during a restore final RestoreInProgress restoreInProgress = state.custom(RestoreInProgress.TYPE); - if (restoreInProgress != null) { + if (restoreInProgress != null && restoreInProgress.isEmpty() == false) { return false; } @@ -498,6 +498,7 @@ public void onNewClusterState(ClusterState state) { logger.debug("retrying SLM snapshot retention deletion after snapshot operation has completed"); reRun.accept(state); } else { + logger.trace("received new cluster state but a snapshot operation is still running"); observer.waitForNextChange(this); } } catch (Exception e) { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java index 34d208337f1ff..ec06bd28e1c16 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java @@ -8,6 +8,9 @@ import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotAction; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; +import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; import org.elasticsearch.action.index.IndexRequestBuilder; @@ -22,6 +25,7 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.RepositoryException; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.snapshots.ConcurrentSnapshotExecutionException; import org.elasticsearch.snapshots.SnapshotInfo; import org.elasticsearch.snapshots.SnapshotMissingException; @@ -31,6 +35,7 @@ import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.ilm.LifecycleSettings; +import org.elasticsearch.xpack.core.slm.SnapshotInvocationRecord; import org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicy; import org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicyItem; import org.elasticsearch.xpack.core.slm.SnapshotRetentionConfiguration; @@ -415,6 +420,74 @@ private void testUnsuccessfulSnapshotRetention(boolean partialSuccess) throws Ex } } + public void testSLMRetentionAfterRestore() throws Exception { + final String indexName = "test"; + final String policyName = "test-policy"; + int docCount = 20; + for (int i = 0; i < docCount; i++) { + index(indexName, i + "", Collections.singletonMap("foo", "bar")); + } + + // Create a snapshot repo + initializeRepo(REPO); + + logger.info("--> creating policy {}", policyName); + createSnapshotPolicy(policyName, "snap", NEVER_EXECUTE_CRON_SCHEDULE, REPO, indexName, true, false, + new SnapshotRetentionConfiguration(TimeValue.ZERO, null, null)); + + logger.info("--> executing snapshot lifecycle"); + final String snapshotName = executePolicy(policyName); + + // Check that the executed snapshot shows up in the SLM output + assertBusy(() -> { + GetSnapshotLifecycleAction.Response getResp = + client().execute(GetSnapshotLifecycleAction.INSTANCE, new GetSnapshotLifecycleAction.Request(policyName)).get(); + logger.info("--> checking for in progress snapshot..."); + + assertThat(getResp.getPolicies().size(), greaterThan(0)); + SnapshotLifecyclePolicyItem item = getResp.getPolicies().get(0); + SnapshotInvocationRecord lastSuccess = item.getLastSuccess(); + assertNotNull(lastSuccess); + assertThat(lastSuccess.getSnapshotName(), equalTo(snapshotName)); + }); + + logger.info("--> restoring index"); + RestoreSnapshotRequest restoreReq = new RestoreSnapshotRequest(REPO, snapshotName); + restoreReq.indices(indexName); + restoreReq.renamePattern("(.+)"); + restoreReq.renameReplacement("restored_$1"); + restoreReq.waitForCompletion(true); + RestoreSnapshotResponse resp = client().execute(RestoreSnapshotAction.INSTANCE, restoreReq).get(); + assertThat(resp.status(), equalTo(RestStatus.OK)); + + logger.info("--> executing SLM retention"); + assertAcked(client().execute(ExecuteSnapshotRetentionAction.INSTANCE, new ExecuteSnapshotRetentionAction.Request()).get()); + logger.info("--> waiting for {} snapshot to be deleted", snapshotName); + assertBusy(() -> { + try { + try { + GetSnapshotsResponse snapshotsStatusResponse = client().admin().cluster() + .prepareGetSnapshots(REPO).setSnapshots(snapshotName).get(); + assertThat(snapshotsStatusResponse.getSnapshots(REPO), empty()); + } catch (SnapshotMissingException e) { + // This is what we want to happen + } + logger.info("--> snapshot [{}] has been deleted", snapshotName); + } catch (RepositoryException re) { + // Concurrent status calls and write operations may lead to failures in determining the current repository generation + // TODO: Remove this hack once tracking the current repository generation has been made consistent + throw new AssertionError(re); + } + }); + + // Cancel/delete the snapshot + try { + client().admin().cluster().prepareDeleteSnapshot(REPO, snapshotName).get(); + } catch (SnapshotMissingException e) { + // ignore + } + } + private SnapshotsStatusResponse getSnapshotStatus(String snapshotName) { try { return client().admin().cluster().prepareSnapshotStatus(REPO).setSnapshots(snapshotName).get(); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java index ede01f35b04b0..91887f3146aec 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java @@ -67,6 +67,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class SnapshotRetentionTaskTests extends ESTestCase { @@ -363,6 +364,14 @@ public void testOkToDeleteSnapshots() { .build(); assertThat(SnapshotRetentionTask.okayToDeleteSnapshots(state), equalTo(false)); + + restoreInProgress = mock(RestoreInProgress.class); + when(restoreInProgress.isEmpty()).thenReturn(true); + state = ClusterState.builder(new ClusterName("cluster")) + .putCustom(RestoreInProgress.TYPE, restoreInProgress) + .build(); + + assertThat(SnapshotRetentionTask.okayToDeleteSnapshots(state), equalTo(true)); } public void testSkipWhileStopping() throws Exception { @@ -420,10 +429,10 @@ private void doTestRunManuallyDuringMode(OperationMode mode) throws Exception { final String repoId = "repo"; SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy(policyId, "snap", "1 * * * * ?", repoId, null, new SnapshotRetentionConfiguration(TimeValue.timeValueDays(30), null, null)); - + ClusterState state = createState(mode, policy); ClusterServiceUtils.setState(clusterService, state); - + AtomicBoolean retentionWasRun = new AtomicBoolean(false); MockSnapshotRetentionTask task = new MockSnapshotRetentionTask(noOpClient, clusterService, new SnapshotLifecycleTaskTests.VerifyingHistoryStore(noOpClient, ZoneOffset.UTC, (historyItem) -> { @@ -436,10 +445,10 @@ private void doTestRunManuallyDuringMode(OperationMode mode) throws Exception { (deletionPolicyId, repo, snapId, slmStats, listener) -> { }, System::nanoTime); - + long time = System.currentTimeMillis(); task.triggered(new SchedulerEngine.Event(SnapshotRetentionService.SLM_RETENTION_MANUAL_JOB_ID, time, time)); - + assertTrue("retention should be run manually even if SLM is disabled", retentionWasRun.get()); } finally { threadPool.shutdownNow();