This is an automated email from the ASF dual-hosted git repository.
smengcl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 6a4513ddaec HDDS-15826. Recon: add cycle guard to NSSummary /du tree
walks (#10723)
6a4513ddaec is described below
commit 6a4513ddaec9ab08fd25c0c74b43da2cc7fa43c9
Author: Siyao Meng <[email protected]>
AuthorDate: Wed Aug 12 02:48:54 2026 -0700
HDDS-15826. Recon: add cycle guard to NSSummary /du tree walks (#10723)
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
.../org/apache/hadoop/ozone/recon/ReconServer.java | 10 ++
.../org/apache/hadoop/ozone/recon/ReconUtils.java | 55 +++---
.../ozone/recon/api/handlers/EntityHandler.java | 122 +++++++++++--
.../ozone/recon/metrics/NSSummaryMetrics.java | 91 ++++++++++
.../recon/spi/ReconNamespaceSummaryManager.java | 6 +
.../spi/impl/ReconNamespaceSummaryManagerImpl.java | 21 ++-
.../apache/hadoop/ozone/recon/TestReconUtils.java | 40 +++++
.../api/handlers/TestEntityHandlerCycleGuard.java | 196 +++++++++++++++++++++
.../ozone/recon/metrics/TestNSSummaryMetrics.java | 60 +++++++
9 files changed, 558 insertions(+), 43 deletions(-)
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
index 76858731d80..07956814d4f 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java
@@ -47,6 +47,7 @@
import org.apache.hadoop.hdds.utils.HddsServerUtil;
import org.apache.hadoop.ozone.OzoneSecurityUtil;
import org.apache.hadoop.ozone.recon.api.types.FeatureProvider;
+import org.apache.hadoop.ozone.recon.metrics.NSSummaryMetrics;
import org.apache.hadoop.ozone.recon.metrics.ReconTaskStatusMetrics;
import org.apache.hadoop.ozone.recon.scm.ReconSafeModeManager;
import org.apache.hadoop.ozone.recon.scm.ReconStorageConfig;
@@ -87,6 +88,7 @@ public class ReconServer extends GenericCli implements
Callable<Void> {
private OzoneConfiguration configuration;
private ReconStorageConfig reconStorage;
private CertificateClient certClient;
+ private NSSummaryMetrics nsSummaryMetrics;
private ReconTaskStatusMetrics reconTaskStatusMetrics;
private OzoneAdmins reconAdmins;
@@ -172,6 +174,7 @@ public Void call() throws Exception {
this.reconTaskStatusMetrics =
injector.getInstance(ReconTaskStatusMetrics.class);
+ this.nsSummaryMetrics = injector.getInstance(NSSummaryMetrics.class);
LOG.info("Initializing support of Recon Features...");
FeatureProvider.initFeatureSupport(configuration);
@@ -200,6 +203,10 @@ public Void call() throws Exception {
reconTaskStatusMetrics.register();
LOG.debug("ReconTaskStatusMetrics registered after schema upgrade");
}
+ if (nsSummaryMetrics != null) {
+ nsSummaryMetrics.register();
+ LOG.debug("NSSummaryMetrics registered after schema upgrade");
+ }
LOG.info("Recon server initialized successfully!");
} catch (Exception e) {
@@ -336,6 +343,9 @@ public void stop() {
if (reconTaskStatusMetrics != null) {
reconTaskStatusMetrics.unregister();
}
+ if (nsSummaryMetrics != null) {
+ nsSummaryMetrics.unregister();
+ }
isStarted = false;
if (reconDBProvider != null) {
try {
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java
index bddbb6da572..94028c0d55f 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java
@@ -43,9 +43,12 @@
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.Instant;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
+import java.util.Deque;
+import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -781,27 +784,37 @@ public static void gatherSubPaths(long parentId,
List<String> subPaths,
long volumeID, long bucketID,
ReconNamespaceSummaryManager
reconNamespaceSummaryManager)
throws IOException {
- // Fetch the NSSummary object for parentId
- NSSummary parentSummary =
- reconNamespaceSummaryManager.getNSSummary(parentId);
- if (parentSummary == null) {
- return;
- }
-
- Set<Long> childDirIds = parentSummary.getChildDir();
- for (Long childId : childDirIds) {
- // Fetch the NSSummary for each child directory
- NSSummary childSummary =
- reconNamespaceSummaryManager.getNSSummary(childId);
- if (childSummary != null) {
- String subPath =
- ReconUtils.constructObjectPathWithPrefix(volumeID, bucketID,
- childId);
- // Add to subPaths
- subPaths.add(subPath);
- // Recurse into this child directory
- gatherSubPaths(childId, subPaths, volumeID, bucketID,
- reconNamespaceSummaryManager);
+ // Iterative traversal with a visited-set cycle guard. A corrupted
NSSummary
+ // tree (e.g. a directory listing itself or an ancestor as a child) would
+ // otherwise recurse forever and crash Recon with a StackOverflowError.
+ // Each node is fetched once, on pop, and its path recorded (except the
+ // traversal root, which is the caller's starting directory).
+ Set<Long> visited = new HashSet<>();
+ Deque<Long> stack = new ArrayDeque<>();
+ stack.push(parentId);
+ visited.add(parentId);
+ boolean cycleLogged = false;
+ while (!stack.isEmpty()) {
+ long currentId = stack.pop();
+ NSSummary summary = reconNamespaceSummaryManager.getNSSummary(currentId);
+ if (summary == null) {
+ continue;
+ }
+ if (currentId != parentId) {
+ subPaths.add(ReconUtils.constructObjectPathWithPrefix(volumeID,
+ bucketID, currentId));
+ }
+ for (Long childId : summary.getChildDir()) {
+ if (visited.add(childId)) {
+ stack.push(childId);
+ } else if (!cycleLogged) {
+ reconNamespaceSummaryManager.recordNSSummaryInvalidTreeDetection();
+ log.warn("Detected a repeated reference to object {} while walking "
+
+ "the NSSummary tree under object {} (volume {}, bucket {}); the
" +
+ "NSSummary data may be corrupted.", childId, parentId, volumeID,
+ bucketID);
+ cycleLogged = true;
+ }
}
}
}
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/EntityHandler.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/EntityHandler.java
index dfb087a2234..ac9c78eed45 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/EntityHandler.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/EntityHandler.java
@@ -20,7 +20,12 @@
import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
import java.io.IOException;
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.HashSet;
+import java.util.Iterator;
import java.util.Set;
+import java.util.function.Consumer;
import org.apache.hadoop.hdds.scm.server.OzoneStorageContainerManager;
import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
@@ -33,12 +38,16 @@
import org.apache.hadoop.ozone.recon.api.types.QuotaUsageResponse;
import org.apache.hadoop.ozone.recon.recovery.ReconOMMetadataManager;
import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Class for handling all entity types.
*/
public abstract class EntityHandler {
+ private static final Logger LOG =
LoggerFactory.getLogger(EntityHandler.class);
+
private final ReconNamespaceSummaryManager reconNamespaceSummaryManager;
private final ReconOMMetadataManager omMetadataManager;
@@ -205,32 +214,111 @@ public static EntityHandler getEntityHandler(
* @throws IOException ioEx
*/
protected int[] getTotalFileSizeDist(long objectId) throws IOException {
- NSSummary nsSummary = reconNamespaceSummaryManager.getNSSummary(objectId);
- if (nsSummary == null) {
- return new int[ReconConstants.NUM_OF_FILE_SIZE_BINS];
- }
- int[] res = nsSummary.getFileSizeBucket();
- for (long childId: nsSummary.getChildDir()) {
- int[] subDirFileSizeDist = getTotalFileSizeDist(childId);
+ int[] res = new int[ReconConstants.NUM_OF_FILE_SIZE_BINS];
+ walkNSSummaryTree(objectId, nsSummary -> {
+ int[] fileSizeBucket = nsSummary.getFileSizeBucket();
for (int i = 0; i < ReconConstants.NUM_OF_FILE_SIZE_BINS; ++i) {
- res[i] += subDirFileSizeDist[i];
+ res[i] += fileSizeBucket[i];
}
- }
+ });
return res;
}
protected int getTotalDirCount(long objectId) throws IOException {
- NSSummary nsSummary =
- getReconNamespaceSummaryManager().getNSSummary(objectId);
- if (nsSummary == null) {
+ return walkNSSummaryTree(objectId, null);
+ }
+
+ /**
+ * Walk the NSSummary tree without retaining every object ID. Each stack
frame
+ * keeps one child iterator, so live memory is proportional to tree depth
+ * instead of the number of directories. The ancestor set prevents a corrupt
+ * child reference from walking back into the active path.
+ *
+ * @param objectId root object ID
+ * @param summaryConsumer optional consumer for each available NSSummary
+ * @return number of reachable subdirectory references
+ * @throws IOException if an NSSummary cannot be read
+ */
+ private int walkNSSummaryTree(long objectId,
+ Consumer<NSSummary> summaryConsumer) throws IOException {
+ NSSummary rootSummary =
reconNamespaceSummaryManager.getNSSummary(objectId);
+ if (rootSummary == null) {
return 0;
}
- Set<Long> subdirs = nsSummary.getChildDir();
- int totalCnt = subdirs.size();
- for (long subdir : subdirs) {
- totalCnt += getTotalDirCount(subdir);
+ if (summaryConsumer != null) {
+ summaryConsumer.accept(rootSummary);
+ }
+
+ Set<Long> ancestors = new HashSet<>();
+ Deque<NSSummaryTraversalFrame> stack = new ArrayDeque<>();
+ ancestors.add(objectId);
+ stack.push(new NSSummaryTraversalFrame(objectId,
+ rootSummary.getChildDir().iterator()));
+ int totalDirCount = 0;
+ boolean cycleLogged = false;
+ while (!stack.isEmpty()) {
+ NSSummaryTraversalFrame frame = stack.peek();
+ if (!frame.getChildIterator().hasNext()) {
+ stack.pop();
+ ancestors.remove(frame.getObjectId());
+ continue;
+ }
+
+ long childId = frame.getChildIterator().next();
+ if (ancestors.contains(childId)) {
+ if (!cycleLogged) {
+ logNSSummaryCycle(childId);
+ cycleLogged = true;
+ }
+ continue;
+ }
+
+ totalDirCount++;
+ NSSummary childSummary =
+ reconNamespaceSummaryManager.getNSSummary(childId);
+ if (childSummary == null) {
+ continue;
+ }
+ if (summaryConsumer != null) {
+ summaryConsumer.accept(childSummary);
+ }
+ ancestors.add(childId);
+ stack.push(new NSSummaryTraversalFrame(childId,
+ childSummary.getChildDir().iterator()));
+ }
+ return totalDirCount;
+ }
+
+ /**
+ * Warn that the NSSummary tree contains a self or ancestor loop. The walk
+ * skips the cyclic edge so the request still completes and operators can see
+ * that the persisted NSSummary data may be corrupted. Callers invoke this at
+ * most once per walk.
+ */
+ private void logNSSummaryCycle(long objectId) {
+ reconNamespaceSummaryManager.recordNSSummaryInvalidTreeDetection();
+ LOG.warn("Detected a cycle through object {} while walking the " +
+ "NSSummary tree under {}; skipping the cyclic reference. The " +
+ "NSSummary data may be corrupted.", objectId, getNormalizedPath());
+ }
+
+ private static final class NSSummaryTraversalFrame {
+ private final long objectId;
+ private final Iterator<Long> childIterator;
+
+ private NSSummaryTraversalFrame(long objectId,
+ Iterator<Long> childIterator) {
+ this.objectId = objectId;
+ this.childIterator = childIterator;
+ }
+
+ private long getObjectId() {
+ return objectId;
+ }
+
+ private Iterator<Long> getChildIterator() {
+ return childIterator;
}
- return totalCnt;
}
/**
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/metrics/NSSummaryMetrics.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/metrics/NSSummaryMetrics.java
new file mode 100644
index 00000000000..4c35774402c
--- /dev/null
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/metrics/NSSummaryMetrics.java
@@ -0,0 +1,91 @@
+/*
+ * 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
+ *
+ * http://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.hadoop.ozone.recon.metrics;
+
+import com.google.inject.Singleton;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.annotation.InterfaceAudience;
+import org.apache.hadoop.metrics2.MetricsCollector;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.MetricsRecordBuilder;
+import org.apache.hadoop.metrics2.MetricsSource;
+import org.apache.hadoop.metrics2.annotation.Metrics;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.apache.hadoop.metrics2.lib.Interns;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.util.Time;
+
+/**
+ * Metrics for NSSummary tree traversals that detect invalid references.
+ */
[email protected]
+@Singleton
+@Metrics(about = "Recon NSSummary Metrics", context = OzoneConsts.OZONE)
+public final class NSSummaryMetrics implements MetricsSource {
+
+ private static final String SOURCE_NAME =
+ NSSummaryMetrics.class.getSimpleName();
+
+ private static final MetricsInfo INVALID_TREE_DETECTION_COUNT = Interns.info(
+ "invalidTreeDetectionCount",
+ "Number of NSSummary tree traversals that detected an invalid reference
since Recon started");
+
+ private static final MetricsInfo LAST_INVALID_TREE_DETECTION_MILLIS =
+ Interns.info("lastInvalidTreeDetectionMillis",
+ "Epoch time in milliseconds of the last NSSummary tree traversal
that detected an invalid reference");
+
+ private final AtomicLong invalidTreeDetectionCount = new AtomicLong();
+ private final AtomicLong lastInvalidTreeDetectionMillis =
+ new AtomicLong();
+
+ public void register() {
+ DefaultMetricsSystem.instance().register(
+ SOURCE_NAME, "Recon NSSummary Metrics", this);
+ }
+
+ public void unregister() {
+ DefaultMetricsSystem.instance().unregisterSource(SOURCE_NAME);
+ }
+
+ public void recordInvalidTreeDetection() {
+ recordInvalidTreeDetection(Time.now());
+ }
+
+ void recordInvalidTreeDetection(long detectedMillis) {
+ invalidTreeDetectionCount.incrementAndGet();
+ lastInvalidTreeDetectionMillis.accumulateAndGet(
+ detectedMillis, Math::max);
+ }
+
+ public long getInvalidTreeDetectionCount() {
+ return invalidTreeDetectionCount.get();
+ }
+
+ public long getLastInvalidTreeDetectionMillis() {
+ return lastInvalidTreeDetectionMillis.get();
+ }
+
+ @Override
+ public void getMetrics(MetricsCollector collector, boolean all) {
+ MetricsRecordBuilder builder = collector.addRecord(SOURCE_NAME);
+ builder.addCounter(INVALID_TREE_DETECTION_COUNT,
+ getInvalidTreeDetectionCount());
+ builder.addGauge(LAST_INVALID_TREE_DETECTION_MILLIS,
+ getLastInvalidTreeDetectionMillis());
+ }
+}
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconNamespaceSummaryManager.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconNamespaceSummaryManager.java
index 0c59f0921b4..b40867c4ad1 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconNamespaceSummaryManager.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconNamespaceSummaryManager.java
@@ -49,6 +49,12 @@ void batchStoreNSSummaries(BatchOperation batch, long
objectId,
NSSummary getNSSummary(long objectId) throws IOException;
+ /**
+ * Record that a read-side traversal detected at least one invalid or
+ * repeated child reference in the NSSummary tree.
+ */
+ void recordNSSummaryInvalidTreeDetection();
+
void commitBatchOperation(RDBBatchOperation rdbBatchOperation)
throws IOException;
}
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconNamespaceSummaryManagerImpl.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconNamespaceSummaryManagerImpl.java
index 0287859399d..8525ec3f714 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconNamespaceSummaryManagerImpl.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconNamespaceSummaryManagerImpl.java
@@ -26,6 +26,7 @@
import org.apache.hadoop.hdds.utils.db.RDBBatchOperation;
import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+import org.apache.hadoop.ozone.recon.metrics.NSSummaryMetrics;
import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
import org.apache.hadoop.ozone.recon.tasks.NSSummaryTask;
@@ -38,23 +39,28 @@ public class ReconNamespaceSummaryManagerImpl
private Table<Long, NSSummary> nsSummaryTable;
private DBStore namespaceDbStore;
private NSSummaryTask nsSummaryTask;
+ private final NSSummaryMetrics nsSummaryMetrics;
@Inject
- public ReconNamespaceSummaryManagerImpl(ReconDBProvider reconDBProvider,
NSSummaryTask nsSummaryTask)
- throws IOException {
- this(reconDBProvider.getDbStore(), nsSummaryTask);
+ public ReconNamespaceSummaryManagerImpl(ReconDBProvider reconDBProvider,
+ NSSummaryTask nsSummaryTask, NSSummaryMetrics nsSummaryMetrics)
+ throws IOException {
+ this(reconDBProvider.getDbStore(), nsSummaryTask, nsSummaryMetrics);
}
- private ReconNamespaceSummaryManagerImpl(DBStore dbStore, NSSummaryTask
nsSummaryTask)
+ private ReconNamespaceSummaryManagerImpl(DBStore dbStore,
+ NSSummaryTask nsSummaryTask, NSSummaryMetrics nsSummaryMetrics)
throws IOException {
namespaceDbStore = dbStore;
this.nsSummaryTable = NAMESPACE_SUMMARY.getTable(namespaceDbStore);
this.nsSummaryTask = nsSummaryTask;
+ this.nsSummaryMetrics = nsSummaryMetrics;
}
@Override
public ReconNamespaceSummaryManager getStagedNsSummaryManager(DBStore
dbStore) throws IOException {
- return new ReconNamespaceSummaryManagerImpl(dbStore, nsSummaryTask);
+ return new ReconNamespaceSummaryManagerImpl(
+ dbStore, nsSummaryTask, nsSummaryMetrics);
}
@Override
@@ -97,6 +103,11 @@ public NSSummary getNSSummary(long objectId) throws
IOException {
return nsSummaryTable.get(objectId);
}
+ @Override
+ public void recordNSSummaryInvalidTreeDetection() {
+ nsSummaryMetrics.recordInvalidTreeDetection();
+ }
+
@Override
public void commitBatchOperation(RDBBatchOperation rdbBatchOperation)
throws IOException {
diff --git
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/TestReconUtils.java
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/TestReconUtils.java
index 6f613894dd8..8b061ad7403 100644
---
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/TestReconUtils.java
+++
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/TestReconUtils.java
@@ -19,12 +19,14 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.hadoop.ozone.recon.ReconUtils.createTarFile;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.File;
@@ -36,6 +38,10 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.RandomUtils;
@@ -45,7 +51,10 @@
import org.apache.hadoop.hdds.scm.container.ContainerInfo;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.hdfs.web.URLConnectionFactory;
+import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.io.TempDir;
/**
@@ -175,6 +184,37 @@ public void testNextClosestPowerIndexOfTwo() {
}
}
+ @Test
+ @Timeout(30)
+ public void testGatherSubPathsToleratesCyclicTree() throws IOException {
+ // Corrupted NSSummary tree: 1 -> {2, 3}, 2 -> 1 (back edge), 3 -> 3 (self
+ // loop). gatherSubPaths must terminate instead of overflowing the stack,
+ // and must list each reachable child directory once.
+ ReconNamespaceSummaryManager nsSummaryManager =
+ mock(ReconNamespaceSummaryManager.class);
+ when(nsSummaryManager.getNSSummary(1L))
+ .thenReturn(nsSummaryWithChildren(2L, 3L));
+ when(nsSummaryManager.getNSSummary(2L))
+ .thenReturn(nsSummaryWithChildren(1L));
+ when(nsSummaryManager.getNSSummary(3L))
+ .thenReturn(nsSummaryWithChildren(3L));
+
+ List<String> subPaths = new ArrayList<>();
+ ReconUtils.gatherSubPaths(1L, subPaths, 100L, 200L, nsSummaryManager);
+
+ // Child directories reachable from parent 1 are exactly {2, 3}, each
emitted
+ // once. The traversal root (1) must be excluded and no child dropped, so
+ // assert the exact "/volumeId/bucketId/objectId" subpaths (in any order).
+ assertThat(subPaths).containsExactlyInAnyOrder("/100/200/2", "/100/200/3");
+ verify(nsSummaryManager).recordNSSummaryInvalidTreeDetection();
+ }
+
+ private static NSSummary nsSummaryWithChildren(Long... childIds) {
+ NSSummary nsSummary = new NSSummary();
+ nsSummary.setChildDir(new HashSet<>(Arrays.asList(childIds)));
+ return nsSummary;
+ }
+
static void assertNextClosestPowerIndexOfTwo(long n) {
final int expected = oldNextClosestPowerIndexOfTwoFixed(n);
final int computed = ReconUtils.nextClosestPowerIndexOfTwo(n);
diff --git
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/handlers/TestEntityHandlerCycleGuard.java
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/handlers/TestEntityHandlerCycleGuard.java
new file mode 100644
index 00000000000..69dd9e60a0a
--- /dev/null
+++
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/handlers/TestEntityHandlerCycleGuard.java
@@ -0,0 +1,196 @@
+/*
+ * 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
+ *
+ * http://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.hadoop.ozone.recon.api.handlers;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.util.AbstractSet;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.hdds.scm.server.OzoneStorageContainerManager;
+import org.apache.hadoop.ozone.recon.ReconConstants;
+import org.apache.hadoop.ozone.recon.api.types.DUResponse;
+import org.apache.hadoop.ozone.recon.api.types.FileSizeDistributionResponse;
+import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+import org.apache.hadoop.ozone.recon.api.types.NamespaceSummaryResponse;
+import org.apache.hadoop.ozone.recon.api.types.QuotaUsageResponse;
+import org.apache.hadoop.ozone.recon.recovery.ReconOMMetadataManager;
+import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+/**
+ * Tests that the NSSummary tree walks in {@link EntityHandler} tolerate a
+ * corrupted (self-referencing / cyclic) tree instead of crashing Recon with a
+ * {@link StackOverflowError}.
+ */
+public class TestEntityHandlerCycleGuard {
+
+ private final ReconNamespaceSummaryManager nsSummaryManager =
+ mock(ReconNamespaceSummaryManager.class);
+
+ private EntityHandler newHandler() {
+ ReconOMMetadataManager omMetadataManager =
+ mock(ReconOMMetadataManager.class);
+ OzoneStorageContainerManager reconSCM =
+ mock(OzoneStorageContainerManager.class);
+ return new EntityHandler(nsSummaryManager, omMetadataManager, reconSCM,
+ null, "/") {
+ @Override
+ public NamespaceSummaryResponse getSummaryResponse() {
+ return null;
+ }
+
+ @Override
+ public DUResponse getDuResponse(boolean listFile, boolean withReplica,
+ boolean sort) {
+ return null;
+ }
+
+ @Override
+ public QuotaUsageResponse getQuotaResponse() {
+ return null;
+ }
+
+ @Override
+ public FileSizeDistributionResponse getDistResponse() {
+ return null;
+ }
+ };
+ }
+
+ private NSSummary nsSummary(int numFiles, long size, Set<Long> children) {
+ int[] bucket = new int[ReconConstants.NUM_OF_FILE_SIZE_BINS];
+ bucket[0] = numFiles;
+ return new NSSummary(numFiles, size, size, bucket, children, "dir", 0);
+ }
+
+ @Test
+ @Timeout(30)
+ public void testSelfReferencingDirDoesNotOverflow() throws IOException {
+ // Directory 1 lists itself as its own child: the corruption that would
+ // otherwise recurse forever. Both walks must terminate.
+ when(nsSummaryManager.getNSSummary(1L))
+ .thenReturn(nsSummary(2, 100L, newSet(1L)));
+
+ EntityHandler handler = newHandler();
+ assertEquals(0, handler.getTotalDirCount(1L));
+ int[] dist = handler.getTotalFileSizeDist(1L);
+ assertEquals(2, dist[0]);
+ verify(nsSummaryManager, times(2))
+ .recordNSSummaryInvalidTreeDetection();
+ }
+
+ @Test
+ @Timeout(30)
+ public void testCyclicTreeCountsEachDirOnce() throws IOException {
+ // 1 -> 2 -> 3 -> 1 (back edge to the root) and 2 -> 2 (self loop).
+ when(nsSummaryManager.getNSSummary(1L))
+ .thenReturn(nsSummary(1, 10L, newSet(2L)));
+ when(nsSummaryManager.getNSSummary(2L))
+ .thenReturn(nsSummary(1, 20L, newSet(3L, 2L)));
+ when(nsSummaryManager.getNSSummary(3L))
+ .thenReturn(nsSummary(1, 30L, newSet(1L)));
+
+ EntityHandler handler = newHandler();
+ // Reachable directories other than the root object 1 are {2, 3}.
+ assertEquals(2, handler.getTotalDirCount(1L));
+ // Each distinct directory contributes its file count exactly once.
+ assertEquals(3, handler.getTotalFileSizeDist(1L)[0]);
+ verify(nsSummaryManager, times(2))
+ .recordNSSummaryInvalidTreeDetection();
+ }
+
+ @Test
+ @Timeout(30)
+ public void testCleanTreeUnaffected() throws IOException {
+ // 1 -> {2, 3}, 2 -> {4}. No cycles: counts match the pre-fix behavior.
+ when(nsSummaryManager.getNSSummary(1L))
+ .thenReturn(nsSummary(1, 10L, newSet(2L, 3L)));
+ when(nsSummaryManager.getNSSummary(2L))
+ .thenReturn(nsSummary(1, 20L, newSet(4L)));
+ when(nsSummaryManager.getNSSummary(3L))
+ .thenReturn(nsSummary(1, 30L, newSet()));
+ when(nsSummaryManager.getNSSummary(4L))
+ .thenReturn(nsSummary(1, 40L, newSet()));
+
+ EntityHandler handler = newHandler();
+ assertEquals(3, handler.getTotalDirCount(1L));
+ assertThat(handler.getTotalFileSizeDist(1L)[0]).isEqualTo(4);
+ }
+
+ @Test
+ @Timeout(30)
+ public void testWideTreeIsTraversedIncrementally() throws IOException {
+ int childCount = 10_000;
+ AtomicInteger generatedChildren = new AtomicInteger();
+ AtomicInteger childLookups = new AtomicInteger();
+ Set<Long> children = new AbstractSet<Long>() {
+ @Override
+ public Iterator<Long> iterator() {
+ return new Iterator<Long>() {
+ private long nextId = 2;
+
+ @Override
+ public boolean hasNext() {
+ return nextId <= childCount + 1L;
+ }
+
+ @Override
+ public Long next() {
+ assertEquals(childLookups.get(), generatedChildren.get(),
+ "Tree walk buffered child IDs before reading their NSSummary");
+ generatedChildren.incrementAndGet();
+ return nextId++;
+ }
+ };
+ }
+
+ @Override
+ public int size() {
+ return childCount;
+ }
+ };
+
+ when(nsSummaryManager.getNSSummary(anyLong())).thenAnswer(invocation -> {
+ long objectId = invocation.getArgument(0);
+ if (objectId == 1L) {
+ return nsSummary(0, 0L, children);
+ }
+ childLookups.incrementAndGet();
+ return null;
+ });
+
+ assertEquals(childCount, newHandler().getTotalDirCount(1L));
+ assertEquals(childCount, childLookups.get());
+ }
+
+ private static Set<Long> newSet(Long... ids) {
+ return new HashSet<>(Arrays.asList(ids));
+ }
+}
diff --git
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/metrics/TestNSSummaryMetrics.java
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/metrics/TestNSSummaryMetrics.java
new file mode 100644
index 00000000000..b3cb7360031
--- /dev/null
+++
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/metrics/TestNSSummaryMetrics.java
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * http://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.hadoop.ozone.recon.metrics;
+
+import static org.apache.ozone.test.MetricsAsserts.getLongCounter;
+import static org.apache.ozone.test.MetricsAsserts.getLongGauge;
+import static org.apache.ozone.test.MetricsAsserts.getMetrics;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.hadoop.metrics2.MetricsRecordBuilder;
+import org.apache.hadoop.util.Time;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for {@link NSSummaryMetrics}.
+ */
+class TestNSSummaryMetrics {
+
+ @Test
+ void testRecordInvalidTreeDetection() {
+ NSSummaryMetrics metrics = new NSSummaryMetrics();
+ long before = Time.now();
+
+ metrics.recordInvalidTreeDetection();
+ metrics.recordInvalidTreeDetection();
+
+ MetricsRecordBuilder builder = getMetrics(metrics);
+ assertThat(getLongCounter("invalidTreeDetectionCount", builder))
+ .isEqualTo(2L);
+ assertThat(getLongGauge("lastInvalidTreeDetectionMillis", builder))
+ .isBetween(before, Time.now());
+ }
+
+ @Test
+ void testLastDetectedMillisDoesNotRegress() {
+ NSSummaryMetrics metrics = new NSSummaryMetrics();
+
+ metrics.recordInvalidTreeDetection(200L);
+ metrics.recordInvalidTreeDetection(100L);
+
+ assertThat(metrics.getInvalidTreeDetectionCount()).isEqualTo(2L);
+ assertThat(metrics.getLastInvalidTreeDetectionMillis())
+ .isEqualTo(200L);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
