[GitHub] [cassandra] dcapwell commented on a diff in pull request #1687: CASSANDRA-15968 4.0

[GitBox <gi...@apache.org>]

dcapwell commented on code in PR #1687:
URL: https://github.com/apache/cassandra/pull/1687#discussion_r905600923


##########
src/java/org/apache/cassandra/gms/Gossiper.java:
##########
@@ -2015,8 +2010,14 @@ public void addLocalApplicationStates(List<Pair<ApplicationState, VersionedValue
 
     }
 
-    public void stop()
+    public synchronized void stop()
     {
+        if (isShuttingDown)
+        {
+            logger.info("Gossiper was already shutdown.");
+            return;
+        }
+        isShuttingDown = true;

Review Comment:
   `org.apache.cassandra.gms.Gossiper#start(int)` can be called via jmx and is a normal operational activity, same for `stop`.
   
   We need to revert this config when `start` is called else this will break users



##########
src/java/org/apache/cassandra/hints/HintsService.java:
##########
@@ -242,7 +242,10 @@ public void resumeDispatch()
     public synchronized void shutdownBlocking() throws ExecutionException, InterruptedException
     {
         if (isShutDown)
-            throw new IllegalStateException("HintsService has already been shut down");
+        {
+            logger.info("Hint service was already shut down.");

Review Comment:
   I think I handle this in Instance on trunk, but good to just not fail



##########
src/java/org/apache/cassandra/net/MessagingService.java:
##########
@@ -529,6 +529,12 @@ public void listen()
         inboundSockets.open();
     }
 
+    public void unsafeResetListen()
+    {
+        inboundSockets.open();
+        isShuttingDown = false;

Review Comment:
   why not just move this to `listen`?  supporting a new method that is similar can be brittle



##########
src/java/org/apache/cassandra/net/MessagingService.java:
##########
@@ -529,6 +529,12 @@ public void listen()
         inboundSockets.open();
     }
 
+    public void unsafeResetListen()
+    {
+        inboundSockets.open();
+        isShuttingDown = false;
+    }
+
     public void waitUntilListening() throws InterruptedException

Review Comment:
   shouldn't we set false here as well?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org