Reindex sort deprecation warning take 2 (#49855)

Moved the deprecation warning to ReindexValidator to ensure it runs early and works with resilient reindex. Also check that the warning is reported back for wait_for_completion=false. Follow-up to #49458
elastic · Dec 6, 2019 · 67529c9 · 67529c9
1 parent 671d400
commit 67529c9
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 10 deletions.
diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexValidator.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexValidator.java
@@ -19,6 +19,8 @@
 
 package org.elasticsearch.index.reindex;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.automaton.Automata;
 import org.apache.lucene.util.automaton.Automaton;
 import org.apache.lucene.util.automaton.CharacterRunAutomaton;
@@ -32,12 +34,18 @@
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.search.builder.SearchSourceBuilder;
 
 import java.util.List;
 
 class ReindexValidator {
+    private static final Logger logger = LogManager.getLogger(ReindexValidator.class);
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
+    static final String SORT_DEPRECATED_MESSAGE = "The sort option in reindex is deprecated. " +
+        "Instead consider using query filtering to find the desired subset of data.";
 
     private final CharacterRunAutomaton remoteWhitelist;
     private final ClusterService clusterService;
@@ -57,6 +65,10 @@ void initialValidation(ReindexRequest request) {
         ClusterState state = clusterService.state();
         validateAgainstAliases(request.getSearchRequest(), request.getDestination(), request.getRemoteInfo(), resolver, autoCreateIndex,
             state);
+        SearchSourceBuilder searchSource = request.getSearchRequest().source();
+        if (searchSource != null && searchSource.sorts() != null && searchSource.sorts().isEmpty() == false) {
+            deprecationLogger.deprecated(SORT_DEPRECATED_MESSAGE);
+        }
     }
 
     static void checkRemoteWhitelist(CharacterRunAutomaton whitelist, RemoteInfo remoteInfo) {

diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/Reindexer.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/Reindexer.java
@@ -40,7 +40,6 @@
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@@ -52,7 +51,6 @@
 import org.elasticsearch.index.reindex.remote.RemoteScrollableHitSource;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
-import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.IOException;
@@ -73,9 +71,6 @@
 public class Reindexer {
 
     private static final Logger logger = LogManager.getLogger(Reindexer.class);
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
-    static final String SORT_DEPRECATED_MESSAGE = "The sort option in reindex is deprecated. " +
-        "Instead consider using query filtering to find the desired subset of data.";
 
     private final ClusterService clusterService;
     private final Client client;
@@ -93,10 +88,6 @@ public class Reindexer {
     }
 
     public void initTask(BulkByScrollTask task, ReindexRequest request, ActionListener<Void> listener) {
-        SearchSourceBuilder searchSource = request.getSearchRequest().source();
-        if (searchSource != null && searchSource.sorts() != null && searchSource.sorts().isEmpty() == false) {
-            deprecationLogger.deprecated(SORT_DEPRECATED_MESSAGE);
-        }
         BulkByScrollParallelizationHelper.initTaskState(task, request, client, listener);
     }
 

diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexSingleNodeTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexSingleNodeTests.java
@@ -55,6 +55,6 @@ public void testDeprecatedSort() {
 
         assertHitCount(client().prepareSearch("dest").setSize(0).get(), subsetSize);
         assertHitCount(client().prepareSearch("dest").setQuery(new RangeQueryBuilder("foo").gte(0).lt(max-subsetSize)).get(), 0);
-        assertWarnings(Reindexer.SORT_DEPRECATED_MESSAGE);
+        assertWarnings(ReindexValidator.SORT_DEPRECATED_MESSAGE);
     }
 }
diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/30_search.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/30_search.yml
@@ -121,6 +121,41 @@
         q: order:1
   - match: { hits.total: 1 }
 
+---
+"Sorting deprecated wait_for_completion false":
+  - skip:
+      version: " - 7.5.99"
+      reason: "sort deprecated in 7.6"
+      features: "warnings"
+
+  - do:
+      index:
+        index:  test
+        id:     1
+        body:   { "order": 1 }
+  - do:
+      indices.refresh: {}
+
+  - do:
+      warnings:
+        - The sort option in reindex is deprecated. Instead consider using query
+          filtering to find the desired subset of data.
+      reindex:
+        refresh: true
+        wait_for_completion: false
+        body:
+          source:
+            index: test
+            sort: order
+          dest:
+            index: target
+  - set: {task: task}
+
+  - do:
+      tasks.get:
+        wait_for_completion: true
+        task_id: $task
+
 ---
 "max_docs in URL":
   - skip: