ZOOKEEPER-5097: Make DataTree snapshot serialization iterative

zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java

@@ -21,9 +21,11 @@
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
@@ -1318,29 +1320,44 @@ private void setupQuota() {
      * @throws IOException
      */
     void serializeNode(OutputArchive oa, StringBuilder path) throws IOException {
-        String pathString = path.toString();
-        DataNode node = getNode(pathString);
-        if (node == null) {
-            return;
-        }
-        String[] children;
-        DataNode nodeCopy;
-        synchronized (node) {
-            StatPersisted statCopy = new StatPersisted();
-            statCopy.copyFrom(node.stat);
-            //we do not need to make a copy of node.data because the contents
-            //are never changed
-            nodeCopy = new DataNode(node.data, node.acl, statCopy);
-            children = node.getChildren().toArray(new String[0]);
-        }
-        serializeNodeData(oa, pathString, nodeCopy);
-        path.append('/');
-        int off = path.length();
-        for (String child : children) {
-            // Since this is single buffer being reused, we need to truncate the previous bytes of string.
-            path.delete(off, Integer.MAX_VALUE);
-            path.append(child);
-            serializeNode(oa, path);
+        // Iterative, explicit-stack pre-order traversal of the subtree rooted
+        // at `path`. This was previously written as recursion with one frame
+        // per znode level, which does not scale to very deep trees; walking the
+        // tree with a heap-allocated stack removes the dependency on tree depth.
+        //
+        // The traversal order (pre-order, children in the same order as
+        // before), the per-node locking, and the emitted records are unchanged,
+        // so the produced snapshot bytes are identical and the on-disk format is
+        // unchanged. The explicit stack holds at most the unvisited frontier of
+        // the tree (a single entry for a chain), all of which the in-memory
+        // DataTree already holds.
+        Deque<String> pathStack = new ArrayDeque<>();
+        pathStack.push(path.toString());
+        while (!pathStack.isEmpty()) {
+            String pathString = pathStack.pop();
+            DataNode node = getNode(pathString);
+            if (node == null) {
+                continue;
+            }
+            String[] children;
+            DataNode nodeCopy;
+            synchronized (node) {
+                StatPersisted statCopy = new StatPersisted();
+                statCopy.copyFrom(node.stat);
+                //we do not need to make a copy of node.data because the contents
+                //are never changed
+                nodeCopy = new DataNode(node.data, node.acl, statCopy);
+                children = node.getChildren().toArray(new String[0]);
+            }
+            // Called outside the node lock: writeRecord can block and holding
+            // the lock would stall writers (ZOOKEEPER-2201).
+            serializeNodeData(oa, pathString, nodeCopy);
+            // Push children in reverse so that, popped from this LIFO stack,
+            // they are visited in the same order as the previous recursion.
+            String childPathPrefix = pathString + "/";
+            for (int i = children.length - 1; i >= 0; i--) {
+                pathStack.push(childPathPrefix + children[i]);
+            }
         }
     }
 

zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java

@@ -418,6 +418,47 @@ public void run() {
         assertTrue(ranTestCase.get(), "Didn't find the expected node");
     }
 
+    /**
+     * Snapshot serialization must not depend on tree depth. This builds a very
+     * deep znode chain and serializes it on a thread with a small (512 KiB)
+     * stack, so the test is independent of the runner's default -Xss, then
+     * checks the result round-trips back to an equivalent tree.
+     */
+    @Test
+    @Timeout(value = 120)
+    public void testSerializeDeepTree() throws Exception {
+        final int depth = 4000;
+        final DataTree tree = new DataTree();
+        final StringBuilder path = new StringBuilder();
+        for (int i = 0; i < depth; i++) {
+            path.append("/a");
+            tree.createNode(path.toString(), new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, -1L, i + 1, i + 1, 1);
+        }
+
+        final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        final BinaryOutputArchive oa = BinaryOutputArchive.getArchive(baos);
+        final Throwable[] failure = new Throwable[1];
+        final Thread serializer = new Thread(null, () -> {
+            try {
+                tree.serialize(oa, "tree");
+            } catch (Throwable t) {
+                failure[0] = t;
+            }
+        }, "deep-tree-serializer", 512 * 1024L);
+        serializer.start();
+        serializer.join();
+
+        assertNull(failure[0], "serialize() must not fail on a deep tree, but threw: " + failure[0]);
+
+        // The produced bytes must load back into an equivalent tree.
+        final DataTree restored = new DataTree();
+        final BinaryInputArchive ia =
+            BinaryInputArchive.getArchive(new ByteArrayInputStream(baos.toByteArray()));
+        restored.deserialize(ia, "tree");
+        assertNotNull(restored.getNode(path.toString()),
+            "the deepest node must survive a serialize/deserialize round-trip");
+    }
+
     @Test
     @Timeout(value = 60)
     public void testReconfigACLClearOnDeserialize() throws Exception {