devmadhuu commented on code in PR #10723:
URL: https://github.com/apache/ozone/pull/10723#discussion_r3756150337
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/EntityHandler.java:
##########
@@ -205,32 +214,110 @@ 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) {
+ LOG.warn("Detected a cycle through object {} while walking the " +
Review Comment:
A part from `WARN` log, its better to introduce a counter +
`lastDetectedMillis`, so that matching TS and `WARN` log will have path, which
can be a candidate for investigation, if issue was at OM or Recon derive
`NSSummary` tree path somehow got corrupted. Ideally we should have a scanner,
but that will be too much to ask at this point. Metrics is good starting point
along with WARN logs including path.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
