WorkQueue: Do not busy-reschedule canceled or done one-shot tasks

Change a4c7a7384ef made Task.run() reschedule the task whenever its
runningState is already set, so that a periodic task that preempts
its own previous run is not dropped. The reschedule delay is
nanosPeriod / 3, which is 0 for one-shot tasks, and the branch also
applies to tasks that will never run again:

* cancel() sets runningState to RUNNING for a CancelableRunnable
  that has not started, to let it clean up.
* A finished one-shot task has runningState DONE.

If a pool thread dequeues such a task just before it is canceled or
after it completed, run() reschedules it with no delay, the next run
reschedules it again, and so on. The task has already been removed
from the queue's task list, so show-queue does not show it, but the
pool thread spins at 100% CPU indefinitely. In production we observed
dozens of replication threads, whose PushOne tasks are canceled when
pruned, each burning a full core in this loop.

Only reschedule periodic tasks that are not canceled or done. A
canceled or completed one-shot task has nothing left to do.

Release-Notes: Fix WorkQueue threads spinning at full CPU after a task was canceled
Change-Id: I3cc878fedc730643739a58853153b7a49b7d5733

java/com/google/gerrit/server/git/WorkQueue.java

diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 888205d..ff3821a 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -1001,7 +1001,7 @@
             executor.remove(this);
           }
         }
-      } else {
+      } else if (isPeriodic() && !isDone()) {
         Future<?> unusedFuture = executor.schedule(this, nanosPeriod / 3, TimeUnit.NANOSECONDS);
       }
     }

javatests/com/google/gerrit/acceptance/server/util/WorkQueueIT.java

diff --git a/javatests/com/google/gerrit/acceptance/server/util/WorkQueueIT.java b/javatests/com/google/gerrit/acceptance/server/util/WorkQueueIT.java
index 92aca78..a560496 100644
--- a/javatests/com/google/gerrit/acceptance/server/util/WorkQueueIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/util/WorkQueueIT.java
@@ -30,6 +30,7 @@
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import org.junit.Test;
 
@@ -117,6 +118,29 @@
     testExecutor.shutdownNow();
   }
 
+  @Test
+  public void testCanceledOneShotTaskIsNotRescheduled() throws Exception {
+    ScheduledThreadPoolExecutor testExecutor =
+        workQueue.createQueue(POOL_CORE_SIZE, QUEUE_NAME, false);
+    WorkQueue.CancelableRunnable cancelable =
+        new WorkQueue.CancelableRunnable() {
+          @Override
+          public void run() {}
+
+          @Override
+          public void cancel() {}
+        };
+    Runnable task = (Runnable) testExecutor.schedule(cancelable, 1, TimeUnit.HOURS);
+    assertThat(((Future<?>) task).cancel(false)).isTrue();
+
+    // Simulate a pool thread that dequeued the task just before it got canceled.
+    task.run();
+
+    TimeUnit.MILLISECONDS.sleep(FIXED_RATE_SCHEDULE_INTERVAL_MILLI_SEC);
+    assertThat(testExecutor.getCompletedTaskCount()).isEqualTo(0);
+    testExecutor.shutdownNow();
+  }
+
   public void assertTasksInStateEventually(String queue, State expectedState, int expectedCount)
       throws Exception {
     assertEventually(