You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/03/01 16:54:01 UTC

[GitHub] [accumulo] DomGarguilo opened a new pull request #1952: Convert ZooLockITs use of MAC to ZooKeeperTestingServer. Fixes #1885

DomGarguilo opened a new pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952


   Fixes #1885 
   
   This change required ZooLockIT to be renamed to ZooLockTest and moved to a different directory since it no longer uses MAC. Not sure that I moved the to the correct directory however.


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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586496272



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -75,66 +70,65 @@
    */
   @Test
   public void concurrentMutatorTest() throws Exception {
-    try (var client = Accumulo.newClient().from(getClientProps()).build();
-        var context = (ClientContext) client) {
-      String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET);
 
-      ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(),
-          context.getZooKeepersSessionTimeOut(), secret);
+    ZooKeeperTestingServer szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/" + UUID.randomUUID().toString());
+    ZooReaderWriter zk = new ZooReaderWriter(szk.getConn(), 10_0000, "aPasswd");
 
-      var executor = Executors.newFixedThreadPool(16);
+    var executor = Executors.newFixedThreadPool(16);
 
-      String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
+    String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
 
-      List<Future<?>> futures = new ArrayList<>();
+    List<Future<?>> futures = new ArrayList<>();
 
-      // This map is used to ensure multiple threads do not successfully write the same value and no
-      // values are skipped. The hash in the value also verifies similar things in a different way.
-      ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
+    // This map is used to ensure multiple threads do not successfully write the same value and no
+    // values are skipped. The hash in the value also verifies similar things in a different way.
+    ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
 
