keith-turner commented on code in PR #4861:
URL: https://github.com/apache/accumulo/pull/4861#discussion_r1759498246
##########
test/src/main/java/org/apache/accumulo/test/functional/BackupManagerIT.java:
##########
@@ -58,7 +56,7 @@ public void test() throws Exception {
// generate a false zookeeper event
List<String> children =
ServiceLock.validateAndSort(path,
writer.getChildren(path.toString()));
- String lockPath = root + Constants.ZMANAGER_LOCK + "/" + children.get(0);
+ String lockPath = path.toString() + "/" + children.get(0);
Review Comment:
```suggestion
String lockPath = path + "/" + children.get(0);
```
##########
server/base/src/main/java/org/apache/accumulo/server/manager/LiveTServerSet.java:
##########
@@ -314,24 +321,17 @@ public void process(WatchedEvent event) {
if (event.getPath().endsWith(Constants.ZTSERVERS)) {
scanServers();
} else if (event.getPath().contains(Constants.ZTSERVERS)) {
Review Comment:
Would this work here? Was trying to figure out how to leverage the new code
here.
```java
} else {
try{
final ServiceLockPath slp =
ServiceLockPaths.parse(Optional.empty(), event.getPath());
if (slp.getType().equals(Constants.ZTSERVERS)) {
```
##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -258,16 +258,18 @@ protected void checkIfCanceled() {
protected void announceExistence(HostAndPort clientAddress)
throws KeeperException, InterruptedException {
- String hostPort = ExternalCompactionUtil.getHostPortString(clientAddress);
-
ZooReaderWriter zoo = getContext().getZooReaderWriter();
- String compactorQueuePath =
- getContext().getZooKeeperRoot() + Constants.ZCOMPACTORS + "/" +
this.getResourceGroup();
- String zPath = compactorQueuePath + "/" + hostPort;
+ final ServiceLockPath path =
+ getContext().getServerPaths().createCompactorPath(getResourceGroup(),
clientAddress);
+ // The ServiceLockPath contains a resource group in the path which is not
created
+ // at initialization time. If it does not exist, then create it.
+ final String compactorGroupPath =
+ path.toString().substring(0, path.toString().lastIndexOf("/" +
path.getServer()));
+ LOG.debug("Creating compactor resource group path in zookeeper: {}",
compactorGroupPath);
try {
- zoo.mkdirs(compactorQueuePath);
- zoo.putPersistentData(zPath, new byte[] {}, NodeExistsPolicy.SKIP);
+ zoo.mkdirs(compactorGroupPath);
+ zoo.putPersistentData(path.toString(), new byte[] {},
NodeExistsPolicy.SKIP);
Review Comment:
No suggesting any changes for this PR. Wondering how we could have common
code for creating the ZK node for a RG. Seems like compactor, scan sever, and
tablet server all need to do this. Would be nice to avoid making assumptions
about that path here and maybe some push this into ServiceLockPaths.
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java:
##########
@@ -347,4 +350,27 @@ private void handlePartialSplits(ServerContext context,
String table) {
throw new IllegalStateException(e);
}
}
+
+ private void validateEmptyZKWorkerServerPaths(ServerContext context) {
+ // #4861 added the resource group to the compactor, sserver, tserver
+ // and dead tserver zookeeper paths. Make sure that the these paths
+ // are empty. This means that for the Accumulo 4.0 upgrade, the Manager
+ // should be started first before any other process.
+ final String zkRoot = ZooUtil.getRoot(context.getInstanceID());
+ final ZooReader zr = context.getZooReader();
+ for (String serverPath : new String[] {Constants.ZCOMPACTORS,
Constants.ZSSERVERS,
+ Constants.ZTSERVERS, Constants.ZDEADTSERVERS}) {
+ try {
+ List<String> children = zr.getChildren(zkRoot + serverPath);
+ for (String child : children) {
+ if (child.contains(":")) {
Review Comment:
Do we have any limits on what can be in a RG name? We probably should, not
sure if there is. Wondering if a RG name could contain `:`
--
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]
