[ML] Don't mention unrelated indices when deleting job index aliases (elastic/x-pack-elasticsearch#3160)

This change fixes a problem that would cause job deletion to fail if ANY
index had a block on it, e.g. read-only.

The problem was that we were requesting the job aliases be deleted from
ALL indices in the system due to a misunderstanding with the format of the
get_aliases response.  This didn't usually cause any noticable effects, as
only the ML indices would have the aliases.  But in the case of a read-only
index it would cause an error, leading to unnecessary failure of the job
deletion.

Fixes elastic/machine-learning-cpp#428

Original commit: elastic/x-pack-elasticsearch@a573f85a00
This commit is contained in:
David Roberts 2017-11-29 11:39:30 +00:00 committed by GitHub
parent 9ef9edc1ca
commit ef96831515
2 changed files with 72 additions and 11 deletions

View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.ml.job.persistence;
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
@ -17,6 +18,7 @@ import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.common.CheckedConsumer;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Settings;
@ -191,22 +193,14 @@ public class JobStorageDeletionTask extends Task {
executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, aliasesRequest,
ActionListener.<GetAliasesResponse>wrap(
getAliasesResponse -> {
Set<String> aliases = new HashSet<>();
getAliasesResponse.getAliases().valuesIt().forEachRemaining(
metaDataList -> metaDataList.forEach(metadata -> aliases.add(metadata.getAlias())));
if (aliases.isEmpty()) {
// remove the aliases from the concrete indices found in the first step
IndicesAliasesRequest removeRequest = buildRemoveAliasesRequest(getAliasesResponse);
if (removeRequest == null) {
// don't error if the job's aliases have already been deleted - carry on and delete the
// rest of the job's data
finishedHandler.onResponse(true);
return;
}
List<String> indices = new ArrayList<>();
getAliasesResponse.getAliases().keysIt().forEachRemaining(indices::add);
// remove the aliases from the concrete indices found in the first step
IndicesAliasesRequest removeRequest = new IndicesAliasesRequest().addAliasAction(
IndicesAliasesRequest.AliasActions.remove()
.aliases(aliases.toArray(new String[aliases.size()]))
.indices(indices.toArray(new String[indices.size()])));
executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, removeRequest,
ActionListener.<IndicesAliasesResponse>wrap(removeResponse -> finishedHandler.onResponse(true),
finishedHandler::onFailure),
@ -214,4 +208,21 @@ public class JobStorageDeletionTask extends Task {
},
finishedHandler::onFailure), client.admin().indices()::getAliases);
}
private IndicesAliasesRequest buildRemoveAliasesRequest(GetAliasesResponse getAliasesResponse) {
Set<String> aliases = new HashSet<>();
List<String> indices = new ArrayList<>();
for (ObjectObjectCursor<String, List<AliasMetaData>> entry : getAliasesResponse.getAliases()) {
// The response includes _all_ indices, but only those associated with
// the aliases we asked about will have associated AliasMetaData
if (entry.value.isEmpty() == false) {
indices.add(entry.key);
entry.value.forEach(metadata -> aliases.add(metadata.getAlias()));
}
}
return aliases.isEmpty() ? null : new IndicesAliasesRequest().addAliasAction(
IndicesAliasesRequest.AliasActions.remove()
.aliases(aliases.toArray(new String[aliases.size()]))
.indices(indices.toArray(new String[indices.size()])));
}
}

View File

@ -0,0 +1,50 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.ml.integration;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.xpack.ml.action.DeleteJobAction;
import org.elasticsearch.xpack.ml.action.OpenJobAction;
import org.elasticsearch.xpack.ml.action.PutJobAction;
import org.elasticsearch.xpack.ml.job.config.Job;
import org.elasticsearch.xpack.ml.support.BaseMlIntegTestCase;
/**
* Test that ML does not touch unnecessary indices when removing job index aliases
*/
public class JobStorageDeletionTaskIT extends BaseMlIntegTestCase {
private static final String UNRELATED_INDEX = "unrelated-data";
public void testUnrelatedIndexNotTouched() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(1);
ensureStableCluster(1);
client().admin().indices().prepareCreate(UNRELATED_INDEX).get();
enableIndexBlock(UNRELATED_INDEX, IndexMetaData.SETTING_READ_ONLY);
Job.Builder job = createJob("delete-aliases-test-job", new ByteSizeValue(2, ByteSizeUnit.MB));
PutJobAction.Request putJobRequest = new PutJobAction.Request(job);
PutJobAction.Response putJobResponse = client().execute(PutJobAction.INSTANCE, putJobRequest).actionGet();
assertTrue(putJobResponse.isAcknowledged());
OpenJobAction.Request openJobRequest = new OpenJobAction.Request(job.getId());
client().execute(OpenJobAction.INSTANCE, openJobRequest).actionGet();
awaitJobOpenedAndAssigned(job.getId(), null);
DeleteJobAction.Request deleteJobRequest = new DeleteJobAction.Request(job.getId());
deleteJobRequest.setForce(true);
client().execute(DeleteJobAction.INSTANCE, deleteJobRequest).actionGet();
// If the deletion of aliases touches the unrelated index with the block
// then the line above will throw a ClusterBlockException
disableIndexBlock(UNRELATED_INDEX, IndexMetaData.SETTING_READ_ONLY);
}
}