Skip to content

Commit

Permalink
Avoid indexing anything into SLM history indices when SLM disabled
Browse files Browse the repository at this point in the history
This commit adds a check to the `SnapshotHistoryStore.putAsync` method
that checks to see whether SLM is enabled prior to indexing the history
document.

Resolves failures from elastic#46508 that were caused by a retention job being
kicked off, the test finishing and deleting all indices (including the
`.slm-history-*` index), and then retention completing and indexing
another history document. This caused the `.slm-history-*` index to be
re-created and the test to fail due to the shard lock check in
`InternalTestCluster.assertAfterTest` that checks all shards being
unlocked.
  • Loading branch information
dakrone committed Sep 30, 2019
1 parent bc96370 commit 10f2fe1
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
package org.elasticsearch.xpack.core.ilm;

import org.elasticsearch.action.admin.indices.shrink.ShrinkAction;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.xpack.core.slm.SnapshotLifecycleMetadata;

import java.util.Optional;

/**
* Enum representing the different modes that Index Lifecycle Service can operate in.
Expand Down Expand Up @@ -44,4 +48,24 @@ public boolean isValidChange(OperationMode nextMode) {
};

public abstract boolean isValidChange(OperationMode nextMode);

/**
* Returns true if ILM is in the stopped or stopped state
*/
public static boolean ilmStoppedOrStopping(ClusterState state) {
return Optional.ofNullable((IndexLifecycleMetadata) state.metaData().custom(IndexLifecycleMetadata.TYPE))
.map(IndexLifecycleMetadata::getOperationMode)
.map(mode -> OperationMode.STOPPING == mode || OperationMode.STOPPED == mode)
.orElse(false);
}

