Skip to content

HDDS-15765. Fail fast when --node-id is omitted for OM compact/defrag on an HA cluster#10699

Merged
sarvekshayr merged 4 commits into
apache:masterfrom
chihsuan:HDDS-15765
Jul 15, 2026
Merged

HDDS-15765. Fail fast when --node-id is omitted for OM compact/defrag on an HA cluster#10699
sarvekshayr merged 4 commits into
apache:masterfrom
chihsuan:HDDS-15765

Conversation

@chihsuan

@chihsuan chihsuan commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

On an HA OM cluster, ozone repair om compact and ozone admin om snapshot defrag without --node-id do not fail cleanly; they hang for a long time before eventually timing out.

Root cause: with no --node-id/--service-id, OMNodeDetails.getOMNodeDetailsFromConf(conf, null, null) falls back to the base config key ozone.om.address, which in an HA deployment defaults to 0.0.0.0:9862. That default is the OM server bind address (bind to all interfaces) and is meaningless as a client target. Because it resolves to a non-null (but useless) address, the tool builds a valid-looking OMNodeDetails for 0.0.0.0 and proceeds to the RPC, which then retries in the Hadoop IPC retry loop and blocks.

This PR adds a fail-fast check in both CLI commands: when --node-id is omitted but the config defines an HA service (OmUtils.isServiceIdsDefined(conf)), the command exits immediately and asks the operator to specify --node-id, instead of resolving to 0.0.0.0. Each command reports the error with its own existing convention.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15765

How was this patch tested?

  • New unit tests TestCompactOMDB#testCompactHAWithoutNodeIdFailsFast and TestDefragSubCommand#testDefragHAWithoutNodeIdFailsFast
  • Existing TestCompactOMDB, TestDefragSubCommand, and TestOMNodeDetails still pass, and checkstyle is clean.
  • Manual end-to-end run on the compose/ozone-ha cluster (om1/om2/om3, service.ids=omservice):
Command Before After
ozone repair om compact --cf keyTable (no --node-id) hangs until timeout returns immediately: This is an HA OM cluster; specify --node-id ...
ozone admin om snapshot defrag (no --node-id) hangs until timeout returns immediately: Error: This is an HA OM cluster; specify --node-id ...
ozone repair om compact --cf keyTable --service-id omservice --node-id om3 issues request unchanged: Compaction request issued for om.db of om node: om3
ozone admin om snapshot defrag --service-id omservice --node-id om3 reaches om3:9862 unchanged: reaches real om3:9862 (not 0.0.0.0)

Generated-by: Claude Code (Opus 4.8)

@rich7420 rich7420 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks for the patch

@sreejasahithi sreejasahithi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chihsuan for fixing this

  1. CommandLine descriptions or the --help text for both commands does not mention the HA --node-id requirement.
    It would be better to add there as well.

  2. Should HA nodeId validation failure exit non-zero, or is exit 0 ok here?

@adoroszlai

Copy link
Copy Markdown
Contributor
  1. Should HA nodeId validation failure exit non-zero, or is exit 0 ok here?

Good point.

Copilot AI review requested due to automatic review settings July 13, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chihsuan

Copy link
Copy Markdown
Contributor Author

Thanks for raising this! @sreejasahithi

  1. CommandLine descriptions or the --help text for both commands does not mention the HA --node-id requirement.
    It would be better to add there as well.

Sounds good! I updated the --node-id descriptions for both commands in e6758ff.

  1. Should HA nodeId validation failure exit non-zero, or is exit 0 ok here?

Yeah, I agree. I considered returning a non-zero value, but the existing OMNodeDetails validation returns exit code 0, so I kept the new validation consistent. I can change it in this PR if you prefer; otherwise, we can address exit codes consistently in a follow-up. What do you think?

@jojochuang
jojochuang self-requested a review July 13, 2026 16:34
@adoroszlai
adoroszlai requested a review from sreejasahithi July 13, 2026 21:05
@sreejasahithi

Copy link
Copy Markdown
Contributor

Yeah, I agree. I considered returning a non-zero value, but the existing OMNodeDetails validation returns exit code 0, so I kept the new validation consistent. I can change it in this PR if you prefer; otherwise, we can address exit codes consistently in a follow-up. What do you think?

I am fine tracking consistent non zero exit codes in a follow up Jira if you want to keep this PR scoped for fail fast fix for missing --node-id.

@sreejasahithi sreejasahithi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update the --help outputs of the commands in the respective docs as they still show the old --node-id description.

Copilot AI review requested due to automatic review settings July 15, 2026 09:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chihsuan

Copy link
Copy Markdown
Contributor Author

I am fine tracking consistent non zero exit codes in a follow up Jira if you want to keep this PR scoped for fail fast fix for missing --node-id.

Thanks @sreejasahithi I created HDDS-15865 to track consistent non-zero exit codes for OM compact and snapshot defrag validation failures.

