keith-turner commented on code in PR #6073:
URL: https://github.com/apache/accumulo/pull/6073#discussion_r2728533261
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java:
##########
@@ -754,6 +755,44 @@ public static void deleteLocks(ZooReaderWriter zk, String
zPath,
}
}
+ @Deprecated(since = "2.1.5")
Review Comment:
Why mark this deprecated? This package is not in the public API, so does
not seem needed.
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java:
##########
@@ -754,6 +755,44 @@ public static void deleteLocks(ZooReaderWriter zk, String
zPath,
}
}
+ @Deprecated(since = "2.1.5")
+ public static void deleteScanServerLocks(ZooReaderWriter zk, String zPath,
+ Predicate<HostAndPort> hostPortPredicate, Predicate<String>
groupPredicate,
+ Consumer<String> messageOutput, Boolean dryRun) throws KeeperException,
InterruptedException {
+
+ Objects.requireNonNull(zPath, "Lock path cannot be null");
+ Objects.requireNonNull(groupPredicate, "group predicate cannot be null");
+ if (!zk.exists(zPath)) {
+ throw new IllegalStateException("Path " + zPath + " does not exist");
+ }
+
+ List<String> servers = zk.getChildren(zPath);
+ if (servers.isEmpty()) {
+ throw new IllegalStateException("No server locks are held at " + zPath);
Review Comment:
Will this cause a failure if there are no scan servers? If so attempting to
delete some scan servers when there are no scan servers seems like it should
not fail.
##########
server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java:
##########
@@ -93,4 +102,114 @@ public void testCannotQualifySessionId() {
EasyMock.verify(zc);
}
+ /**
+ * SServer group filter should use lock data (UUID,group).
+ */
+ @SuppressWarnings("deprecation")
+ @Test
+ public void testSserverGroupFilterUsesLockData() throws Exception {
+
+ ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class);
+ ZooKeeper zk = EasyMock.createNiceMock(ZooKeeper.class);
+
+ String basePath = "/accumulo/iid/sservers";
+ String hostDefault = "host1:10000";
+ String hostOther = "host2:10001";
+ String zlock1 = "zlock#00000000-0000-0000-0000-aaaaaaaaaaaa#0000000001";
+ String zlock2 = "zlock#00000000-0000-0000-0000-bbbbbbbbbbbb#0000000001";
+
+ EasyMock.expect(zoo.exists(basePath)).andReturn(true);
+ EasyMock.expect(zoo.getChildren(basePath)).andReturn(List.of(hostDefault,
hostOther));
+ EasyMock.expect(zoo.getZooKeeper()).andReturn(zk);
+ EasyMock.expect(zk.getChildren(basePath + "/" + hostDefault,
null)).andReturn(List.of(zlock1));
+ EasyMock.expect(zk.getData(basePath + "/" + hostDefault + "/" + zlock1,
false, null))
+ .andReturn((UUID.randomUUID().toString() +
",default").getBytes(UTF_8));
+ EasyMock.expect(zk.getChildren(basePath + "/" + hostOther,
null)).andReturn(List.of(zlock2));
+ EasyMock.expect(zk.getData(basePath + "/" + hostOther + "/" + zlock2,
false, null))
+ .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8));
+
+ AtomicBoolean deletedDefault = new AtomicBoolean(false);
Review Comment:
Rather than using these booleans could strict mocks be used instead that
fail if any unexpected `recursiveDelete` call is seen?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