-      for (int i = 0; i < 16; i++) {
-        futures.add(executor.submit(() -> {
-          try {
+    for (int i = 0; i < 16; i++) {
+      futures.add(executor.submit(() -> {
+        try {
 
-            int count = -1;
-            while (count < 200) {
-              byte[] val =
-                  zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
-              int nextCount = getCount(val);
-              assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
-              count = nextCount;
-              countCounts.merge(count, 1, Integer::sum);
-            }
-
-          } catch (Exception e) {
-            throw new RuntimeException(e);
+          int count = -1;
+          while (count < 200) {
+            byte[] val =
+                zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
+            int nextCount = getCount(val);
+            assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
+            count = nextCount;
+            countCounts.merge(count, 1, Integer::sum);
           }
-        }));
-      }
 
-      // wait and check for errors in background threads
-      for (Future<?> future : futures) {
-        future.get();
-      }
-      executor.shutdown();
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      }));
+    }
 
-      byte[] actual = zk.getData("/test-zm");
-      int settledCount = getCount(actual);
+    // wait and check for errors in background threads
+    for (Future<?> future : futures) {
+      future.get();
+    }
+    executor.shutdown();
 
-      assertTrue(settledCount >= 200);
+    byte[] actual = zk.getData("/test-zm");
+    int settledCount = getCount(actual);
 
-      String expected = initialData;
+    assertTrue(settledCount >= 200);
 
-      assertEquals(1, (int) countCounts.get(0));
+    String expected = initialData;
 
-      for (int i = 1; i <= settledCount; i++) {
-        assertEquals(1, (int) countCounts.get(i));
-        expected = nextValue(expected);
-      }
+    assertEquals(1, (int) countCounts.get(0));
 
-      assertEquals(settledCount + 1, countCounts.size());
-      assertEquals(expected, new String(actual, UTF_8));
+    for (int i = 1; i <= settledCount; i++) {
+      assertEquals(1, (int) countCounts.get(i));
+      expected = nextValue(expected);
     }
+
+    assertEquals(settledCount + 1, countCounts.size());
+    assertEquals(expected, new String(actual, UTF_8));
+
+    szk.close();

Review comment:
       ZooKeeperTestingServer does not implement auto-closable so it wont work in a regular try-with-resources block. I could either make the ZooKeeperTestingServer a member variable and use a try-finally to close it or, have a `@BeforeClass` and `@AfterClass` to initialize and then close it. Not sure which would be best in this case.




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

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



[GitHub] [accumulo] DomGarguilo commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789164274


   I think these changes are ready for review. I ask that the location that these files were moved to be reviewed because I am not too familiar with where the best place is for these renamed tests.


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

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



[GitHub] [accumulo] ctubbsii commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586701985



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -75,66 +70,65 @@
    */
   @Test
   public void concurrentMutatorTest() throws Exception {
-    try (var client = Accumulo.newClient().from(getClientProps()).build();
-        var context = (ClientContext) client) {
-      String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET);
 
-      ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(),
-          context.getZooKeepersSessionTimeOut(), secret);
+    ZooKeeperTestingServer szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/" + UUID.randomUUID().toString());
+    ZooReaderWriter zk = new ZooReaderWriter(szk.getConn(), 10_0000, "aPasswd");
 
-      var executor = Executors.newFixedThreadPool(16);
+    var executor = Executors.newFixedThreadPool(16);
 
-      String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
+    String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
 
-      List<Future<?>> futures = new ArrayList<>();
+    List<Future<?>> futures = new ArrayList<>();
 
-      // This map is used to ensure multiple threads do not successfully write the same value and no
-      // values are skipped. The hash in the value also verifies similar things in a different way.
-      ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
+    // This map is used to ensure multiple threads do not successfully write the same value and no
+    // values are skipped. The hash in the value also verifies similar things in a different way.
+    ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
 
-      for (int i = 0; i < 16; i++) {
-        futures.add(executor.submit(() -> {
-          try {
+    for (int i = 0; i < 16; i++) {
+      futures.add(executor.submit(() -> {
+        try {
 
-            int count = -1;
-            while (count < 200) {
-              byte[] val =
-                  zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
-              int nextCount = getCount(val);
-              assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
-              count = nextCount;
-              countCounts.merge(count, 1, Integer::sum);
-            }
-
-          } catch (Exception e) {
-            throw new RuntimeException(e);
+          int count = -1;
+          while (count < 200) {
+            byte[] val =
+                zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
+            int nextCount = getCount(val);
+            assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
+            count = nextCount;
+            countCounts.merge(count, 1, Integer::sum);
           }
-        }));
-      }
 
-      // wait and check for errors in background threads
-      for (Future<?> future : futures) {
-        future.get();
-      }
-      executor.shutdown();
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      }));
+    }
 
-      byte[] actual = zk.getData("/test-zm");
-      int settledCount = getCount(actual);
+    // wait and check for errors in background threads
+    for (Future<?> future : futures) {
+      future.get();
+    }
+    executor.shutdown();
 
-      assertTrue(settledCount >= 200);
+    byte[] actual = zk.getData("/test-zm");
+    int settledCount = getCount(actual);
 
-      String expected = initialData;
+    assertTrue(settledCount >= 200);
 
-      assertEquals(1, (int) countCounts.get(0));
+    String expected = initialData;
 
-      for (int i = 1; i <= settledCount; i++) {
-        assertEquals(1, (int) countCounts.get(i));
-        expected = nextValue(expected);
-      }
+    assertEquals(1, (int) countCounts.get(0));
 
-      assertEquals(settledCount + 1, countCounts.size());
-      assertEquals(expected, new String(actual, UTF_8));
+    for (int i = 1; i <= settledCount; i++) {
+      assertEquals(1, (int) countCounts.get(i));
+      expected = nextValue(expected);
     }
+
+    assertEquals(settledCount + 1, countCounts.size());
+    assertEquals(expected, new String(actual, UTF_8));
+
+    szk.close();

Review comment:
       It has a close method, so it might as well implement the AutoCloseable interface. Whether you use the before/after class methods or try-with-resources or try-finally pattern, might depend on case-by-case basis, but there's no reason not to add the interface to the class, since it already has an implementing method.




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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586742631



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -75,66 +70,65 @@
    */
   @Test
   public void concurrentMutatorTest() throws Exception {
-    try (var client = Accumulo.newClient().from(getClientProps()).build();
-        var context = (ClientContext) client) {
-      String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET);
 
-      ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(),
-          context.getZooKeepersSessionTimeOut(), secret);
+    ZooKeeperTestingServer szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/" + UUID.randomUUID().toString());
+    ZooReaderWriter zk = new ZooReaderWriter(szk.getConn(), 10_0000, "aPasswd");
 
-      var executor = Executors.newFixedThreadPool(16);
+    var executor = Executors.newFixedThreadPool(16);
 
-      String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
+    String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
 
-      List<Future<?>> futures = new ArrayList<>();
+    List<Future<?>> futures = new ArrayList<>();
 
-      // This map is used to ensure multiple threads do not successfully write the same value and no
-      // values are skipped. The hash in the value also verifies similar things in a different way.
-      ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
+    // This map is used to ensure multiple threads do not successfully write the same value and no
+    // values are skipped. The hash in the value also verifies similar things in a different way.
+    ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
 
-      for (int i = 0; i < 16; i++) {
-        futures.add(executor.submit(() -> {
-          try {
+    for (int i = 0; i < 16; i++) {
+      futures.add(executor.submit(() -> {
+        try {
 
-            int count = -1;
-            while (count < 200) {
-              byte[] val =
-                  zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
-              int nextCount = getCount(val);
-              assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
-              count = nextCount;
-              countCounts.merge(count, 1, Integer::sum);
-            }
-
-          } catch (Exception e) {
-            throw new RuntimeException(e);
+          int count = -1;
+          while (count < 200) {
+            byte[] val =
+                zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
+            int nextCount = getCount(val);
+            assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
+            count = nextCount;
+            countCounts.merge(count, 1, Integer::sum);
           }
-        }));
-      }
 
-      // wait and check for errors in background threads
-      for (Future<?> future : futures) {
-        future.get();
-      }
-      executor.shutdown();
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      }));
+    }
 
-      byte[] actual = zk.getData("/test-zm");
-      int settledCount = getCount(actual);
+    // wait and check for errors in background threads
+    for (Future<?> future : futures) {
+      future.get();
+    }
+    executor.shutdown();
 
-      assertTrue(settledCount >= 200);
+    byte[] actual = zk.getData("/test-zm");
+    int settledCount = getCount(actual);
 
-      String expected = initialData;
+    assertTrue(settledCount >= 200);
 
-      assertEquals(1, (int) countCounts.get(0));
+    String expected = initialData;
 
-      for (int i = 1; i <= settledCount; i++) {
-        assertEquals(1, (int) countCounts.get(i));
-        expected = nextValue(expected);
-      }
+    assertEquals(1, (int) countCounts.get(0));
 
-      assertEquals(settledCount + 1, countCounts.size());
-      assertEquals(expected, new String(actual, UTF_8));
+    for (int i = 1; i <= settledCount; i++) {
+      assertEquals(1, (int) countCounts.get(i));
+      expected = nextValue(expected);
     }
+
+    assertEquals(settledCount + 1, countCounts.size());
+    assertEquals(expected, new String(actual, UTF_8));
+
+    szk.close();

Review comment:
       These changes are included in the most recent commit.

##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -75,66 +70,65 @@
    */
   @Test
   public void concurrentMutatorTest() throws Exception {
-    try (var client = Accumulo.newClient().from(getClientProps()).build();
-        var context = (ClientContext) client) {
-      String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET);
 
-      ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(),
-          context.getZooKeepersSessionTimeOut(), secret);
+    ZooKeeperTestingServer szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/" + UUID.randomUUID().toString());
+    ZooReaderWriter zk = new ZooReaderWriter(szk.getConn(), 10_0000, "aPasswd");
 
-      var executor = Executors.newFixedThreadPool(16);
+    var executor = Executors.newFixedThreadPool(16);
 
-      String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
+    String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
 
-      List<Future<?>> futures = new ArrayList<>();
+    List<Future<?>> futures = new ArrayList<>();
 
-      // This map is used to ensure multiple threads do not successfully write the same value and no
-      // values are skipped. The hash in the value also verifies similar things in a different way.
-      ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
+    // This map is used to ensure multiple threads do not successfully write the same value and no
+    // values are skipped. The hash in the value also verifies similar things in a different way.
+    ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
 
-      for (int i = 0; i < 16; i++) {
-        futures.add(executor.submit(() -> {
-          try {
+    for (int i = 0; i < 16; i++) {
+      futures.add(executor.submit(() -> {
+        try {
 
-            int count = -1;
-            while (count < 200) {
-              byte[] val =
-                  zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
-              int nextCount = getCount(val);
-              assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
-              count = nextCount;
-              countCounts.merge(count, 1, Integer::sum);
-            }
-
-          } catch (Exception e) {
-            throw new RuntimeException(e);
+          int count = -1;
+          while (count < 200) {
+            byte[] val =
+                zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
+            int nextCount = getCount(val);
+            assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
+            count = nextCount;
+            countCounts.merge(count, 1, Integer::sum);
           }
-        }));
-      }
 
-      // wait and check for errors in background threads
-      for (Future<?> future : futures) {
-        future.get();
-      }
-      executor.shutdown();
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      }));
+    }
 
-      byte[] actual = zk.getData("/test-zm");
-      int settledCount = getCount(actual);
+    // wait and check for errors in background threads
+    for (Future<?> future : futures) {
+      future.get();
+    }
+    executor.shutdown();
 
-      assertTrue(settledCount >= 200);
+    byte[] actual = zk.getData("/test-zm");
+    int settledCount = getCount(actual);
 
-      String expected = initialData;
+    assertTrue(settledCount >= 200);
 
-      assertEquals(1, (int) countCounts.get(0));
+    String expected = initialData;
 
-      for (int i = 1; i <= settledCount; i++) {
-        assertEquals(1, (int) countCounts.get(i));
-        expected = nextValue(expected);
-      }
+    assertEquals(1, (int) countCounts.get(0));
 
-      assertEquals(settledCount + 1, countCounts.size());
-      assertEquals(expected, new String(actual, UTF_8));
+    for (int i = 1; i <= settledCount; i++) {
+      assertEquals(1, (int) countCounts.get(i));
+      expected = nextValue(expected);
     }
+
+    assertEquals(settledCount + 1, countCounts.size());
+    assertEquals(expected, new String(actual, UTF_8));
+
+    szk.close();

Review comment:
       These changes are included in the most recent commits.




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

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



[GitHub] [accumulo] EdColeman commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789206103


   It could be handled as a separate issue - especially if it needs to touch other code - but I think we should try to get resolution before changing the log level to mask the symptom if it is actually an underlying issue.  
   
   It is also something that should be explainable before releasing 2.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.

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



[GitHub] [accumulo] dlmarion commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789201388


   Or, this maybe outside the scope of this issue. I'm also fine if you want to create a new issue for investigation.


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

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



[GitHub] [accumulo] ctubbsii commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586698289



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {

Review comment:
       The simplest solution is to just create a new annotation to use as a category of test. Perhaps `start/src/main/java/org/apache/accumulo/test/categories/ZooKeeperTests`




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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r587792966



##########
File path: test/src/main/java/org/apache/accumulo/test/zookeeper/fate/ZooLockIT.java
##########
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.fate.zookeeper;
+package org.apache.accumulo.test.zookeeper.fate;

Review comment:
       I have included this change in the most recent commit.




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

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



[GitHub] [accumulo] EdColeman commented on a change in pull request #1952: Convert ZooLockITs use of MAC to ZooKeeperTestingServer. Fixes #1885

Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r584941877



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {
+
+  private static ZooKeeperTestingServer szk = null;
 
   @BeforeClass
   public static void setup() throws Exception {
-    SharedMiniClusterBase.startMiniCluster();
+    szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/1234");

Review comment:
       You may want to consider using UUID.randomUUID().toString() - and if the UUID is used elsewhere, create / initialize it as a static final variable for the class.  This more closely emulates the Accumulo instance id / paths used by Accumulo. 
   
   With a quick glance I think because you are using the `super.create(path, data, acl, createMode);` later - will also create a path that is `/accumulo/[instance-id]`
   




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

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



[GitHub] [accumulo] dlmarion commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789170221


   Seems that the spammy log messages must be due to the switch from MAC to the Curator test ZK server. I don't see anything mentioned online about watches being invoked in the test server.


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

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



[GitHub] [accumulo] EdColeman commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789172258


   Can you determine where / what is causing the log messages?  Its not so much the appropriateness of the log level - but rather if there is an potential performance issue where the condition that is triggering the logging is occurring more frequently than originally expected / necessary?  Also, are these messages generated frequently while running (say using uno instance) or is it something specific to testing?


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

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



[GitHub] [accumulo] dlmarion commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789200931


   > Could one possibility be if something was trying to get the lock and didn't because something else held it that it is repeatedly deleting and retrying, failing,....
   
   It would be useful to know which test or tests is causing it. I believe I only added two tests in my ZooLock changes (testLockSerial and testLockParallel)
   
   @DomGarguilo - would you be able to run the tests independently to see if you can narrow down which one(s) are causing the lock watcher to renew?


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

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



[GitHub] [accumulo] ctubbsii commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r585931112



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -16,31 +16,26 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.functional;
+package org.apache.accumulo.test.metrics.fate;

Review comment:
       Not a metrics test.

##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {

Review comment:
       While it may not need to extend SharedMiniClusterBase anymore, it is still an integration test, as it is specifically testing the integration between Accumulo code and a test instance of ZooKeeper that is started as an extra process. It should keep its original name, `ZooLockIT`.
   
   The existing `FateMetricsTest` should probably also be called `FateMetricsIT` for the same reason, if you want to rename it for consistency, as part of this PR.

##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.fate.zookeeper;
+package org.apache.accumulo.test.metrics.fate;

Review comment:
       This should not be relocated to a metrics subpackage. It is not related to metrics.

##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -75,66 +70,65 @@
    */
   @Test
   public void concurrentMutatorTest() throws Exception {
-    try (var client = Accumulo.newClient().from(getClientProps()).build();
-        var context = (ClientContext) client) {
-      String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET);
 
-      ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(),
-          context.getZooKeepersSessionTimeOut(), secret);
+    ZooKeeperTestingServer szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/" + UUID.randomUUID().toString());
+    ZooReaderWriter zk = new ZooReaderWriter(szk.getConn(), 10_0000, "aPasswd");
 
-      var executor = Executors.newFixedThreadPool(16);
+    var executor = Executors.newFixedThreadPool(16);
 
-      String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
+    String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
 
-      List<Future<?>> futures = new ArrayList<>();
+    List<Future<?>> futures = new ArrayList<>();
 
-      // This map is used to ensure multiple threads do not successfully write the same value and no
-      // values are skipped. The hash in the value also verifies similar things in a different way.
-      ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
+    // This map is used to ensure multiple threads do not successfully write the same value and no
+    // values are skipped. The hash in the value also verifies similar things in a different way.
+    ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
 
-      for (int i = 0; i < 16; i++) {
-        futures.add(executor.submit(() -> {
-          try {
+    for (int i = 0; i < 16; i++) {
+      futures.add(executor.submit(() -> {
+        try {
 
-            int count = -1;
-            while (count < 200) {
-              byte[] val =
-                  zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
-              int nextCount = getCount(val);
-              assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
-              count = nextCount;
-              countCounts.merge(count, 1, Integer::sum);
-            }
-
-          } catch (Exception e) {
-            throw new RuntimeException(e);
+          int count = -1;
+          while (count < 200) {
+            byte[] val =
+                zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
+            int nextCount = getCount(val);
+            assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
+            count = nextCount;
+            countCounts.merge(count, 1, Integer::sum);
           }
-        }));
-      }
 
-      // wait and check for errors in background threads
-      for (Future<?> future : futures) {
-        future.get();
-      }
-      executor.shutdown();
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      }));
+    }
 
-      byte[] actual = zk.getData("/test-zm");
-      int settledCount = getCount(actual);
+    // wait and check for errors in background threads
+    for (Future<?> future : futures) {
+      future.get();
+    }
+    executor.shutdown();
 
-      assertTrue(settledCount >= 200);
+    byte[] actual = zk.getData("/test-zm");
+    int settledCount = getCount(actual);
 
-      String expected = initialData;
+    assertTrue(settledCount >= 200);
 
-      assertEquals(1, (int) countCounts.get(0));
+    String expected = initialData;
 
-      for (int i = 1; i <= settledCount; i++) {
-        assertEquals(1, (int) countCounts.get(i));
-        expected = nextValue(expected);
-      }
+    assertEquals(1, (int) countCounts.get(0));
 
-      assertEquals(settledCount + 1, countCounts.size());
-      assertEquals(expected, new String(actual, UTF_8));
+    for (int i = 1; i <= settledCount; i++) {
+      assertEquals(1, (int) countCounts.get(i));
+      expected = nextValue(expected);
     }
+
+    assertEquals(settledCount + 1, countCounts.size());
+    assertEquals(expected, new String(actual, UTF_8));
+
+    szk.close();

Review comment:
       Instead of a close here, the szk should be declared as a resource in a try-with-resources block, so it gets closed, even if the other code throws an exception.

##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -16,31 +16,26 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.functional;
+package org.apache.accumulo.test.metrics.fate;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 
-import org.apache.accumulo.core.client.Accumulo;
-import org.apache.accumulo.core.clientImpl.ClientContext;
-import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.fate.zookeeper.ZooReaderWriter;
-import org.apache.accumulo.harness.AccumuloClusterHarness;
-import org.apache.accumulo.test.categories.MiniClusterOnlyTests;
+import org.apache.accumulo.test.zookeeper.ZooKeeperTestingServer;
 import org.junit.Test;
-import org.junit.experimental.categories.Category;
 
 import com.google.common.hash.Hashing;
 
-@Category(MiniClusterOnlyTests.class)
-public class ZooMutatorIT extends AccumuloClusterHarness {
+public class ZooMutatorTest {

Review comment:
       Still an IT.




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

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



[GitHub] [accumulo] DomGarguilo commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789210532


   > It could be handled as a separate issue - especially if it needs to touch other code - but I think we should try to get resolution before changing the log level to mask the symptom if it is actually an underlying issue.
   
   Sounds good to me. I can revert my change to the log in this PR and then create a separate issue.


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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586500695



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {

Review comment:
       I agree that this should be an IT. I was running into the issue that @Manno15 has outlined. I'm not sure what the best way to overcome this issue might be and am open to suggestions.




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

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



[GitHub] [accumulo] DomGarguilo commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789172523


   > Seems that the spammy log messages must be due to the switch from MAC to the Curator test ZK server. I don't see anything mentioned online about watches being invoked in the test server.
   
   @dlmarion I am seeing the spammy log message on both versions. That is before _and_ after the switch from MAC to ZooKeeperTestServer.


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

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



[GitHub] [accumulo] DomGarguilo commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789837306


   > It could be handled as a separate issue - especially if it needs to touch other code - but I think we should try to get resolution before changing the log level to mask the symptom if it is actually an underlying issue.
   
   I have created #1955 to track this issue. @EdColeman @dlmarion 


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

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



[GitHub] [accumulo] dlmarion commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789175479


   I wonder if this was always happening. I went back to the code before my changes, at https://github.com/apache/accumulo/blob/f88cb3bcebb744d7d1f3150877243c756d717ddb/core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java. The log message was at trace. I must have changed it to debug in my ZooLock UUID changes.


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

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



[GitHub] [accumulo] Manno15 commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
Manno15 commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586376320



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {

Review comment:
       This rule in our `pom.xml` will cause the build to fail if we keep the IT nomenclature and do not extend one of the classes (or add it to the test itself) that includes the import specified below: 
   ```
   <classAnnotationPattern>org[.]junit[.]experimental[.]categories[.]Category</classAnnotationPattern>
   ```
   https://github.com/apache/accumulo/blob/b12d1103d788473168616f28ae87a65bb76aa880/pom.xml#L1176-L1183




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

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



[GitHub] [accumulo] DomGarguilo commented on pull request #1952: Convert ZooLockITs use of MAC to ZooKeeperTestingServer. Fixes #1885

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-788202642


   My most recent commit ([af4e4b2](https://github.com/apache/accumulo/pull/1952/commits/af4e4b25e89d412308890ea52fcf108d3777e3e4)) is somewhat unrelated however when this test is run, the logs are inundated with the following log at the rate of hundreds per second:
   ```java
   2021-02-26T16:23:31,422 [zookeeper.ZooLock] DEBUG: [zlock#81282aef-d8a3-4bf1-a376-23a31e1d7b12#] Renewing watch on prior node  /zltestDeleteWaiting-1025168844-l1/zlock#eadf5a4c-680e-411f-92b5-62a55c615031#0000000000
   ```
   
   Thought I would include this change here to avoid making another PR for this. For reference, another PR has been recently merged with a very similar change #1918.


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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r586496272



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooMutatorTest.java
##########
@@ -75,66 +70,65 @@
    */
   @Test
   public void concurrentMutatorTest() throws Exception {
-    try (var client = Accumulo.newClient().from(getClientProps()).build();
-        var context = (ClientContext) client) {
-      String secret = cluster.getSiteConfiguration().get(Property.INSTANCE_SECRET);
 
-      ZooReaderWriter zk = new ZooReaderWriter(context.getZooKeepers(),
-          context.getZooKeepersSessionTimeOut(), secret);
+    ZooKeeperTestingServer szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/" + UUID.randomUUID().toString());
+    ZooReaderWriter zk = new ZooReaderWriter(szk.getConn(), 10_0000, "aPasswd");
 
-      var executor = Executors.newFixedThreadPool(16);
+    var executor = Executors.newFixedThreadPool(16);
 
-      String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
+    String initialData = hash("Accumulo Zookeeper Mutator test data") + " 0";
 
-      List<Future<?>> futures = new ArrayList<>();
+    List<Future<?>> futures = new ArrayList<>();
 
-      // This map is used to ensure multiple threads do not successfully write the same value and no
-      // values are skipped. The hash in the value also verifies similar things in a different way.
-      ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
+    // This map is used to ensure multiple threads do not successfully write the same value and no
+    // values are skipped. The hash in the value also verifies similar things in a different way.
+    ConcurrentHashMap<Integer,Integer> countCounts = new ConcurrentHashMap<>();
 
-      for (int i = 0; i < 16; i++) {
-        futures.add(executor.submit(() -> {
-          try {
+    for (int i = 0; i < 16; i++) {
+      futures.add(executor.submit(() -> {
+        try {
 
-            int count = -1;
-            while (count < 200) {
-              byte[] val =
-                  zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
-              int nextCount = getCount(val);
-              assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
-              count = nextCount;
-              countCounts.merge(count, 1, Integer::sum);
-            }
-
-          } catch (Exception e) {
-            throw new RuntimeException(e);
+          int count = -1;
+          while (count < 200) {
+            byte[] val =
+                zk.mutateOrCreate("/test-zm", initialData.getBytes(UTF_8), this::nextValue);
+            int nextCount = getCount(val);
+            assertTrue("nextCount <= count " + nextCount + " " + count, nextCount > count);
+            count = nextCount;
+            countCounts.merge(count, 1, Integer::sum);
           }
-        }));
-      }
 
-      // wait and check for errors in background threads
-      for (Future<?> future : futures) {
-        future.get();
-      }
-      executor.shutdown();
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+      }));
+    }
 
-      byte[] actual = zk.getData("/test-zm");
-      int settledCount = getCount(actual);
+    // wait and check for errors in background threads
+    for (Future<?> future : futures) {
+      future.get();
+    }
+    executor.shutdown();
 
-      assertTrue(settledCount >= 200);
+    byte[] actual = zk.getData("/test-zm");
+    int settledCount = getCount(actual);
 
-      String expected = initialData;
+    assertTrue(settledCount >= 200);
 
-      assertEquals(1, (int) countCounts.get(0));
+    String expected = initialData;
 
-      for (int i = 1; i <= settledCount; i++) {
-        assertEquals(1, (int) countCounts.get(i));
-        expected = nextValue(expected);
-      }
+    assertEquals(1, (int) countCounts.get(0));
 
-      assertEquals(settledCount + 1, countCounts.size());
-      assertEquals(expected, new String(actual, UTF_8));
+    for (int i = 1; i <= settledCount; i++) {
+      assertEquals(1, (int) countCounts.get(i));
+      expected = nextValue(expected);
     }
+
+    assertEquals(settledCount + 1, countCounts.size());
+    assertEquals(expected, new String(actual, UTF_8));
+
+    szk.close();

Review comment:
       ZooKeeperTestingServer does not implement auto-closable so it wont work in a regular try-with-resources block. I could either make the ZooKeeperTestingServer a member variable and use a try-finally to close it or, have a @BeforeClass and @AfterClass to initialize and then close it. Not sure which would be best in this case.




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

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



[GitHub] [accumulo] ctubbsii merged pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952


   


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

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



[GitHub] [accumulo] EdColeman commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789187479


   Quickly looking at the original code - it seems the watcher is being set on a node basis - if that is correct it does not seem "normal" that a single node watcher should be firing enough to be frequent in the logs, not saying that its not in the logs, rather I don't understand why its in the log - and that's an independent issue of the log level.
   
   Could one possibility be if something was trying to get the lock and didn't because something else held it that it is repeatedly deleting and retrying, failing,....


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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r587673562



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {

Review comment:
       @ctubbsii, the most recent commit should include the remaining changes you have mentioned.




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

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



[GitHub] [accumulo] DomGarguilo commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789163506


   I went back to check if the spammy debug logs that I was seeing were happening as a result of my changes and confirmed that they were not. 
   
   Without my change that drops the log to trace, the log would be displayed hundreds of times a second.


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

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



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1952: Convert ZooLockITs use of MAC to ZooKeeperTestingServer. Fixes #1885

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#discussion_r584969908



##########
File path: test/src/main/java/org/apache/accumulo/test/metrics/fate/ZooLockTest.java
##########
@@ -57,16 +57,19 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ZooLockIT extends SharedMiniClusterBase {
+public class ZooLockTest {
+
+  private static ZooKeeperTestingServer szk = null;
 
   @BeforeClass
   public static void setup() throws Exception {
-    SharedMiniClusterBase.startMiniCluster();
+    szk = new ZooKeeperTestingServer();
+    szk.initPaths("/accumulo/1234");

Review comment:
       Good idea. Will include this in my next commit.




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

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



[GitHub] [accumulo] dlmarion commented on pull request #1952: Convert tests from MAC to ZooKeeperTestingServer

Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #1952:
URL: https://github.com/apache/accumulo/pull/1952#issuecomment-789173112


   @DomGarguilo - yep, I misread your comment


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

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