[ML] Prevent UnusuedStatsRemover from failing when deleting documents in read-only indices

docs/changelog/125408.yaml

@@ -0,0 +1,6 @@
+pr: 125408
+summary: Prevent `UnusuedStatsRemover` from failing when deleting documents in read-only
+  indices
+area: Machine Learning
+type: bug
+issues: []

x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/UnusedStatsRemoverIT.java

@@ -10,6 +10,7 @@
 import org.elasticsearch.action.index.IndexRequest;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.client.internal.OriginSettingClient;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.indices.TestIndexNameExpressionResolver;
 import org.elasticsearch.tasks.TaskId;
@@ -37,6 +38,7 @@
 import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 import org.elasticsearch.xpack.ml.inference.persistence.TrainedModelProvider;
 import org.elasticsearch.xpack.ml.job.retention.UnusedStatsRemover;
+import org.elasticsearch.xpack.ml.job.retention.WritableIndexExpander;
 import org.elasticsearch.xpack.ml.support.BaseMlIntegTestCase;
 import org.junit.Before;
 
@@ -47,10 +49,12 @@
 public class UnusedStatsRemoverIT extends BaseMlIntegTestCase {
 
     private OriginSettingClient client;
+    private WritableIndexExpander writableIndexExpander;
 
     @Before
     public void createComponents() {
         client = new OriginSettingClient(client(), ClientHelper.ML_ORIGIN);
+        writableIndexExpander = new WritableIndexExpander(clusterService(), TestIndexNameExpressionResolver.newInstance());
         PlainActionFuture<Boolean> future = new PlainActionFuture<>();
         MlStatsIndex.createStatsIndexAndAliasIfNecessary(
             client(),
@@ -63,91 +67,118 @@ public void createComponents() {
     }
 
     public void testRemoveUnusedStats() throws Exception {
+        String modelId = "model-with-stats";
+        putDFA(modelId);
 
-        prepareIndex("foo").setId("some-empty-doc").setSource("{}", XContentType.JSON).get();
-
-        PutDataFrameAnalyticsAction.Request request = new PutDataFrameAnalyticsAction.Request(
-            new DataFrameAnalyticsConfig.Builder().setId("analytics-with-stats")
-                .setModelMemoryLimit(ByteSizeValue.ofGb(1))
-                .setSource(new DataFrameAnalyticsSource(new String[] { "foo" }, null, null, null))
-                .setDest(new DataFrameAnalyticsDest("bar", null))
-                .setAnalysis(new Regression("prediction"))
-                .build()
-        );
-        client.execute(PutDataFrameAnalyticsAction.INSTANCE, request).actionGet();
-
-        client.execute(
-            PutTrainedModelAction.INSTANCE,
-            new PutTrainedModelAction.Request(
-                TrainedModelConfig.builder()
-                    .setModelId("model-with-stats")
-                    .setInferenceConfig(RegressionConfig.EMPTY_PARAMS)
-                    .setInput(new TrainedModelInput(Arrays.asList("foo", "bar")))
-                    .setParsedDefinition(
-                        new TrainedModelDefinition.Builder().setPreProcessors(Collections.emptyList())
-                            .setTrainedModel(
-                                Tree.builder()
-                                    .setFeatureNames(Arrays.asList("foo", "bar"))
-                                    .setRoot(TreeNode.builder(0).setLeafValue(42))
-                                    .build()
-                            )
-                    )
-                    .validate(true)
-                    .build(),
-                false
-            )
-        ).actionGet();
-
+        // Existing analytics and models
         indexStatDocument(new DataCounts("analytics-with-stats", 1, 1, 1), DataCounts.documentId("analytics-with-stats"));
-        indexStatDocument(new DataCounts("missing-analytics-with-stats", 1, 1, 1), DataCounts.documentId("missing-analytics-with-stats"));
+        indexStatDocument(new InferenceStats(1, 1, 1, 1, modelId, "test", Instant.now()), InferenceStats.docId(modelId, "test"));
         indexStatDocument(
             new InferenceStats(1, 1, 1, 1, TrainedModelProvider.MODELS_STORED_AS_RESOURCE.iterator().next(), "test", Instant.now()),
             InferenceStats.docId(TrainedModelProvider.MODELS_STORED_AS_RESOURCE.iterator().next(), "test")
         );
+
+        // Unused analytics/model stats
+        indexStatDocument(new DataCounts("missing-analytics-with-stats", 1, 1, 1), DataCounts.documentId("missing-analytics-with-stats"));
         indexStatDocument(
             new InferenceStats(1, 1, 1, 1, "missing-model", "test", Instant.now()),
             InferenceStats.docId("missing-model", "test")
         );
+
+        refreshStatsIndex();
+        runUnusedStatsRemover();
+
+        final String index = MlStatsIndex.TEMPLATE_NAME + "-000001";
+
+        // Validate expected docs
+        assertDocExists(index, InferenceStats.docId(modelId, "test"));
+        assertDocExists(index, DataCounts.documentId("analytics-with-stats"));
+        assertDocExists(index, InferenceStats.docId(TrainedModelProvider.MODELS_STORED_AS_RESOURCE.iterator().next(), "test"));
+
+        // Validate removed docs
+        assertDocDoesNotExist(index, InferenceStats.docId("missing-model", "test"));
+        assertDocDoesNotExist(index, DataCounts.documentId("missing-analytics-with-stats"));
+    }
+
+    public void testRemovingUnusedStatsFromReadOnlyIndexShouldFailSilently() throws Exception {
+        String modelId = "model-with-stats";
+        putDFA(modelId);
+
         indexStatDocument(
-            new InferenceStats(1, 1, 1, 1, "model-with-stats", "test", Instant.now()),
-            InferenceStats.docId("model-with-stats", "test")
+            new InferenceStats(1, 1, 1, 1, "missing-model", "test", Instant.now()),
+            InferenceStats.docId("missing-model", "test")
         );
-        client().admin().indices().prepareRefresh(MlStatsIndex.indexPattern()).get();
+        makeIndexReadOnly();
+        refreshStatsIndex();
 
-        PlainActionFuture<Boolean> deletionListener = new PlainActionFuture<>();
-        UnusedStatsRemover statsRemover = new UnusedStatsRemover(client, new TaskId("test", 0L));
-        statsRemover.remove(10000.0f, deletionListener, () -> false);
-        deletionListener.actionGet();
+        runUnusedStatsRemover();
+        refreshStatsIndex();
 
-        client().admin().indices().prepareRefresh(MlStatsIndex.indexPattern()).get();
+        final String index = MlStatsIndex.TEMPLATE_NAME + "-000001";
+        assertDocExists(index, InferenceStats.docId("missing-model", "test")); // should still exist
+    }
 
-        final String initialStateIndex = MlStatsIndex.TEMPLATE_NAME + "-000001";
+    private void putDFA(String modelId) {
+        prepareIndex("foo").setId("some-empty-doc").setSource("{}", XContentType.JSON).get();
 
-        // Make sure that stats that should exist still exist
-        assertTrue(client().prepareGet(initialStateIndex, InferenceStats.docId("model-with-stats", "test")).get().isExists());
-        assertTrue(
-            client().prepareGet(
-                initialStateIndex,
-                InferenceStats.docId(TrainedModelProvider.MODELS_STORED_AS_RESOURCE.iterator().next(), "test")
-            ).get().isExists()
+        PutDataFrameAnalyticsAction.Request analyticsRequest = new PutDataFrameAnalyticsAction.Request(
+            new DataFrameAnalyticsConfig.Builder().setId("analytics-with-stats")
+                .setModelMemoryLimit(ByteSizeValue.ofGb(1))
+                .setSource(new DataFrameAnalyticsSource(new String[] { "foo" }, null, null, null))
+                .setDest(new DataFrameAnalyticsDest("bar", null))
+                .setAnalysis(new Regression("prediction"))
+                .build()
         );
-        assertTrue(client().prepareGet(initialStateIndex, DataCounts.documentId("analytics-with-stats")).get().isExists());
-
-        // make sure that unused stats were deleted
-        assertFalse(client().prepareGet(initialStateIndex, DataCounts.documentId("missing-analytics-with-stats")).get().isExists());
-        assertFalse(client().prepareGet(initialStateIndex, InferenceStats.docId("missing-model", "test")).get().isExists());
+        client.execute(PutDataFrameAnalyticsAction.INSTANCE, analyticsRequest).actionGet();
+
+        TrainedModelDefinition.Builder definition = new TrainedModelDefinition.Builder().setPreProcessors(Collections.emptyList())
+            .setTrainedModel(
+                Tree.builder().setFeatureNames(Arrays.asList("foo", "bar")).setRoot(TreeNode.builder(0).setLeafValue(42)).build()
+            );
+
+        TrainedModelConfig modelConfig = TrainedModelConfig.builder()
+            .setModelId(modelId)
+            .setInferenceConfig(RegressionConfig.EMPTY_PARAMS)
+            .setInput(new TrainedModelInput(Arrays.asList("foo", "bar")))
+            .setParsedDefinition(definition)
+            .validate(true)
+            .build();
+
+        client.execute(PutTrainedModelAction.INSTANCE, new PutTrainedModelAction.Request(modelConfig, false)).actionGet();
     }
 
     private void indexStatDocument(ToXContentObject object, String docId) throws Exception {
-        ToXContent.Params params = new ToXContent.MapParams(
-            Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, Boolean.toString(true))
-        );
-        IndexRequest doc = new IndexRequest(MlStatsIndex.writeAlias());
-        doc.id(docId);
+        IndexRequest doc = new IndexRequest(MlStatsIndex.writeAlias()).id(docId);
         try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
-            object.toXContent(builder, params);
+            object.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")));
             doc.source(builder);
             client.index(doc).actionGet();
         }
     }
+
+    private void refreshStatsIndex() {
+        client().admin().indices().prepareRefresh(MlStatsIndex.indexPattern()).get();
+    }
+
+    private void runUnusedStatsRemover() {
+        PlainActionFuture<Boolean> deletionListener = new PlainActionFuture<>();
+        new UnusedStatsRemover(client, new TaskId("test", 0L), writableIndexExpander).remove(10000.0f, deletionListener, () -> false);
+        deletionListener.actionGet();
+    }
+
+    private void makeIndexReadOnly() {
+        client().admin()
+            .indices()
+            .prepareUpdateSettings(MlStatsIndex.indexPattern())
+            .setSettings(Settings.builder().put("index.blocks.write", true))
+            .get();
+    }
+
+    private void assertDocExists(String index, String docId) {
+        assertTrue(client().prepareGet(index, docId).get().isExists());
+    }
+
+    private void assertDocDoesNotExist(String index, String docId) {
+        assertFalse(client().prepareGet(index, docId).get().isExists());
+    }
 }

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteExpiredDataAction.java

@@ -15,6 +15,7 @@
 import org.elasticsearch.action.support.ThreadedActionListener;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.client.internal.OriginSettingClient;
+import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.TimeValue;
@@ -40,6 +41,7 @@
 import org.elasticsearch.xpack.ml.job.retention.MlDataRemover;
 import org.elasticsearch.xpack.ml.job.retention.UnusedStateRemover;
 import org.elasticsearch.xpack.ml.job.retention.UnusedStatsRemover;
+import org.elasticsearch.xpack.ml.job.retention.WritableIndexExpander;
 import org.elasticsearch.xpack.ml.notifications.AnomalyDetectionAuditor;
 import org.elasticsearch.xpack.ml.utils.VolatileCursorIterator;
 import org.elasticsearch.xpack.ml.utils.persistence.WrappedBatchedJobsIterator;
@@ -62,16 +64,19 @@ public class TransportDeleteExpiredDataAction extends HandledTransportAction<
 
     private final ThreadPool threadPool;
     private final Executor executor;
+    private final IndexNameExpressionResolver indexNameExpressionResolver;
     private final OriginSettingClient client;
     private final ClusterService clusterService;
     private final Clock clock;
     private final JobConfigProvider jobConfigProvider;
     private final JobResultsProvider jobResultsProvider;
     private final AnomalyDetectionAuditor auditor;
+    private final WritableIndexExpander writableIndexExpander;
 
     @Inject
     public TransportDeleteExpiredDataAction(
         ThreadPool threadPool,
+        IndexNameExpressionResolver indexNameExpressionResolver,
         TransportService transportService,
         ActionFilters actionFilters,
         Client client,
@@ -83,6 +88,7 @@ public TransportDeleteExpiredDataAction(
         this(
             threadPool,
             threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME),
+            indexNameExpressionResolver,
             transportService,
             actionFilters,
             client,
@@ -97,6 +103,7 @@ public TransportDeleteExpiredDataAction(
     TransportDeleteExpiredDataAction(
         ThreadPool threadPool,
         Executor executor,
+        IndexNameExpressionResolver indexNameExpressionResolver,
         TransportService transportService,
         ActionFilters actionFilters,
         Client client,
@@ -109,12 +116,14 @@ public TransportDeleteExpiredDataAction(
         super(DeleteExpiredDataAction.NAME, transportService, actionFilters, DeleteExpiredDataAction.Request::new, executor);
         this.threadPool = threadPool;
         this.executor = executor;
+        this.indexNameExpressionResolver = indexNameExpressionResolver;
         this.client = new OriginSettingClient(client, ClientHelper.ML_ORIGIN);
         this.clusterService = clusterService;
         this.clock = clock;
         this.jobConfigProvider = jobConfigProvider;
         this.jobResultsProvider = jobResultsProvider;
         this.auditor = auditor;
+        this.writableIndexExpander = new WritableIndexExpander(clusterService, indexNameExpressionResolver);
     }
 
     @Override
@@ -240,30 +249,34 @@ private List<MlDataRemover> createDataRemovers(
         TaskId parentTaskId,
         AnomalyDetectionAuditor anomalyDetectionAuditor
     ) {
+
         return Arrays.asList(
             new ExpiredResultsRemover(
                 originClient,
                 new WrappedBatchedJobsIterator(new SearchAfterJobsIterator(originClient)),
                 parentTaskId,
+                writableIndexExpander,
                 anomalyDetectionAuditor,
                 threadPool
             ),
-            new ExpiredForecastsRemover(originClient, threadPool, parentTaskId),
+            new ExpiredForecastsRemover(originClient, threadPool, parentTaskId, writableIndexExpander),
             new ExpiredModelSnapshotsRemover(
                 originClient,
                 new WrappedBatchedJobsIterator(new SearchAfterJobsIterator(originClient)),
-                threadPool,
                 parentTaskId,
+                writableIndexExpander,
+                threadPool,
                 jobResultsProvider,
                 anomalyDetectionAuditor
             ),
-            new UnusedStateRemover(originClient, parentTaskId),
+            new UnusedStateRemover(originClient, parentTaskId, writableIndexExpander),
             new EmptyStateIndexRemover(originClient, parentTaskId),
-            new UnusedStatsRemover(originClient, parentTaskId),
+            new UnusedStatsRemover(originClient, parentTaskId, writableIndexExpander),
             new ExpiredAnnotationsRemover(
                 originClient,
                 new WrappedBatchedJobsIterator(new SearchAfterJobsIterator(originClient)),
                 parentTaskId,
+                writableIndexExpander,
                 anomalyDetectionAuditor,
                 threadPool
             )
@@ -272,20 +285,35 @@ private List<MlDataRemover> createDataRemovers(
 
     private List<MlDataRemover> createDataRemovers(List<Job> jobs, TaskId parentTaskId, AnomalyDetectionAuditor anomalyDetectionAuditor) {
         return Arrays.asList(
-            new ExpiredResultsRemover(client, new VolatileCursorIterator<>(jobs), parentTaskId, anomalyDetectionAuditor, threadPool),
-            new ExpiredForecastsRemover(client, threadPool, parentTaskId),
+            new ExpiredResultsRemover(
+                client,
+                new VolatileCursorIterator<>(jobs),
+                parentTaskId,
+                writableIndexExpander,
+                anomalyDetectionAuditor,
+                threadPool
+            ),
+            new ExpiredForecastsRemover(client, threadPool, parentTaskId, writableIndexExpander),
             new ExpiredModelSnapshotsRemover(
                 client,
                 new VolatileCursorIterator<>(jobs),
-                threadPool,
                 parentTaskId,
+                writableIndexExpander,
+                threadPool,
                 jobResultsProvider,
                 anomalyDetectionAuditor
             ),
-            new UnusedStateRemover(client, parentTaskId),
+            new UnusedStateRemover(client, parentTaskId, writableIndexExpander),
             new EmptyStateIndexRemover(client, parentTaskId),
-            new UnusedStatsRemover(client, parentTaskId),
-            new ExpiredAnnotationsRemover(client, new VolatileCursorIterator<>(jobs), parentTaskId, anomalyDetectionAuditor, threadPool)
+            new UnusedStatsRemover(client, parentTaskId, writableIndexExpander),
+            new ExpiredAnnotationsRemover(
+                client,
+                new VolatileCursorIterator<>(jobs),
+                parentTaskId,
+                writableIndexExpander,
+                anomalyDetectionAuditor,
+                threadPool
+            )
         );
     }
 

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/retention/AbstractExpiredJobDataRemover.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.xpack.core.ml.job.config.Job;
 
 import java.util.Iterator;
+import java.util.Objects;
 import java.util.function.BooleanSupplier;
 
 /**
@@ -26,11 +27,18 @@ abstract class AbstractExpiredJobDataRemover implements MlDataRemover {
     protected final OriginSettingClient client;
     private final Iterator<Job> jobIterator;
     private final TaskId parentTaskId;
+    protected final WritableIndexExpander writableIndexExpander;
 
-    AbstractExpiredJobDataRemover(OriginSettingClient client, Iterator<Job> jobIterator, TaskId parentTaskId) {
+    AbstractExpiredJobDataRemover(
+        OriginSettingClient client,
+        Iterator<Job> jobIterator,
+        TaskId parentTaskId,
+        WritableIndexExpander writableIndexExpander
+    ) {
         this.client = client;
         this.jobIterator = jobIterator;
         this.parentTaskId = parentTaskId;
+        this.writableIndexExpander = Objects.requireNonNull(writableIndexExpander);
     }
 
     protected TaskId getParentTaskId() {