You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "Mmuzaf (via GitHub)" <gi...@apache.org> on 2023/08/19 17:31:10 UTC

[GitHub] [cassandra] Mmuzaf opened a new pull request, #2613: CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException

Mmuzaf opened a new pull request, #2613:
URL: https://github.com/apache/cassandra/pull/2613

   Thanks for sending a pull request! Here are some tips if you're new here:
    
    * Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
    * Be sure to keep the PR description updated to reflect all changes.
    * Write your PR title to summarize what this PR proposes.
    * If possible, provide a concise example to reproduce the issue for a faster review.
    * Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
    * If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
    
   Commit messages should follow the following format:
   
   ```
   <One sentence description, usually Jira title or CHANGES.txt summary>
   
   <Optional lengthier description (context on patch)>
   
   patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
   
   Co-authored-by: Name1 <email1>
   Co-authored-by: Name2 <email2>
   
   ```
   
   The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
   
   


-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1377561000


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +491,42 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                int added = 0;
+                while (deadline > nanoTime() && added == 0)
                 {
                     reporters.update();
                     reporters.print();
-                    Uninterruptibles.awaitUninterruptibly(failed, 30L, TimeUnit.SECONDS);
+                    added = Queues.drainUninterruptibly(unexpectedExceptions, exceptions, 10, 30L, TimeUnit.SECONDS);
                 }
 
                 executor.shutdownNow();
                 ExecutorUtils.awaitTermination(5L, TimeUnit.MINUTES, executor);
+
+                if (!exceptions.isEmpty())
+                    throw exceptions.get(0);

Review Comment:
   Fixed.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1474487505


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -256,19 +271,19 @@ List<Connection> getConnections(InetAddressAndPort endpoint, boolean inbound)
         /**
          * Test connections with broken messages, live in-flight bytes updates, reconnect
          */