HDDS-15765. Update command help output in docs

I’ve also updated the checked-in --help output for both commands. 4747b6a

@chihsuan
chihsuan requested a review from sreejasahithi July 15, 2026 09:32

@sreejasahithi sreejasahithi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sarvekshayr
sarvekshayr merged commit e0bdbc2 into apache:master Jul 15, 2026
46 checks passed
@sarvekshayr

Copy link
Copy Markdown
Contributor

Thanks @chihsuan for the patch, @rich7420 and @sreejasahithi for the review.

errose28 added a commit to errose28/ozone that referenced this pull request Jul 16, 2026
* master: (53 commits)
  HDDS-15876. Remove ozone-csi (apache#10780)
  HDDS-15508. Add 2.2.0 to compatibility/upgrade test (apache#10782)
  HDDS-15867. [JDK25] Bump byte-buddy to 1.18.11 (apache#10776)
  HDDS-15809. Validate x-amz-copy-source-range on UploadPartCopy (apache#10711).
  HDDS-15678. OFS isDirectory/isFile should not trigger pipeline refresh or return block locations (apache#10741)
  HDDS-15171. Add available space check on follower during bootstrap. (apache#10185)
  HDDS-14043. Fix ls -e UnsupportedOperationException on ofs/o3fs (apache#10209)
  HDDS-15709. Replace random name suffix with uniqueObjectName (apache#10773)
  HDDS-15849. Force channel shutdown in XceiverClientGrpc.close (apache#10747)
  HDDS-15765. Fail fast when --node-id is omitted for OM compact/defrag on an HA cluster (apache#10699)
  HDDS-14989. Delay follower SCM DN server start until Ratis log catch-up. (apache#10617).
  HDDS-15456. Add SCM DB lookup and identify orphan(wrt SCM) and deleted-but-present containers (apache#10547)
  HDDS-15812. Rename test utils for naming convention (apache#10766)
  HDDS-15600. Fix ListObjects response for encoding-type and empty delimiter (apache#10586).
  HDDS-15656. Fix non-atomic data size accumulation in ContainerBalancer move callback (apache#10708)
  HDDS-15667. `containerbalancer status`: show stop reason and iteration details after stop (apache#10681)
  HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException (apache#10721)
  HDDS-15837. Add health-aware container ID listing in SCM (apache#10736)
  HDDS-15851. Return 403 for expired or out-of-range pre-signed S3 URLs (apache#10749)
  HDDS-15836. Add Archiver.appendFile for incremental TAR writes (apache#10735)
  ...

Conflicts:
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeMetrics.java
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java
hadoop-ozone/dist/src/main/compose/upgrade/test.sh
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/HddsUpgradeTestUtils.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestDNDataDistributionFinalization.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestScmDataDistributionFinalization.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestScmHAFinalization.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestBlockDeletionService.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
errose28 added a commit to errose28/ozone that referenced this pull request Jul 16, 2026
* HDDS-14496-zdu: (56 commits)
  HDDS-15848. Add new upgrade action annotations for component versions (apache#10744)
  HDDS-15876. Remove ozone-csi (apache#10780)
  HDDS-14825. Add Grafana Dashboard and Metrics for ZDU (apache#10602)
  HDDS-15508. Add 2.2.0 to compatibility/upgrade test (apache#10782)
  HDDS-15867. [JDK25] Bump byte-buddy to 1.18.11 (apache#10776)
  HDDS-15809. Validate x-amz-copy-source-range on UploadPartCopy (apache#10711).
  HDDS-15678. OFS isDirectory/isFile should not trigger pipeline refresh or return block locations (apache#10741)
  HDDS-15171. Add available space check on follower during bootstrap. (apache#10185)
  HDDS-14043. Fix ls -e UnsupportedOperationException on ofs/o3fs (apache#10209)
  HDDS-15709. Replace random name suffix with uniqueObjectName (apache#10773)
  HDDS-15677. Update client side version checks to use `isSupportedBy`. (apache#10611)
  HDDS-15849. Force channel shutdown in XceiverClientGrpc.close (apache#10747)
  HDDS-15765. Fail fast when --node-id is omitted for OM compact/defrag on an HA cluster (apache#10699)
  HDDS-14989. Delay follower SCM DN server start until Ratis log catch-up. (apache#10617).
  HDDS-15456. Add SCM DB lookup and identify orphan(wrt SCM) and deleted-but-present containers (apache#10547)
  HDDS-15812. Rename test utils for naming convention (apache#10766)
  HDDS-15600. Fix ListObjects response for encoding-type and empty delimiter (apache#10586).
  HDDS-15656. Fix non-atomic data size accumulation in ContainerBalancer move callback (apache#10708)
  HDDS-15667. `containerbalancer status`: show stop reason and iteration details after stop (apache#10681)
  HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException (apache#10721)
  ...

Conflicts:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerPrepare.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants