You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ct...@apache.org on 2022/03/16 01:03:08 UTC

[accumulo] branch main updated: Fix CleanUpIT failure (#2568)

This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new 68606c7  Fix CleanUpIT failure (#2568)
68606c7 is described below

commit 68606c780123bab7c8646a6742f573221571649b
Author: Christopher Tubbs <ct...@apache.org>
AuthorDate: Tue Mar 15 21:02:59 2022 -0400

    Fix CleanUpIT failure (#2568)
    
    * Throw correct runtime exception when accessing a closed scanner
      (should be an illegal state, not an illegal argument)
    * Close the scanner before closing the client in CleanUpIT because
      closing the scanner requires the client to not be closed in order to
      execute cleanup tasks without blocking
---
 .../accumulo/core/clientImpl/ScannerImpl.java      |  2 +-
 .../apache/accumulo/test/functional/CleanUpIT.java | 30 ++++++++++++----------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerImpl.java
index 338726a..b4d989f 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerImpl.java
@@ -104,7 +104,7 @@ public class ScannerImpl extends ScannerOptions implements Scanner {
 
   private synchronized void ensureOpen() {
     if (closed)
-      throw new IllegalArgumentException("Scanner is closed");
+      throw new IllegalStateException("Scanner is closed");
   }
 
   public ScannerImpl(ClientContext context, TableId tableId, Authorizations authorizations) {
diff --git a/test/src/main/java/org/apache/accumulo/test/functional/CleanUpIT.java b/test/src/main/java/org/apache/accumulo/test/functional/CleanUpIT.java
index 76753cd..1b4860f 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/CleanUpIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/CleanUpIT.java
@@ -112,24 +112,28 @@ public class CleanUpIT extends SharedMiniClusterBase {
         fail("Not seeing expected threads. Saw " + threadCount);
       }
 
-      org.apache.accumulo.core.util.CleanUp.shutdownNow(conn);
+      // explicitly close the scanner to verify that the scanner throws after close when iterated
+      scanner.close();
+      assertThrows(IllegalStateException.class, () -> Iterables.size(scanner));
+    }
 
-      Mutation m2 = new Mutation("r2");
-      m2.put("cf1", "cq1", 1, "6");
+    // close the scanners before closing the client, because the scanners need the client's cleanup
+    // thread pool to execute their cleanup tasks when they are closed, so they don't block
+    org.apache.accumulo.core.util.CleanUp.shutdownNow(conn);
 
-      bw.addMutation(m1);
-      assertThrows(MutationsRejectedException.class, bw::flush);
+    Mutation m2 = new Mutation("r2");
+    m2.put("cf1", "cq1", 1, "6");
 
-      // expect this to fail also, want to clean up batch writer threads
-      assertThrows(MutationsRejectedException.class, bw::close);
+    bw.addMutation(m1);
+    assertThrows(MutationsRejectedException.class, bw::flush);
 
-      assertThrows(IllegalStateException.class, () -> Iterables.size(scanner));
+    // expect this to fail also, want to clean up batch writer threads
+    assertThrows(MutationsRejectedException.class, bw::close);
 
-      threadCount = countThreads();
-      if (threadCount > 0) {
-        printThreadNames();
-        fail("Threads did not go away. Saw " + threadCount);
-      }
+    var threadCount = countThreads();
+    if (threadCount > 0) {
+      printThreadNames();
+      fail("Threads did not go away. Saw " + threadCount);
     }
   }