From d92c94008fbfc02d5a5a8136ffebdcaa19a6d199 Mon Sep 17 00:00:00 2001
From: Zachary Tong <polyfractal@elastic.co>
Date: Fri, 15 Feb 2019 19:28:20 -0500
Subject: [PATCH] Rollup jobs should be cleaned up before indices are deleted
 (#38930)

Rollup jobs should be stopped + deleted before the indices are removed.
It's possible for an active rollup job to issue a bulk request, the test
ends and the cleanup code deletes all indices.  The in-flight bulk
request will then stall + error because the index no-longer exists...
but this process might take longer than the StopRollup timeout.

Which means the test fails, and often fails several other tests since
the job is still active (e.g. other tests cannot create the same-named
job, or fail to stop the job in their cleanup because it's still stalled).

This tends to knock over several tests before the bulk finally times
out and the job shuts down.

Instead, we need to simply stop jobs first.  Inflight bulks will resolve
quickly, and we can carry on with deleting indices after the jobs are
confirmed inactive.

stop-job.asciidoc tended to trigger this issue because it executed
an async stop API and then exited, which setup the above situation. In
can and did happen with other tests though.  As an extra precaution,
the doc test was modified to substitute in wait_for_completion
to help head off these issues too.
---
 docs/reference/rollup/apis/stop-job.asciidoc       |  1 +
 .../elasticsearch/test/rest/ESRestTestCase.java    | 14 +++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/docs/reference/rollup/apis/stop-job.asciidoc b/docs/reference/rollup/apis/stop-job.asciidoc
index 50935826f5f53..35162246a5fbb 100644
--- a/docs/reference/rollup/apis/stop-job.asciidoc
+++ b/docs/reference/rollup/apis/stop-job.asciidoc
@@ -56,6 +56,7 @@ POST _rollup/job/sensor/_stop
 --------------------------------------------------
 // CONSOLE
 // TEST[setup:sensor_started_rollup_job]
+// TEST[s/_stop/_stop?wait_for_completion=true&timeout=10s/]
 
 Which will return the response:
 
diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java
index 00900271a2569..35902f80f8469 100644
--- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java
@@ -458,6 +458,15 @@ protected boolean preserveILMPoliciesUponCompletion() {
     }
 
     private void wipeCluster() throws Exception {
+
+        // Cleanup rollup before deleting indices.  A rollup job might have bulks in-flight,
+        // so we need to fully shut them down first otherwise a job might stall waiting
+        // for a bulk to finish against a non-existing index (and then fail tests)
+        if (hasXPack && false == preserveRollupJobsUponCompletion()) {
+            wipeRollupJobs();
+            waitForPendingRollupTasks();
+        }
+
         if (preserveIndicesUponCompletion() == false) {
             // wipe indices
             try {
@@ -505,11 +514,6 @@ private void wipeCluster() throws Exception {
             wipeClusterSettings();
         }
 
-        if (hasXPack && false == preserveRollupJobsUponCompletion()) {
-            wipeRollupJobs();
-            waitForPendingRollupTasks();
-        }
-
         if (hasXPack && false == preserveILMPoliciesUponCompletion()) {
             deleteAllPolicies();
         }