[reindex] Don't attempt to refresh on noop

If the user asks for a refresh but their reindex or update-by-query operation touched no indexes we should just skip the resfresh call entirely. Without this commit we refresh *all* indexes which is totally wrong. Closes #17296
elastic · Mar 24, 2016 · 95de616 · 95de616
1 parent eb0714c
commit 95de616
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/...eindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java b/...eindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java
@@ -319,9 +319,12 @@ private void recordFailure(Failure failure, List<Failure> failures) {
         failures.add(failure);
     }
 
+    /**
+     * Start terminating a request that finished non-catastrophically.
+     */
     void startNormalTermination(final List<Failure> indexingFailures, final List<ShardSearchFailure> searchFailures,
             final boolean timedOut) {
-        if (task.isCancelled() || false == mainRequest.isRefresh()) {
+        if (task.isCancelled() || false == mainRequest.isRefresh() || destinationIndices.isEmpty()) {
             finishHim(null, indexingFailures, searchFailures, timedOut);
             return;
         }
@@ -432,4 +435,4 @@ public void remove() {
             }
         };
     }
-}
+}
diff --git a/...s/reindex/src/test/java/org/elasticsearch/index/reindex/AsyncBulkByScrollActionTests.java b/...s/reindex/src/test/java/org/elasticsearch/index/reindex/AsyncBulkByScrollActionTests.java
@@ -415,23 +415,29 @@ public void testDefaultRetryTimes() {
     }
 
     public void testRefreshIsFalseByDefault() throws Exception {
-        refreshTestCase(null, false);
+        refreshTestCase(null, true, false);
     }
 
-    public void testRefreshFalseDoesntMakeVisible() throws Exception {
-        refreshTestCase(false, false);
+    public void testRefreshFalseDoesntExecuteRefresh() throws Exception {
+        refreshTestCase(false, true, false);
     }
 
-    public void testRefreshTrueMakesVisible() throws Exception {
-        refreshTestCase(true, true);
+    public void testRefreshTrueExecutesRefresh() throws Exception {
+        refreshTestCase(true, true, true);
     }
 
-    private void refreshTestCase(Boolean refresh, boolean shouldRefresh) {
+    public void testRefreshTrueSkipsRefreshIfNoDestinationIndexes() throws Exception {
+        refreshTestCase(true, false, false);
+    }
+
+    private void refreshTestCase(Boolean refresh, boolean addDestinationIndexes, boolean shouldRefresh) {
         if (refresh != null) {
             mainRequest.setRefresh(refresh);
         }
         DummyAbstractAsyncBulkByScrollAction action = new DummyAbstractAsyncBulkByScrollAction();
-        action.addDestinationIndices(singleton("foo"));
+        if (addDestinationIndexes) {
+            action.addDestinationIndices(singleton("foo"));
+        }
         action.startNormalTermination(Collections.<Failure>emptyList(), Collections.<ShardSearchFailure>emptyList(), false);
         if (shouldRefresh) {
             assertArrayEquals(new String[] {"foo"}, client.lastRefreshRequest.get().indices());
@@ -657,4 +663,4 @@ > void doExecute(Action<Request, Response, RequestBuilder> action, Request reque
             super.doExecute(action, request, listener);
         }
     }
-}
+}