devmadhuu commented on code in PR #10547:
URL: https://github.com/apache/ozone/pull/10547#discussion_r3528917149
##########
hadoop-ozone/cli-debug/src/main/java/org/apache/hadoop/ozone/debug/datanode/container/analyze/AnalyzeSubcommand.java:
##########
@@ -34,65 +42,134 @@
*/
@Command(
name = "analyze",
- description = "Analyze container consistency between on-disk container " +
- "directories on this DataNode and SCM metadata. Must be run
locally on a DataNode.")
+ description = {
+ "Analyze container consistency between on-disk container directories
on this DataNode and SCM metadata.",
+ "Must be run locally on a DataNode.",
+ "",
+ "Reports:",
+ " Duplicate container directories: same containerID found on more
than one volume.",
+ " Orphan containers (requires --scm-db): present on disk but not
present in SCM metadata.",
+ " Containers marked DELETED in SCM but present on disk (requires
--scm-db).",
+ "",
+ "Each reported occurrence includes container directory path(s), size
and an on-disk metadata status:",
+ " MISSING_METADATA: metadata/{containerId}.container does not exist.",
+ " INVALID_METADATA: metadata file exists but cannot be parsed, or the
containerID in the",
+ " file does not match the directory name.",
+ " VALID: metadata file is present, parses correctly, and its
containerID matches the directory name."
+ })
public class AnalyzeSubcommand extends AbstractSubcommand implements
Callable<Void> {
- @CommandLine.Option(names = {"--count"},
- defaultValue = "20",
- description = "Number of containers to display")
- private int count;
+ @CommandLine.Mixin
+ private ListLimitOptions listOptions;
+
+ @CommandLine.Option(names = {"--scm-db"},
+ description = "Path to an offline scm.db directory, or its parent
metadata directory.")
+ private File scmDb;
@Override
public Void call() throws Exception {
- if (count < 1) {
- throw new IOException("Count must be an integer greater than 0.");
- }
+ validateOptions();
OzoneConfiguration conf = getOzoneConf();
ContainerScanResult scanResult = ContainerDirectoryScanner.scan(conf);
Map<Long, List<ContainerDiskOccurrence>> enrichedDuplicates =
ContainerDirectoryScanner.enrichDuplicates(scanResult.getDuplicates());
- // TODO: SCM metadata lookup from --scm-db when provided.
- // TODO: For each id in scanResult.getSingles().keySet() classified
NOT_IN_SCM or DELETED:
- // enrichOccurrence(id, scanResult.getSingles().get(id)) and report.
- // TODO: For each id in enrichedDuplicates.keySet() classified NOT_IN_SCM
or DELETED:
- // enrichedDuplicates.get(id) is already enriched — just report.
+ if (scmDb != null) {
+ findOrphanAndDeletedButPresentContainers(conf, scanResult,
enrichedDuplicates);
+ } else {
+ out().println("To identify orphan containers (wrt SCM) and containers
that are marked as DELETED in SCM but"
+ + " exist in the datanode's current directory, provide the SCM
database path using the --scm-db option."
+ );
+ }
printDuplicates(enrichedDuplicates);
printVolumeScanErrors(scanResult.getVolumeScanErrors());
return null;
}
- private void printDuplicates(Map<Long, List<ContainerDiskOccurrence>>
duplicates) {
- long totalDuplicateIds = duplicates.size();
- out().printf("Number of containers with duplicate container directories on
this DataNode: %d%n", totalDuplicateIds);
+ /**
+ * Validate CLI options before starting the on-disk DN scan.
+ * {@link ListLimitOptions#getLimit()} is also called from
+ * {@link #printContainerOccurrenceReport(String, Map)}, but validating here
fails fast
+ * before the DN volume scan and SCM DB lookup.
+ */
+ private void validateOptions() {
+ listOptions.getLimit();
+ }
+
+ private void findOrphanAndDeletedButPresentContainers(OzoneConfiguration
conf, ContainerScanResult scanResult,
+ Map<Long, List<ContainerDiskOccurrence>> enrichedDuplicates) throws
IOException {
+ Map<Long, List<ContainerDiskOccurrence>> enrichedOrphanContainers = new
HashMap<>();
+ Map<Long, List<ContainerDiskOccurrence>> enrichedDeletedButPresent = new
HashMap<>();
+
+ try (ScmContainerMetadataReader reader = new
ScmContainerMetadataReader(conf, scmDb)) {
+ Set<Long> containerIds = new HashSet<>(scanResult.getSingles().keySet());
+ containerIds.addAll(enrichedDuplicates.keySet());
+
+ for (long containerId : containerIds) {
+ Optional<ScmContainerMetadataReader.ScmContainerClassification>
classification = reader.classify(containerId);
+ if (!classification.isPresent()) {
+ continue;
+ }
+ List<ContainerDiskOccurrence> occurrences =
enrichedDuplicates.get(containerId);
+ if (occurrences == null) {
+ String path = scanResult.getSingles().get(containerId);
+ occurrences =
Collections.singletonList(ContainerDirectoryScanner.enrichOccurrence(containerId,
path));
+ }
+ if (classification.get() ==
ScmContainerMetadataReader.ScmContainerClassification.NOT_IN_SCM) {
+ enrichedOrphanContainers.put(containerId, occurrences);
+ } else {
+ enrichedDeletedButPresent.put(containerId, occurrences);
+ }
+ }
+ }
+
+ printContainerOccurrenceReport("Number of orphan containers(wrt SCM) on
this DataNode: %d%n",
+ enrichedOrphanContainers);
+ printContainerOccurrenceReport(
+ "Number of containers marked DELETED in SCM but present on disk on
this DataNode: %d%n",
Review Comment:
This output doesn't match with output in your PR description.
##########
hadoop-ozone/cli-debug/src/main/java/org/apache/hadoop/ozone/debug/datanode/container/analyze/AnalyzeSubcommand.java:
##########
@@ -34,65 +42,134 @@
*/
@Command(
name = "analyze",
- description = "Analyze container consistency between on-disk container " +
- "directories on this DataNode and SCM metadata. Must be run
locally on a DataNode.")
+ description = {
+ "Analyze container consistency between on-disk container directories
on this DataNode and SCM metadata.",
+ "Must be run locally on a DataNode.",
+ "",
+ "Reports:",
+ " Duplicate container directories: same containerID found on more
than one volume.",
+ " Orphan containers (requires --scm-db): present on disk but not
present in SCM metadata.",
+ " Containers marked DELETED in SCM but present on disk (requires
--scm-db).",
+ "",
+ "Each reported occurrence includes container directory path(s), size
and an on-disk metadata status:",
+ " MISSING_METADATA: metadata/{containerId}.container does not exist.",
+ " INVALID_METADATA: metadata file exists but cannot be parsed, or the
containerID in the",
+ " file does not match the directory name.",
+ " VALID: metadata file is present, parses correctly, and its
containerID matches the directory name."
+ })
public class AnalyzeSubcommand extends AbstractSubcommand implements
Callable<Void> {
- @CommandLine.Option(names = {"--count"},
- defaultValue = "20",
- description = "Number of containers to display")
- private int count;
+ @CommandLine.Mixin
+ private ListLimitOptions listOptions;
+
+ @CommandLine.Option(names = {"--scm-db"},
+ description = "Path to an offline scm.db directory, or its parent
metadata directory.")
+ private File scmDb;
@Override
public Void call() throws Exception {
- if (count < 1) {
- throw new IOException("Count must be an integer greater than 0.");
- }
+ validateOptions();
Review Comment:
Changing from `IOException` to `IllegalArgumentException` might be okay ,
but just make sure that CLI exit code is what you expect from here in case of
error.
--
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]
