From 6bd35098cd4ebc75fa5bcc01e455729f62a2c747 Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Tue, 10 Mar 2020 08:02:02 +0100 Subject: [PATCH] ILM Freeze step retry when not acknowledged (#53287) A freeze operation can partially fail in multiple places, including the close verification step. This left the index in an unfrozen but partially closed state. Now throw an exception to retry the freeze step instead. --- .../xpack/core/ilm/FreezeStep.java | 8 ++++- .../xpack/core/ilm/FreezeStepTests.java | 31 ++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java index 3b67e5bdc17d3..fa5f4c13491d6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.ilm; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; @@ -26,7 +27,12 @@ public FreezeStep(StepKey key, StepKey nextStepKey, Client client) { public void performDuringNoSnapshot(IndexMetaData indexMetaData, ClusterState currentState, Listener listener) { getClient().admin().indices().execute(FreezeIndexAction.INSTANCE, new FreezeRequest(indexMetaData.getIndex().getName()).masterNodeTimeout(getMasterTimeout(currentState)), - ActionListener.wrap(response -> listener.onResponse(true), listener::onFailure)); + ActionListener.wrap(response -> { + if (response.isAcknowledged() == false) { + throw new ElasticsearchException("freeze index request failed to be acknowledged"); + } + listener.onResponse(true); + }, listener::onFailure)); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java index 8cd8a2ef0b36d..8dad3cf3a4a81 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.protocol.xpack.frozen.FreezeRequest; +import org.elasticsearch.protocol.xpack.frozen.FreezeResponse; import org.elasticsearch.xpack.core.frozen.action.FreezeIndexAction; import org.elasticsearch.xpack.core.ilm.Step.StepKey; import org.mockito.Mockito; @@ -73,7 +74,7 @@ public void testFreeze() { assertNotNull(request); assertEquals(1, request.indices().length); assertEquals(indexMetaData.getIndex().getName(), request.indices()[0]); - listener.onResponse(null); + listener.onResponse(new FreezeResponse(true, true)); return null; }).when(indicesClient).execute(Mockito.any(), Mockito.any(), Mockito.any()); @@ -127,4 +128,32 @@ public void onFailure(Exception e) { assertThat(exceptionThrown.get(), equalTo(true)); } + + public void testNotAcknowledged() { + IndexMetaData indexMetaData = getIndexMetaData(); + + Mockito.doAnswer(invocation -> { + @SuppressWarnings("unchecked") + ActionListener listener = (ActionListener) invocation.getArguments()[2]; + listener.onResponse(new FreezeResponse(false, false)); + return null; + }).when(indicesClient).execute(Mockito.any(), Mockito.any(), Mockito.any()); + + SetOnce exceptionThrown = new SetOnce<>(); + FreezeStep step = createRandomInstance(); + step.performAction(indexMetaData, emptyClusterState(), null, new AsyncActionStep.Listener() { + @Override + public void onResponse(boolean complete) { + throw new AssertionError("Unexpected method call"); + } + + @Override + public void onFailure(Exception e) { + assertEquals("freeze index request failed to be acknowledged", e.getMessage()); + exceptionThrown.set(true); + } + }); + + assertThat(exceptionThrown.get(), equalTo(true)); + } }