ArafatKhan2198 commented on code in PR #10673:
URL: https://github.com/apache/ozone/pull/10673#discussion_r3542059210


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/export/ContainerExportManager.java:
##########
@@ -0,0 +1,499 @@
+/*
+ * 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.hdds.scm.container.export;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState;
+import org.apache.hadoop.hdds.scm.container.ContainerHealthState;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.Archiver;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Manages asynchronous container ID export jobs on SCM leader.
+ */
+public class ContainerExportManager {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ContainerExportManager.class);
+
+  private static final DateTimeFormatter METADATA_TIMESTAMP_FORMAT =
+      
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'").withZone(ZoneOffset.UTC);
+  private static final DateTimeFormatter FILENAME_TIMESTAMP_FORMAT =
+      
DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'").withZone(ZoneOffset.UTC);
+
+  private static final String EXPORT_SUBDIR = "exports";
+  private static final int DEFAULT_SHARD_SIZE = 500_000;
+  private static final int DEFAULT_PAGE_SIZE = 100_000;
+  private static final int MAX_SHARD_SIZE = 5_000_000;
+  private static final int MAX_PAGE_SIZE = 1_000_000;
+
+  private final Map<String, ExportJob> jobTracker = new ConcurrentHashMap<>();

Review Comment:
   **`jobTracker` grows unbounded (memory leak).** 
   Jobs are added on submit and never removed (the retention is even asserted 
by `testRecentTerminalJobRetainedAcrossSubmits`). `runningTasks` is cleaned in 
`finally`, but `jobTracker` is not. On a long-lived SCM with repeated exports 
this accumulates `ExportJob` objects forever. 



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/export/ContainerExportManager.java:
##########
@@ -0,0 +1,499 @@
+/*
+ * 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.hdds.scm.container.export;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState;
+import org.apache.hadoop.hdds.scm.container.ContainerHealthState;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.Archiver;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Manages asynchronous container ID export jobs on SCM leader.
+ */
+public class ContainerExportManager {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ContainerExportManager.class);
+
+  private static final DateTimeFormatter METADATA_TIMESTAMP_FORMAT =
+      
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'").withZone(ZoneOffset.UTC);
+  private static final DateTimeFormatter FILENAME_TIMESTAMP_FORMAT =
+      
DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'").withZone(ZoneOffset.UTC);
+
+  private static final String EXPORT_SUBDIR = "exports";
+  private static final int DEFAULT_SHARD_SIZE = 500_000;
+  private static final int DEFAULT_PAGE_SIZE = 100_000;
+  private static final int MAX_SHARD_SIZE = 5_000_000;
+  private static final int MAX_PAGE_SIZE = 1_000_000;
+
+  private final Map<String, ExportJob> jobTracker = new ConcurrentHashMap<>();
+  private final ExecutorService workerPool;
+  private final Map<String, Future<?>> runningTasks = new 
ConcurrentHashMap<>();
+  private final ContainerManager containerManager;
+  private final String exportDirectory;
+  private final int defaultShardSize;
+  private final int defaultPageSize;
+  private final Object submitLock = new Object();
+
+  public ContainerExportManager(ContainerManager containerManager,
+                                OzoneConfiguration conf) {
+    this(containerManager, resolveExportDirectory(conf),
+        DEFAULT_SHARD_SIZE, DEFAULT_PAGE_SIZE);
+  }
+
+  ContainerExportManager(ContainerManager containerManager,
+      String exportDirectory, int defaultShardSize, int defaultPageSize) {
+    this.containerManager = containerManager;
+    this.exportDirectory = exportDirectory;
+    this.defaultShardSize = defaultShardSize;
+    this.defaultPageSize = defaultPageSize;
+    this.workerPool = Executors.newSingleThreadExecutor(r -> {
+      Thread t = new Thread(r, "ContainerExportWorker");

Review Comment:
   This feature keeps the whole job the progress info *and* the output TAR file 
only on the one SCM machine that happened to handle the "start" command. It's 
in that machine's memory and on that machine's local disk.
   
   Problem: the admin CLI doesn't guarantee it talks to the same SCM every 
time. So you can:
   
   - run `export` → it starts on SCM-A,
   - run `export status` → the request gets routed to SCM-B → SCM-B has never 
heard of this job → **"Export job not found."**
   
   Same thing if leadership flips mid-run: the job keeps churning on the old 
node while you're now pointed at a new one. And despite the description saying 
"runs on the SCM leader," nothing in the code actually checks "am I the 
leader?" it only checks "are you an admin?" So a follower could run it too.
   
   Fix: either force these commands to only run on the leader (and reject on 
followers), or make status reliably reach the node that owns the job. Ask the 
author what they intended.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/export/ContainerExportManager.java:
##########
@@ -0,0 +1,499 @@
+/*
+ * 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.hdds.scm.container.export;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState;
+import org.apache.hadoop.hdds.scm.container.ContainerHealthState;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.Archiver;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Manages asynchronous container ID export jobs on SCM leader.
+ */
+public class ContainerExportManager {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ContainerExportManager.class);
+
+  private static final DateTimeFormatter METADATA_TIMESTAMP_FORMAT =
+      
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'").withZone(ZoneOffset.UTC);
+  private static final DateTimeFormatter FILENAME_TIMESTAMP_FORMAT =
+      
DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'").withZone(ZoneOffset.UTC);
+
+  private static final String EXPORT_SUBDIR = "exports";
+  private static final int DEFAULT_SHARD_SIZE = 500_000;
+  private static final int DEFAULT_PAGE_SIZE = 100_000;
+  private static final int MAX_SHARD_SIZE = 5_000_000;
+  private static final int MAX_PAGE_SIZE = 1_000_000;
+
+  private final Map<String, ExportJob> jobTracker = new ConcurrentHashMap<>();
+  private final ExecutorService workerPool;
+  private final Map<String, Future<?>> runningTasks = new 
ConcurrentHashMap<>();
+  private final ContainerManager containerManager;
+  private final String exportDirectory;
+  private final int defaultShardSize;
+  private final int defaultPageSize;
+  private final Object submitLock = new Object();
+
+  public ContainerExportManager(ContainerManager containerManager,
+                                OzoneConfiguration conf) {
+    this(containerManager, resolveExportDirectory(conf),
+        DEFAULT_SHARD_SIZE, DEFAULT_PAGE_SIZE);
+  }
+
+  ContainerExportManager(ContainerManager containerManager,
+      String exportDirectory, int defaultShardSize, int defaultPageSize) {
+    this.containerManager = containerManager;
+    this.exportDirectory = exportDirectory;
+    this.defaultShardSize = defaultShardSize;
+    this.defaultPageSize = defaultPageSize;
+    this.workerPool = Executors.newSingleThreadExecutor(r -> {
+      Thread t = new Thread(r, "ContainerExportWorker");
+      t.setDaemon(true);
+      return t;
+    });
+
+    try {
+      Files.createDirectories(Paths.get(exportDirectory));
+    } catch (IOException e) {
+      LOG.error("Failed to create export directory: {}", exportDirectory, e);
+    }
+    LOG.info("ContainerExportManager initialized (dir={}, defaultShardSize={}, 
defaultPageSize={})",
+        exportDirectory, defaultShardSize, defaultPageSize);
+  }
+
+  private static String resolveExportDirectory(OzoneConfiguration conf) {
+    File scmDbDir = ServerUtils.getScmDbDir(conf);
+    return new File(scmDbDir, EXPORT_SUBDIR).getAbsolutePath();
+  }
+
+  /**
+   * Submit a container ID export job.
+   *
+   * @param start optional inclusive start container ID (0 for beginning)
+   * @param lifeCycleState optional lifecycle filter
+   * @param healthState optional health filter
+   * @param maxRows optional row limit (0 = unlimited)
+   * @param pageSize IDs fetched per SCM read (0 = manager default)
+   * @param shardSize IDs per TAR entry (0 = manager default)
+   * @return job id
+   */
+  public String submitJob(ContainerID start, LifeCycleState lifeCycleState,
+      ContainerHealthState healthState, long maxRows, int pageSize, int 
shardSize) {
+    if (lifeCycleState == null && healthState == null) {
+      throw new IllegalArgumentException("At least one of healthState or 
lifecycleState filter is required.");
+    }
+    validateRequest(start, maxRows, pageSize, shardSize);
+    int resolvedPageSize = pageSize > 0 ? pageSize : defaultPageSize;
+    int resolvedShardSize = shardSize > 0 ? shardSize : defaultShardSize;
+
+    String jobId = UUID.randomUUID().toString();
+    String scope = buildScope(lifeCycleState, healthState);
+    Instant now = Instant.now();
+    String metadataTimestamp = METADATA_TIMESTAMP_FORMAT.format(now);
+    String fileTimestamp = FILENAME_TIMESTAMP_FORMAT.format(now);
+    String tarFileName = String.format("container-ids-%s-%s-%s.tar", scope, 
fileTimestamp, jobId);
+    String tarPath = exportDirectory + File.separator + tarFileName;
+
+    ExportJob job = new ExportJob(jobId, scope, metadataTimestamp, tarPath,
+        start, lifeCycleState, healthState, maxRows, resolvedPageSize, 
resolvedShardSize);
+
+    synchronized (submitLock) {
+      boolean exportInProgress = jobTracker.values().stream()
+          .anyMatch(j -> j.getState() == ContainerExportStatus.State.RUNNING);
+      if (exportInProgress) {
+        throw new IllegalStateException("Another container ID export is 
already running.");
+      }
+      jobTracker.put(jobId, job);
+    }
+
+    Future<?> future = workerPool.submit(() -> executeExport(job));
+    runningTasks.put(jobId, future);
+    LOG.info("Submitted container ID export job {} (scope={}, start={}, 
maxRows={}, pageSize={}, shardSize={})",
+        jobId, scope, start, maxRows, resolvedPageSize, resolvedShardSize);
+    return jobId;
+  }
+
+  private static void validateRequest(ContainerID start, long maxRows, int 
pageSize, int shardSize) {
+    if (start != null && start.getProtobuf().getId() < 0) {
+      throw new IllegalArgumentException("start container ID must be 
non-negative.");
+    }
+    if (maxRows < 0) {
+      throw new IllegalArgumentException("maxRows must be non-negative.");
+    }
+    if (pageSize < 0 || pageSize > MAX_PAGE_SIZE) {
+      throw new IllegalArgumentException("pageSize must be between 0 and " + 
MAX_PAGE_SIZE + ".");
+    }
+    if (shardSize < 0 || shardSize > MAX_SHARD_SIZE) {
+      throw new IllegalArgumentException("shardSize must be between 0 and " + 
MAX_SHARD_SIZE + ".");
+    }
+  }
+
+  public ContainerExportStatus getJobStatus(String jobId) {
+    ExportJob job = jobTracker.get(jobId);
+    if (job == null) {
+      return null;
+    }
+    return job.toStatus();
+  }
+
+  public void shutdown() {
+    LOG.info("Shutting down ContainerExportManager");
+    workerPool.shutdownNow();
+    try {
+      workerPool.awaitTermination(30, TimeUnit.SECONDS);
+    } catch (InterruptedException e) {
+      LOG.warn("Timeout waiting for export worker shutdown", e);
+      Thread.currentThread().interrupt();
+    }
+    runningTasks.clear();
+  }
+
+  Map<String, ExportJob> getJobTracker() {
+    return jobTracker;
+  }
+
+  private void executeExport(ExportJob job) {
+    Path jobDir = Paths.get(exportDirectory, job.getJobId());
+    Path workDir = jobDir.resolve("work");
+    File tarFile = new File(job.getTarPath());
+    long startTimeMs = System.currentTimeMillis();
+    job.setStartTimeMs(startTimeMs);
+
+    try {
+      Files.createDirectories(workDir);
+      job.setState(ContainerExportStatus.State.RUNNING);
+
+      ContainerID cursor = job.getStartContainerId();
+      int pageSize = job.getPageSize();
+      int shardSize = job.getShardSize();
+      int fileIndex = 1;
+      long totalRows = 0;
+      long recordsInCurrentFile = 0;
+      BufferedWriter writer = null;
+      Path currentShardPath = null;
+      // Pre-allocated buffer: ~12 chars per ID (up to 20 digits + newline) 
per page.
+      StringBuilder buf = new StringBuilder(pageSize * 12);
+
+      try {
+        while (true) {
+          if (Thread.currentThread().isInterrupted()) {
+            throw new InterruptedException("Job cancelled");
+          }
+
+          int fetchCount = pageSize;
+          if (job.getMaxRows() > 0) {
+            long remaining = job.getMaxRows() - totalRows;
+            if (remaining <= 0) {
+              break;
+            }
+            fetchCount = (int) Math.min(fetchCount, remaining);
+          }
+
+          List<ContainerID> page = containerManager.getContainerIDs(
+              cursor, fetchCount, job.getLifeCycleState(), 
job.getHealthState());

Review Comment:
   Let me know if I am missing something!
   
   Each container carries a `healthState` field. It's **not computed when you 
run the export** it's just whatever ReplicationManager (the background 
component that checks replica health) last wrote onto it. It defaults to 
`HEALTHY` until RM evaluates it.
   
   So `--health-state MISSING` gives you the containers RM has *already 
flagged* as missing. If RM hasn't gotten around to a container yet, or is 
running behind, that container still reads `HEALTHY` and won't show up even if 
it's actually missing right now. The export can therefore under-report. Fine 
for a debug tool, but this caveat ("reflects RM's last-known health, may be 
stale") should be written down so nobody treats it as a real-time truth.



##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/export/TestContainerExportManager.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * 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.hdds.scm.container.export;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.hdds.scm.container.ContainerHealthState;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.hdds.utils.Archiver;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Tests for {@link ContainerExportManager}.
+ */
+public class TestContainerExportManager {

Review Comment:
   **Missing unit coverage for the new `ContainerStateMap` filtering logic**
   
   `TestContainerExportManager` mocks `ContainerManager` 
(`when(containerManager.getContainerIDs(...)).thenReturn(...)`), so it only 
exercises the export plumbing shard writing, TAR packing, concurrent-job 
rejection, status reporting. The actual filtering logic added in this PR is 
never run: the test would still pass even if the filter were broken, because 
the canned mock answers bypass it entirely.
   
   The new code in `ContainerStateMap` has three branches that currently have 
no direct coverage:
   
   - **health-only** → `getFilteredContainerIDs` (and the unfiltered path where 
both filters are null)
   - **lifecycle + health** → `getContainerIDsFromLifecycleIndex`
   - **lifecycle-only** → delegates to the existing `getContainerIDs`
   
   `TestContainerStateManager#testGetContainerIDs` was only updated to pass 
`ContainerHealthState.HEALTHY` as an extra arg so it compiles it doesn't assert 
the filter actually selects the right containers.
   
   Could we add a test (e.g. in `TestContainerStateMap`) that inserts real 
`ContainerInfo`s with mixed lifecycle and health states and asserts, for each 
of the three branches:
   
   1. only the matching IDs are returned, in ascending order;
   2. pagination is correct across pages (cursor = `lastId + 1`, no gaps or 
duplicates, terminates on empty page);
   3. `count == 0` returns an empty list.
   
   This is the core new behavior of the feature, so it's worth covering 
directly rather than only through the mocked export path.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/states/ContainerStateMap.java:
##########
@@ -118,6 +120,30 @@ List<ContainerInfo> getInfos(ContainerID start, int count) 
{
           .collect(Collectors.toList());
     }
 
+    List<ContainerID> getFilteredContainerIDs(ContainerID start, int count,
+        LifeCycleState lifeCycleState, ContainerHealthState healthState) {
+      Objects.requireNonNull(start, "start == null");
+      Preconditions.assertTrue(count >= 0, "count < 0");
+      List<ContainerID> result = new ArrayList<>(Math.min(count, 64));
+      if (count == 0) {
+        return result;
+      }
+      for (ContainerEntry entry : map.tailMap(start).values()) {

Review Comment:
   There are fast lookup indexes for lifecycle state (OPEN/CLOSED/etc.), but 
**not** for health state. So when you filter by health only (no lifecycle), the 
code walks through the entire container set and checks each one. That's why 
your `MISSING` run touched all 473k containers even though only 41k matched.
   
   It's still linear (it doesn't re-scan from zero each page it continues from 
a cursor), and since the job is async, it's acceptable. Just know there's no 
shortcut for health-only queries; big clusters will always do a full pass.
   
   We could add like a comment or something somewhere for this.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/StorageContainerServiceProviderImpl.java:
##########
@@ -187,9 +188,9 @@ private RocksDBCheckpoint getRocksDBCheckpoint(String 
snapshotFileName, File tar
 
   @Override
   public List<ContainerID> getListOfContainerIDs(
-      ContainerID startContainerID, int count, HddsProtos.LifeCycleState state)
+      ContainerID startContainerID, int count, HddsProtos.LifeCycleState 
state, ContainerHealthState healthState)
       throws IOException {
-    return scmClient.getListOfContainerIDs(startContainerID, count, state);
+    return scmClient.getListOfContainerIDs(startContainerID, count, state, 
healthState);
   }
 
   /**

Review Comment:
   Two different "list container IDs" paths got the new `healthState` parameter:
   
   - **The in-process one** (`ContainerManager.getContainerIDs`) the export 
actually calls this, so it's needed.
   - **The network/RPC one** (`getListOfContainerIDs`) this got threaded 
through the protocol, client, server, Recon, and ~100 test mocks... but 
**nobody actually passes a real health value to it.** Export doesn't use the 
RPC (it runs inside SCM), and Recon passes `null`.
   
   So that second chunk looks like dead wiring



-- 
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]

Reply via email to