Merge remote-tracking branch 'elastic/main' into batched-exec-short

Author: original-brownbear

docs/changelog/121119.yaml

@@ -0,0 +1,5 @@
+pr: 121327
+summary: Reduce Data Loss in System Indices Migration
+area: Infra/Core
+type: bug
+issues: []

docs/changelog/121327.yaml

@@ -9,14 +9,17 @@
 
 package org.elasticsearch.migration;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.admin.cluster.migration.TransportGetFeatureUpgradeStatusAction;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
 import org.elasticsearch.action.admin.indices.stats.IndexStats;
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
 import org.elasticsearch.action.index.IndexRequestBuilder;
+import org.elasticsearch.action.support.ActionFilter;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterState;
@@ -28,6 +31,7 @@
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.indices.AssociatedIndexDescriptor;
 import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.plugins.SystemIndexPlugin;
@@ -50,6 +54,10 @@
 import java.util.function.BiConsumer;
 import java.util.function.Function;
 
+import static java.util.Collections.emptySet;
+import static java.util.Collections.singletonList;
+import static java.util.Collections.unmodifiableSet;
+import static org.elasticsearch.common.util.set.Sets.newHashSet;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.equalTo;
@@ -255,12 +263,18 @@ protected void assertIndexHasCorrectProperties(
         assertThat(thisIndexStats.getTotal().getDocs().getCount(), is((long) INDEX_DOC_COUNT));
     }
 
-    public static class TestPlugin extends Plugin implements SystemIndexPlugin {
+    public static class TestPlugin extends Plugin implements SystemIndexPlugin, ActionPlugin {
         public final AtomicReference<Function<ClusterState, Map<String, Object>>> preMigrationHook = new AtomicReference<>();
         public final AtomicReference<BiConsumer<ClusterState, Map<String, Object>>> postMigrationHook = new AtomicReference<>();
+        private final BlockingActionFilter blockingActionFilter;
 
         public TestPlugin() {
+            blockingActionFilter = new BlockingActionFilter();
+        }
 
+        @Override
+        public List<ActionFilter> getActionFilters() {
+            return singletonList(blockingActionFilter);
         }
 
         @Override
@@ -299,5 +313,26 @@ public void indicesMigrationComplete(
             postMigrationHook.get().accept(clusterService.state(), preUpgradeMetadata);
             listener.onResponse(true);
         }
+
+        public static class BlockingActionFilter extends org.elasticsearch.action.support.ActionFilter.Simple {
+            private Set<String> blockedActions = emptySet();
+
+            @Override
+            protected boolean apply(String action, ActionRequest request, ActionListener<?> listener) {
+                if (blockedActions.contains(action)) {
+                    throw new ElasticsearchException("force exception on [" + action + "]");
+                }
+                return true;
+            }
+
+            @Override
+            public int order() {
+                return 0;
+            }
+
+            public void blockActions(String... actions) {
+                blockedActions = unmodifiableSet(newHashSet(actions));
+            }
+        }
     }
 }

modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java

@@ -17,11 +17,14 @@
 import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeRequest;
 import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeResponse;
 import org.elasticsearch.action.admin.indices.alias.Alias;
+import org.elasticsearch.action.admin.indices.alias.TransportIndicesAliasesAction;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
 import org.elasticsearch.action.admin.indices.template.put.PutComponentTemplateAction;
 import org.elasticsearch.action.admin.indices.template.put.TransportPutComposableIndexTemplateAction;
 import org.elasticsearch.action.search.SearchRequestBuilder;
+import org.elasticsearch.action.support.ActionFilter;
+import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ClusterStateUpdateTask;
@@ -36,10 +39,12 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.migration.AbstractFeatureMigrationIntegTest.TestPlugin.BlockingActionFilter;
 import org.elasticsearch.painless.PainlessPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.SystemIndexPlugin;
 import org.elasticsearch.reindex.ReindexPlugin;
+import org.elasticsearch.test.InternalTestCluster;
 import org.elasticsearch.upgrades.FeatureMigrationResults;
 import org.elasticsearch.upgrades.SingleFeatureMigrationResult;
 
@@ -272,6 +277,60 @@ public void testMigrateIndexWithWriteBlock() throws Exception {
         });
     }
 