-        public void run() throws ExecutionException, InterruptedException, NoSuchFieldException, IllegalAccessException, TimeoutException
+        public void run() throws Exception
         {
             Reporters reporters = new Reporters(endpoints, connections);
+            List<Exception> exceptions = new LinkedList<>();

Review Comment:
   I meant that `LinkedList` is almost always inferior to `ArrayList`, and there appears to be no reason to prefer it here. It's not an important point, particularly here the distinction is unimportant.
   
   Regarding declaring it more proximal to usage, I meant I don't see why it is declared at the start of the method rather than above the loop that uses it. It's helpful IMO to declare variables close to where they are logically introduced in the code.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1377501662


##########
test/burn/org/apache/cassandra/net/Verifier.java:
##########
@@ -1326,9 +1339,11 @@ Chunk ensureChunk(long chunkSequenceId) throws InterruptedException
                 Map.Entry<Long, Chunk> e;
                 while ( null != (e = chunkList.firstEntry()) && chunkSequenceId - e.getKey() > 1 << 12)
                 {
+                    if (cancelled)
+                        throw new InterruptedException();
                     WaitQueue.Signal signal = writerWaiting.register();
                     if (null != (e = chunkList.firstEntry()) && chunkSequenceId - e.getKey() > 1 << 12)
-                        signal.await();
+                        signal.await(5L, TimeUnit.SECONDS);

Review Comment:
   This is for safety's sake and in accordance with the following comment which gave me the feeling that we'd get the next chunk within a reasonable timeframe (5 seconds), and if not - we could fail the test, but of course I could be missing something.
   
   https://github.com/apache/cassandra/blob/trunk/test/burn/org/apache/cassandra/net/Verifier.java#L668
   ```
   // TODO: even 2s or 5s are unreasonable periods of time without _any_ movement on a message waiting to arrive
   //       this seems to happen regularly on MacOS, but we should confirm this does not happen on Linux
   ```



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1377569583


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +491,42 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                int added = 0;
+                while (deadline > nanoTime() && added == 0)
                 {
                     reporters.update();
                     reporters.print();
-                    Uninterruptibles.awaitUninterruptibly(failed, 30L, TimeUnit.SECONDS);
+                    added = Queues.drainUninterruptibly(unexpectedExceptions, exceptions, 10, 30L, TimeUnit.SECONDS);

Review Comment:
   Fixed, and move this value to a constant value for visibility. 



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#issuecomment-1810765050

   @belliottsmith Hi Benedict, have you had a chance to look at the comments that have been fixed? :-)


-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1374713107


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +491,42 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                int added = 0;

Review Comment:
   at most we need a boolean, but I'd say this should be a part of the loop guard; perhaps use a do/while loop and simply check 0 == Queues.drainUninterruptibly()



##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +491,42 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                int added = 0;
+                while (deadline > nanoTime() && added == 0)
                 {
                     reporters.update();
                     reporters.print();
-                    Uninterruptibles.awaitUninterruptibly(failed, 30L, TimeUnit.SECONDS);
+                    added = Queues.drainUninterruptibly(unexpectedExceptions, exceptions, 10, 30L, TimeUnit.SECONDS);

Review Comment:
   why 10 items, rather than 1?



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1474486347


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +493,43 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                while (deadline > nanoTime() &&
+                        0 == Queues.drainUninterruptibly(unexpectedExceptions,

Review Comment:
   I'm not sure this is the desired behaviour? It looks like the semantics of `drainUninterruptibly` is that it will wait for `exceptionBufferCapacity` before returning, whereas we want to return as soon as non-empty. This isn't super important here due to the timeout, but it is good to get this kind of thing right.
   
   We should either wait for just 1 item, or use a different method for waiting, IMO.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1474491742


##########
test/burn/org/apache/cassandra/net/Verifier.java:
##########
@@ -1326,9 +1339,11 @@ Chunk ensureChunk(long chunkSequenceId) throws InterruptedException
                 Map.Entry<Long, Chunk> e;
                 while ( null != (e = chunkList.firstEntry()) && chunkSequenceId - e.getKey() > 1 << 12)
                 {
+                    if (cancelled)
+                        throw new InterruptedException();
                     WaitQueue.Signal signal = writerWaiting.register();
                     if (null != (e = chunkList.firstEntry()) && chunkSequenceId - e.getKey() > 1 << 12)
-                        signal.await();
+                        signal.await(5L, TimeUnit.SECONDS);

Review Comment:
   Sure, ok. Though given the earlier comment might be worth making this timeout a bit bigger to avoid false positives, or perhaps condition the wait time on the system under test.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1474486347


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +493,43 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                while (deadline > nanoTime() &&
+                        0 == Queues.drainUninterruptibly(unexpectedExceptions,

Review Comment:
   I'm not sure this is the desired behaviour? It looks like the semantics of `drainUninterruptibly` is that it will wait for `exceptionBufferCapacity` before returning, whereas we want to return as soon as non-empty. This isn't super important here due to the timeout, but it is good to get this kind of thing right.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1377569081


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -256,19 +271,19 @@ List<Connection> getConnections(InetAddressAndPort endpoint, boolean inbound)
         /**
          * Test connections with broken messages, live in-flight bytes updates, reconnect
          */
-        public void run() throws ExecutionException, InterruptedException, NoSuchFieldException, IllegalAccessException, TimeoutException
+        public void run() throws Exception
         {
             Reporters reporters = new Reporters(endpoints, connections);
+            List<Exception> exceptions = new LinkedList<>();

Review Comment:
   This is just a local buffer to fetch results from the `unexpectedExceptions` queue, so since it is only used locally it should be safe to use non-thread-safe collection, or did you mean something else? 
   
   I think we can tie `drainUninterruptibly` and this exceptions buffer, so that they use the same buffer size via constant. I set it to `1` since we are only interested in the first exception that occurred, but we could increase it if you'd like. 



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1377561442


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +491,42 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                int added = 0;

Review Comment:
   Good suggestion, fixed.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1474487505


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -256,19 +271,19 @@ List<Connection> getConnections(InetAddressAndPort endpoint, boolean inbound)
         /**
          * Test connections with broken messages, live in-flight bytes updates, reconnect
          */
-        public void run() throws ExecutionException, InterruptedException, NoSuchFieldException, IllegalAccessException, TimeoutException
+        public void run() throws Exception
         {
             Reporters reporters = new Reporters(endpoints, connections);
+            List<Exception> exceptions = new LinkedList<>();

Review Comment:
   I meant that `LinkedList` is almost always inferior to `ArrayList`, and there appears to be no reason to prefer it here. It's not an important point, particularly here the distinction is unimportant. Thanks for making the change, though I'm not sure why `Lists.newArrayList` is preferred here to simply `new ArrayList<>()`.
   
   Regarding declaring it more proximal to usage, I meant I don't see why it is declared at the start of the method rather than above the loop that uses it. It's helpful IMO to declare variables close to where they are logically introduced in the code.



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1374717700


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -467,26 +491,42 @@ public void run() throws ExecutionException, InterruptedException, NoSuchFieldEx
                     }
                 });
 
-                while (deadline > nanoTime() && failed.getCount() > 0)
+                int added = 0;
+                while (deadline > nanoTime() && added == 0)
                 {
                     reporters.update();
                     reporters.print();
-                    Uninterruptibles.awaitUninterruptibly(failed, 30L, TimeUnit.SECONDS);
+                    added = Queues.drainUninterruptibly(unexpectedExceptions, exceptions, 10, 30L, TimeUnit.SECONDS);
                 }
 
                 executor.shutdownNow();
                 ExecutorUtils.awaitTermination(5L, TimeUnit.MINUTES, executor);
+
+                if (!exceptions.isEmpty())
+                    throw exceptions.get(0);

Review Comment:
   should probably addSuppressed any items > index 0



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1374718362


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -256,19 +271,19 @@ List<Connection> getConnections(InetAddressAndPort endpoint, boolean inbound)
         /**
          * Test connections with broken messages, live in-flight bytes updates, reconnect
          */
-        public void run() throws ExecutionException, InterruptedException, NoSuchFieldException, IllegalAccessException, TimeoutException
+        public void run() throws Exception
         {
             Reporters reporters = new Reporters(endpoints, connections);
+            List<Exception> exceptions = new LinkedList<>();

Review Comment:
   also, consider declaring more proximal to usage



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#issuecomment-1787202147

   @belliottsmith I've addressed your comments, please, take a look again :-)


-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1374718881


##########
test/burn/org/apache/cassandra/net/Verifier.java:
##########
@@ -1326,9 +1339,11 @@ Chunk ensureChunk(long chunkSequenceId) throws InterruptedException
                 Map.Entry<Long, Chunk> e;
                 while ( null != (e = chunkList.firstEntry()) && chunkSequenceId - e.getKey() > 1 << 12)
                 {
+                    if (cancelled)
+                        throw new InterruptedException();
                     WaitQueue.Signal signal = writerWaiting.register();
                     if (null != (e = chunkList.firstEntry()) && chunkSequenceId - e.getKey() > 1 << 12)
-                        signal.await();
+                        signal.await(5L, TimeUnit.SECONDS);

Review Comment:
   why is this now timed?



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#discussion_r1374698768


##########
test/burn/org/apache/cassandra/net/ConnectionBurnTest.java:
##########
@@ -256,19 +271,19 @@ List<Connection> getConnections(InetAddressAndPort endpoint, boolean inbound)
         /**
          * Test connections with broken messages, live in-flight bytes updates, reconnect
          */
-        public void run() throws ExecutionException, InterruptedException, NoSuchFieldException, IllegalAccessException, TimeoutException
+        public void run() throws Exception
         {
             Reporters reporters = new Reporters(endpoints, connections);
+            List<Exception> exceptions = new LinkedList<>();

Review Comment:
   why a linked list?



-- 
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


Re: [PR] CASSNDRA-18780 Fixed ConnectionBurnTest fails with NullPointerException [cassandra]

Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on PR #2613:
URL: https://github.com/apache/cassandra/pull/2613#issuecomment-1863129243

   @belliottsmith Hi Benedict, I think we are really close to fixing it! :-) Would you mind taking one last look?


-- 
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