/**
* Returns true if SLM is in the stopped or stopped state
*/
public static boolean slmStoppedOrStopping(ClusterState state) {
return Optional.ofNullable((SnapshotLifecycleMetadata) state.metaData().custom(SnapshotLifecycleMetadata.TYPE))
.map(SnapshotLifecycleMetadata::getOperationMode)
.map(mode -> OperationMode.STOPPING == mode || OperationMode.STOPPED == mode)
.orElse(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.xpack.core.ilm.OperationMode;

import java.io.IOException;

Expand Down Expand Up @@ -59,8 +60,13 @@ public void putAsync(SnapshotHistoryItem item) {
SLM_HISTORY_INDEX_ENABLED_SETTING.getKey(), item);
return;
}
ClusterState state = clusterService.state();
if (OperationMode.slmStoppedOrStopping(state)) {
logger.debug("skipping indexing SLM history item as ILM is currently stopped or stopping");
return;
}
logger.trace("about to index snapshot history item in index [{}]: [{}]", SLM_HISTORY_ALIAS, item);
ensureHistoryIndex(client, clusterService.state(), ActionListener.wrap(createdIndex -> {
ensureHistoryIndex(client, state, ActionListener.wrap(createdIndex -> {
try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
item.toXContent(builder, ToXContent.EMPTY_PARAMS);
IndexRequest request = new IndexRequest(SLM_HISTORY_ALIAS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void clusterChanged(final ClusterChangedEvent event) {
if (this.isMaster) {
final ClusterState state = event.state();

if (ilmStoppedOrStopping(state)) {
if (OperationMode.slmStoppedOrStopping(state)) {
if (scheduler.scheduledJobIds().size() > 0) {
cancelSnapshotJobs();
}
Expand All @@ -82,7 +82,7 @@ public void onMaster() {
this.isMaster = true;
scheduler.register(snapshotTask);
final ClusterState state = clusterService.state();
if (ilmStoppedOrStopping(state)) {
if (OperationMode.slmStoppedOrStopping(state)) {
// ILM is currently stopped, so don't schedule jobs
return;
}
Expand All @@ -101,16 +101,6 @@ SchedulerEngine getScheduler() {
return this.scheduler;
}

/**
* Returns true if ILM is in the stopped or stopped state
*/
static boolean ilmStoppedOrStopping(ClusterState state) {
return Optional.ofNullable((SnapshotLifecycleMetadata) state.metaData().custom(SnapshotLifecycleMetadata.TYPE))
.map(SnapshotLifecycleMetadata::getOperationMode)
.map(mode -> OperationMode.STOPPING == mode || OperationMode.STOPPED == mode)
.orElse(false);
}

/**
* Schedule all non-scheduled snapshot jobs contained in the cluster state
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.ClientHelper;
import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
import org.elasticsearch.xpack.core.ilm.OperationMode;
import org.elasticsearch.xpack.core.scheduler.SchedulerEngine;
import org.elasticsearch.xpack.core.slm.SnapshotLifecycleMetadata;
import org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicy;
Expand Down Expand Up @@ -88,7 +89,7 @@ public void triggered(SchedulerEngine.Event event) {
"expected id to be " + SnapshotRetentionService.SLM_RETENTION_JOB_ID + " but it was " + event.getJobName();

final ClusterState state = clusterService.state();
if (SnapshotLifecycleService.ilmStoppedOrStopping(state)) {
if (OperationMode.slmStoppedOrStopping(state)) {
logger.debug("skipping SLM retention as ILM is currently stopped or stopping");
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

package org.elasticsearch.xpack.slm;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.SnapshotsInProgress;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -20,6 +22,12 @@
import org.elasticsearch.test.junit.annotations.TestLogging;
import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin;
import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
import org.elasticsearch.xpack.core.ilm.OperationMode;
import org.elasticsearch.xpack.core.ilm.StartILMRequest;
import org.elasticsearch.xpack.core.ilm.StopILMRequest;
import org.elasticsearch.xpack.core.ilm.action.GetStatusAction;
import org.elasticsearch.xpack.core.ilm.action.StartILMAction;
import org.elasticsearch.xpack.core.ilm.action.StopILMAction;
import org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicy;
import org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicyItem;
import org.elasticsearch.xpack.core.slm.SnapshotRetentionConfiguration;
Expand All @@ -28,6 +36,7 @@
import org.elasticsearch.xpack.core.slm.action.PutSnapshotLifecycleAction;
import org.elasticsearch.xpack.ilm.IndexLifecycle;
import org.junit.After;
import org.junit.Before;

import java.util.Arrays;
import java.util.Collection;
Expand All @@ -48,6 +57,22 @@ public class SLMSnapshotBlockingIntegTests extends ESIntegTestCase {

static final String REPO = "my-repo";

@Before
public void enableILM() throws Exception {
client().execute(StartILMAction.INSTANCE, new StartILMRequest(), new ActionListener<>() {
@Override
public void onResponse(AcknowledgedResponse acknowledgedResponse) {
assertTrue(acknowledgedResponse.isAcknowledged());
}

@Override
public void onFailure(Exception e) {
logger.error("failed to start ILM", e);
fail("failed to start ILM: " + e);
}
});
}

@After
public void resetSLMSettings() throws Exception {
// unset retention settings
Expand All @@ -70,6 +95,13 @@ public void resetSLMSettings() throws Exception {
}
});
});
// Stop ILM so there are no more history items being written
client().execute(StopILMAction.INSTANCE, new StopILMRequest()).get();
// Wait for ILM to be fully stopped
assertBusy(() -> {
GetStatusAction.Response resp = client().execute(GetStatusAction.INSTANCE, new GetStatusAction.Request()).get();
assertThat(resp.getMode(), equalTo(OperationMode.STOPPED));
});
}

@Override
Expand Down Expand Up @@ -124,7 +156,6 @@ public void testSnapshotInProgress() throws Exception {
}
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/46508")
public void testRetentionWhileSnapshotInProgress() throws Exception {
final String indexName = "test";
final String policyId = "slm-policy";
Expand Down

0 comments on commit 10f2fe1

Please sign in to comment.