+    @AwaitsFix(bugUrl = "ES-10666") // This test uncovered an existing issue
+    public void testIndexBlockIsRemovedWhenAliasRequestFails() throws Exception {
+        createSystemIndexForDescriptor(INTERNAL_UNMANAGED);
+        ensureGreen();
+
+        // Block the alias request to simulate a failure
+        InternalTestCluster internalTestCluster = internalCluster();
+        ActionFilters actionFilters = internalTestCluster.getInstance(ActionFilters.class, internalTestCluster.getMasterName());
+        BlockingActionFilter blockingActionFilter = null;
+        for (ActionFilter filter : actionFilters.filters()) {
+            if (filter instanceof BlockingActionFilter) {
+                blockingActionFilter = (BlockingActionFilter) filter;
+                break;
+            }
+        }
+        assertNotNull("BlockingActionFilter should exist", blockingActionFilter);
+        blockingActionFilter.blockActions(TransportIndicesAliasesAction.NAME);
+
+        // Start the migration
+        client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest(TEST_REQUEST_TIMEOUT)).get();
+
+        // Wait till the migration fails
+        assertBusy(() -> {
+            GetFeatureUpgradeStatusResponse statusResp = client().execute(
+                GetFeatureUpgradeStatusAction.INSTANCE,
+                new GetFeatureUpgradeStatusRequest(TEST_REQUEST_TIMEOUT)
+            ).get();
+            logger.info(Strings.toString(statusResp));
+            assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.ERROR));
+        });
+
+        // Get the settings to see if the write block was removed
+        var allsettings = client().admin().indices().prepareGetSettings(INTERNAL_UNMANAGED.getIndexPattern()).get().getIndexToSettings();
+        var internalUnmanagedOldIndexSettings = allsettings.get(".int-unman-old");
+        var writeBlock = internalUnmanagedOldIndexSettings.get(IndexMetadata.INDEX_BLOCKS_WRITE_SETTING.getKey());
+        assertThat("Write block on old index should be removed on migration ERROR status", writeBlock, equalTo("false"));
+
+        // Unblock the alias request
+        blockingActionFilter.blockActions();
+
+        // Retry the migration
+        client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest(TEST_REQUEST_TIMEOUT)).get();
+
+        // Ensure that the migration is successful after the alias request is unblocked
+        assertBusy(() -> {
+            GetFeatureUpgradeStatusResponse statusResp = client().execute(
+                GetFeatureUpgradeStatusAction.INSTANCE,
+                new GetFeatureUpgradeStatusRequest(TEST_REQUEST_TIMEOUT)
+            ).get();
+            logger.info(Strings.toString(statusResp));
+            assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
+        });
+    }
+
     public void testMigrationWillRunAfterError() throws Exception {
         createSystemIndexForDescriptor(INTERNAL_MANAGED);
 

modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/FeatureMigrationIT.java

@@ -104,9 +104,6 @@ tests:
 - class: org.elasticsearch.discovery.ClusterDisruptionIT
   method: testAckedIndexing
   issue: https://github.com/elastic/elasticsearch/issues/117024
-- class: org.elasticsearch.xpack.test.rest.XPackRestIT
-  method: test {p0=snapshot/10_basic/Create a source only snapshot and then restore it}
-  issue: https://github.com/elastic/elasticsearch/issues/117295
 - class: org.elasticsearch.xpack.inference.InferenceRestIT
   method: test {p0=inference/40_semantic_text_query/Query a field that uses the default ELSER 2 endpoint}
   issue: https://github.com/elastic/elasticsearch/issues/117027
@@ -364,18 +361,13 @@ tests:
 - class: org.elasticsearch.analysis.common.CommonAnalysisClientYamlTestSuiteIT
   method: test {yaml=analysis-common/40_token_filters/stemmer_override file access}
   issue: https://github.com/elastic/elasticsearch/issues/121625
-- class: org.elasticsearch.xpack.esql.plugin.DataNodeRequestSenderTests
-  method: testDoNotRetryOnRequestLevelFailure
-  issue: https://github.com/elastic/elasticsearch/issues/121966
 - class: org.elasticsearch.xpack.searchablesnapshots.hdfs.SecureHdfsSearchableSnapshotsIT
   issue: https://github.com/elastic/elasticsearch/issues/121967
 - class: org.elasticsearch.action.search.SearchQueryThenFetchAsyncActionTests
   method: testBottomFieldSort
   issue: https://github.com/elastic/elasticsearch/issues/121503
 - class: org.elasticsearch.xpack.application.CohereServiceUpgradeIT
   issue: https://github.com/elastic/elasticsearch/issues/121537
-- class: org.elasticsearch.xpack.migrate.action.ReindexDatastreamIndexTransportActionIT
-  issue: https://github.com/elastic/elasticsearch/issues/121737
 - class: org.elasticsearch.xpack.restart.FullClusterRestartIT
   method: testWatcherWithApiKey {cluster=UPGRADED}
   issue: https://github.com/elastic/elasticsearch/issues/122061
@@ -391,16 +383,8 @@ tests:
 - class: org.elasticsearch.xpack.ml.integration.ClassificationIT
   method: testWithOnlyTrainingRowsAndTrainingPercentIsFifty_DependentVariableIsBoolean
   issue: https://github.com/elastic/elasticsearch/issues/121680
-- class: org.elasticsearch.xpack.esql.action.EsqlActionBreakerIT
-  issue: https://github.com/elastic/elasticsearch/issues/122153
 - class: org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT
   issue: https://github.com/elastic/elasticsearch/issues/121411
-- class: org.elasticsearch.xpack.esql.action.EsqlNodeFailureIT
-  method: testFailureLoadingFields
-  issue: https://github.com/elastic/elasticsearch/issues/122132
-- class: org.elasticsearch.xpack.esql.action.CrossClustersCancellationIT
-  method: testCloseSkipUnavailable
-  issue: https://github.com/elastic/elasticsearch/issues/121627
 - class: org.elasticsearch.xpack.downsample.DownsampleActionSingleNodeTests
   method: testDuplicateDownsampleRequest
   issue: https://github.com/elastic/elasticsearch/issues/122158
@@ -416,6 +400,17 @@ tests:
 - class: org.elasticsearch.xpack.esql.analysis.VerifierTests
   method: testChangePoint_valueNumeric
   issue: https://github.com/elastic/elasticsearch/issues/122181
+- class: org.elasticsearch.datastreams.TSDBPassthroughIndexingIT
+  issue: https://github.com/elastic/elasticsearch/issues/121716
+- class: org.elasticsearch.smoketest.SmokeTestMonitoringWithSecurityIT
+  method: testHTTPExporterWithSSL
+  issue: https://github.com/elastic/elasticsearch/issues/122220
+- class: org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT
+  method: testStopQueryLocal
+  issue: https://github.com/elastic/elasticsearch/issues/121672
+- class: org.elasticsearch.xpack.security.authz.IndexAliasesTests
+  method: testRemoveIndex
+  issue: https://github.com/elastic/elasticsearch/issues/122221
 
 # Examples:
 #

muted-tests.yml

@@ -240,13 +240,14 @@ public float score() {
                                             for (int doc = min; doc < max; ++doc) {
                                                 if (acceptDocs == null || acceptDocs.get(doc)) {
                                                     collector.collect(doc);
-                                                    // collect one doc, then throw a timeout, this ensures partial results will be returned
+                                                    // collect one doc per segment, only then throw a timeout: this ensures partial
+                                                    // results are returned
                                                     ((ContextIndexSearcher) searcher).throwTimeExceededException();
                                                 }
                                             }
-                                            throw new AssertionError(
-                                                "Should have thrown a time exceeded exception: [min: " + min + " - max: " + max + "]"
-                                            );
+                                            // there is a slight chance that no docs are scored for a specific segment.
+                                            // other shards / slices will throw the timeout anyway, one is enough.
+                                            return max == maxDoc ? DocIdSetIterator.NO_MORE_DOCS : max;
                                         }
 
                                         @Override

server/src/internalClusterTest/java/org/elasticsearch/search/SearchTimeoutIT.java

@@ -77,6 +77,17 @@ public boolean hasErrors() {
         return errors;
     }
 
+    /**
+     * Get a list of all errors from the response. If there are no errors, an empty list is returned.
+     */
+    public List<ElasticsearchException> getErrors() {
+        if (errors == false) {
+            return List.of();
+        } else {
+            return actionResults.stream().filter(a -> a.getError() != null).map(AliasActionResult::getError).toList();
+        }
+    }
+
     /**
      *  Build a response from a list of action results. Sets the errors boolean based
      *  on whether an of the individual results contain an error.
@@ -165,6 +176,13 @@ public static AliasActionResult buildSuccess(List<String> indices, AliasActions
             return new AliasActionResult(indices, action, null);
         }
 
+        /**
+         * The error result if the action failed, null if the action succeeded.
+         */
+        public ElasticsearchException getError() {
+            return error;
+        }
+
         private int getStatus() {
             return error == null ? 200 : error.status().getStatus();
         }