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

dlmarion 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 42b3c57591 Started upgrade test, fixed issue in upgradeZookeeper 
(#5344)
42b3c57591 is described below

commit 42b3c57591433c4a510b5b815575838c50f0c0db
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Thu Feb 20 16:39:11 2025 -0500

    Started upgrade test, fixed issue in upgradeZookeeper (#5344)
    
    The Upgrader12to13.validateEmptyZKWorkerServerPaths method was
    checking for no children in the zookeeper paths for compactors,
    scan servers, tablet server, and dead tablet servers as an
    indication that no servers were current running. Prior to recent
    changes in 4.0, zookeeper paths were not cleaned up for server
    processes when those processes are stopped. Modified the logic
    in the method to remove any children under those paths that are
    empty. Started the Upgrader12to13 unit test.
    
    Closes #5343
---
 .../accumulo/manager/upgrade/Upgrader12to13.java   | 15 +++-
 .../manager/upgrade/Upgrader12to13Test.java        | 79 ++++++++++++++++++++++
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git 
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java
 
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java
index d2d73dd636..3c7736d2a9 100644
--- 
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java
+++ 
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java
@@ -41,6 +41,7 @@ import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.fate.zookeeper.ZooReader;
 import org.apache.accumulo.core.fate.zookeeper.ZooUtil;
+import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeMissingPolicy;
 import org.apache.accumulo.core.metadata.AccumuloTable;
 import org.apache.accumulo.core.metadata.schema.Ample.DataLevel;
 import org.apache.accumulo.core.metadata.schema.Ample.TabletsMutator;
@@ -363,8 +364,18 @@ public class Upgrader12to13 implements Upgrader {
         List<String> children = zr.getChildren(zkRoot + serverPath);
         for (String child : children) {
           if (child.contains(":")) {
-            throw new IllegalStateException("Found server address at " + 
serverPath + "/" + child
-                + ". Was expecting either a resource group name or nothing. 
Stop any referenced servers.");
+            String childPath = zkRoot + serverPath + "/" + child;
+            if (zr.getChildren(childPath).isEmpty()) {
+              // child is likely host:port and is an empty directory. Since 
there
+              // is no lock here, then the server is likely down (or should 
be).
+              // Remove the entry and move on.
+              
context.getZooSession().asReaderWriter().recursiveDelete(childPath,
+                  NodeMissingPolicy.SKIP);
+            } else {
+              throw new IllegalStateException("Found server address at " + 
serverPath + "/" + child
+                  + " with content in the directory. Was expecting either a 
nothing, a resource group name or an empty directory."
+                  + " Stop any referenced servers.");
+            }
           }
         }
       } catch (InterruptedException | KeeperException e) {
diff --git 
a/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader12to13Test.java
 
b/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader12to13Test.java
new file mode 100644
index 0000000000..ee89172106
--- /dev/null
+++ 
b/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader12to13Test.java
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.manager.upgrade;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import java.util.UUID;
+
+import org.apache.accumulo.core.Constants;
+import org.apache.accumulo.core.data.InstanceId;
+import org.apache.accumulo.core.fate.zookeeper.ZooReader;
+import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter;
+import org.apache.accumulo.core.fate.zookeeper.ZooUtil;
+import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeMissingPolicy;
+import org.apache.accumulo.core.zookeeper.ZooSession;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.zookeeper.KeeperException;
+import org.junit.jupiter.api.Test;
+
+public class Upgrader12to13Test {
+
+  @Test
+  public void testZooKeeperUpgradeFailsServerCheck() throws 
InterruptedException, KeeperException {
+    Upgrader12to13 upgrader = new Upgrader12to13();
+
+    InstanceId iid = InstanceId.of(UUID.randomUUID());
+
+    ServerContext context = createMock(ServerContext.class);
+    ZooSession zk = createStrictMock(ZooSession.class);
+    ZooReader zr = createStrictMock(ZooReader.class);
+    ZooReaderWriter zrw = createStrictMock(ZooReaderWriter.class);
+    final var zkRoot = ZooUtil.getRoot(iid);
+
+    expect(context.getInstanceID()).andReturn(iid).anyTimes();
+    expect(context.getZooSession()).andReturn(zk).anyTimes();
+    expect(zk.asReader()).andReturn(zr).anyTimes();
+    expect(zk.asReaderWriter()).andReturn(zrw).anyTimes();
+    expect(context.getZooKeeperRoot()).andReturn(zkRoot).anyTimes();
+
+    expect(zr.getChildren(zkRoot + 
Constants.ZCOMPACTORS)).andReturn(List.of());
+    expect(zr.getChildren(zkRoot + 
Constants.ZSSERVERS)).andReturn(List.of("localhost:9996"));
+    expect(zr.getChildren(zkRoot + Constants.ZSSERVERS + 
"/localhost:9996")).andReturn(List.of());
+    zrw.recursiveDelete(zkRoot + Constants.ZSSERVERS + "/localhost:9996", 
NodeMissingPolicy.SKIP);
+    expect(zr.getChildren(zkRoot + 
Constants.ZTSERVERS)).andReturn(List.of("localhost:9997"));
+    expect(zr.getChildren(zkRoot + Constants.ZTSERVERS + "/localhost:9997"))
+        .andReturn(List.of(UUID.randomUUID().toString()));
+
+    replay(context, zk, zr, zrw);
+    IllegalStateException e =
+        assertThrows(IllegalStateException.class, () -> 
upgrader.upgradeZookeeper(context));
+    assertTrue(e.getMessage()
+        .contains("Was expecting either a nothing, a resource group name or an 
empty directory"));
+    verify(context, zk, zr, zrw);
+
+  }
+}

Reply via email to