From f121e459f54414903ab2a914e8e981adcec593eb Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@oath.com>
Date: Tue, 21 Nov 2017 13:54:45 +0100
Subject: Do not remove from queue when cancelling

We have already drained the queue before cancelling a delayed response,
so it is a bug to try to remove anything from it
---
 .../vespa/config/server/rpc/DelayedConfigResponses.java | 17 +++++++++++------
 .../config/server/rpc/DelayedConfigResponseTest.java    |  4 +---
 2 files changed, 12 insertions(+), 9 deletions(-)

(limited to 'configserver/src')

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java
index ffd86ac121e..8aaf053247e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java
@@ -76,7 +76,8 @@ public class DelayedConfigResponses {
         }
 
         public synchronized void run() {
-            remove();
+            removeFromQueue();
+            removeWatcher();
             rpcServer.addToRequestQueue(request, true, null);
             if (log.isLoggable(LogLevel.DEBUG)) {
                 log.log(LogLevel.DEBUG, logPre()+"DelayedConfigResponse. putting on queue: " + request.getShortDescription());
@@ -86,9 +87,8 @@ public class DelayedConfigResponses {
         /**
          * Remove delayed response from its queue
          */
-        private void remove() {
+        private void removeFromQueue() {
             delayedResponsesQueue.remove(this);
-            removeWatcher();
         }
 
         public JRTServerConfigRequest getRequest() {
@@ -107,8 +107,13 @@ public class DelayedConfigResponses {
             return Tenants.logPre(app);
         }
 
-        public synchronized boolean cancel() {
-            remove();
+        synchronized void cancelAndRemove() {
+            removeFromQueue();
+            cancel();
+        }
+
+        synchronized boolean cancel() {
+            removeWatcher();
             if (future == null) {
                 throw new IllegalStateException("Cannot cancel a task that has not been scheduled");
             }
@@ -129,7 +134,7 @@ public class DelayedConfigResponses {
          */
         @Override
         public void notifyTargetInvalid(Target target) {
-            cancel();
+            cancelAndRemove();
         }
 
         private void addWatcher() {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java
index 122deadb841..0126a9e2f29 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java
@@ -12,8 +12,6 @@ import com.yahoo.vespa.config.protocol.JRTServerConfigRequest;
 import com.yahoo.vespa.config.protocol.JRTServerConfigRequestV3;
 import com.yahoo.vespa.config.protocol.Trace;
 import com.yahoo.vespa.config.server.GetConfigContext;
-import com.yahoo.vespa.config.server.rpc.DelayedConfigResponses;
-import com.yahoo.vespa.config.server.rpc.MockRpc;
 import org.junit.Test;
 
 import java.util.Collections;
@@ -58,7 +56,7 @@ public class DelayedConfigResponseTest {
         DelayedConfigResponses responses = new DelayedConfigResponses(rpc, 1, false);
         responses.delayResponse(createRequest("foolio", "md5", "myid", "mymd5", 3, 100000, "bar"), context);
         assertThat(responses.size(), is(1));
-        responses.allDelayedResponses().get(0).cancel();
+        responses.allDelayedResponses().get(0).cancelAndRemove();
         assertThat(responses.size(), is(0));
     }
 
-- 
cgit v1.2.3