HDDS-12057. Implement command ozone debug replicas verify checksums #7748

ptlrs · 2025-01-24T17:11:54Z

What changes were proposed in this pull request?

This PR:

adds a new ozone debug replicas verify command
renamed read-replicas to checksums command
updated checksums command with the ability to walk the file tree and calculate checksums of all files
made a URI a required parameter for all subcommands of replicas verify

Note: This is a WIP change. Further enhancements and refactoring will be done and acceptance tests will be updated.

What is the link to the Apache JIRA

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

How was this patch tested?

Manual testing in docker-compose environment
CI: https://github.com/ptlrs/ozone/actions/runs/12954171235

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java

jojochuang · 2025-01-24T20:56:34Z

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java

+      File dir = createDirectory(volumeName, bucketName, keyName);
+      OzoneKeyDetails keyInfoDetails = checksumClient.getKeyDetails(volumeName, bucketName, keyName);
+      Map<OmKeyLocationInfo, Map<DatanodeDetails, OzoneInputStream>> replicas =
+          checksumClient.getKeysEveryReplicas(volumeName, bucketName, keyName);


Not totally related, but RpcClient.getKeysEveryReplicas() doesn't seems to create input stream that refreshes container cache upon failure. Could that lead problems in such a corner case?

https://github.com/apache/ozone/blob/master/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L1609

sarvekshayr

Hi @ptlrs
The Replicas.java implementation already exists under the name ReplicasDebug.java as part of the replicas package. Could you please move these files to that package?

adoroszlai

Thanks @ptlrs for working on this.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java

ptlrs · 2025-02-17T08:30:32Z

Thanks for the reviews @jojochuang @adoroszlai @sarvekshayr.
I have made some changes based on the comments. Could you please take another look.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/Replicas.java

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReplicasVerify.java

sarvekshayr

Thank you @ptlrs for updating the patch. Suggested two minor improvements below:

sarvekshayr

LGTM.

adoroszlai

Thanks @ptlrs for improving the patch.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/Checksums.java

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasUtils.java

errose28

Just left some comments on the CLI construction. See HDDS-12206 for a description of how this should fit together.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/Checksums.java

hadoop-ozone/dist/src/main/smoketest/debug/ozone-debug.robot

…taneously

errose28

Thanks for working on this @ptlrs. Overall looks good as a port of the existing checks. There is a lot of room for improvement within the existing code of those checks though so I'll file a follow-up Jira to redo the read replicas/checksum verification in a more efficient manner.

Usually we don't put log messages in CLI commands:

Users may not see them depending how the log4j config is set up.
It's clunky to control log levels at the command line, and often a full set of levels is not needed.
- Usually just a two level system with regular messages, and additional messages controlled by a --verbose flag will suffice.

There's some older commands using the LOG object fromHandler that we should probably fix up later. I think we can replace IOUtils#close with closeQuietly in this context, since we've already completed verification at this point and failure to close the streams is not actionable by the user.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/KeyParts.java

…/replicas/ReplicasVerify.java Co-authored-by: Ethan Rose <[email protected]>

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerifier.java

errose28

LGTM. We can improve the implementations of existing checks that were ported over in follow up tasks.

adoroszlai · 2025-03-13T08:31:27Z

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerify.java

+  @CommandLine.Option(names = "--checksums",
+      description = "Do client side data checksum validation of all replicas.",
+      // value will be true only if the "--checksums" option was specified on the CLI
+      defaultValue = "false")
+  private boolean doExecuteChecksums;
+
+  @CommandLine.Option(names = "--padding",
+      description = "Check for missing padding in erasure coded replicas.",
+      defaultValue = "false")
+  private boolean doExecutePadding;


The command does not do anything useful without one of these two options. They should be in an @ArgGroup(exclusive = false, multiplicity = "1").

https://picocli.info/#_argument_groups

adoroszlai

Thanks @ptlrs for updating the patch, LGTM.

$ ozone debug replicas verify --output-dir /tmp /
Error: Missing required argument(s): ([--checksums] [--padding])

ptlrs mentioned this pull request Jan 24, 2025

HDDS-12119. Move find-missing-padding command under ozone debug replicas #7749

Draft

jojochuang reviewed Jan 24, 2025

View reviewed changes

sarvekshayr reviewed Jan 28, 2025

View reviewed changes

adoroszlai changed the title ~~HDDS-12094.Implement ozone debug replicas verify checksums command~~ HDDS-12057. Implement command ozone debug replicas verify checksums Jan 29, 2025

adoroszlai reviewed Jan 29, 2025

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java Outdated Show resolved Hide resolved

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReadReplicas.java Outdated Show resolved Hide resolved

errose28 added the tools Tools that helps with debugging label Feb 6, 2025

ptlrs force-pushed the HDDS-12057-checksums branch from 6b649c9 to 53130e9 Compare February 17, 2025 08:21

ptlrs requested review from jojochuang, adoroszlai and sarvekshayr February 17, 2025 08:29

sarvekshayr reviewed Feb 17, 2025

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/Replicas.java Outdated Show resolved Hide resolved

sarvekshayr reviewed Feb 17, 2025

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/ReplicasVerify.java Outdated Show resolved Hide resolved

sarvekshayr reviewed Feb 17, 2025

View reviewed changes

ptlrs force-pushed the HDDS-12057-checksums branch from da5bfee to 4c05627 Compare February 19, 2025 02:32

ptlrs requested a review from sarvekshayr February 19, 2025 02:38

sarvekshayr approved these changes Feb 20, 2025

View reviewed changes

ptlrs marked this pull request as ready for review February 20, 2025 14:07

adoroszlai reviewed Feb 20, 2025

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/Checksums.java Outdated Show resolved Hide resolved

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasUtils.java Outdated Show resolved Hide resolved

errose28 reviewed Feb 20, 2025

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/Checksums.java Outdated Show resolved Hide resolved

hadoop-ozone/dist/src/main/smoketest/debug/ozone-debug.robot Outdated Show resolved Hide resolved

adoroszlai marked this pull request as draft February 25, 2025 20:24

ptlrs added 4 commits February 25, 2025 13:03

HDDS-12094.Implement ozone debug replicas verify checksums command

e02c966

fix review comments

e8773f3

deduplicate the common methods

23bbee2

refactored the changes to allow multiple checks to be performed simul…

d271183

…taneously

ptlrs force-pushed the HDDS-12057-checksums branch from 4c05627 to d271183 Compare February 26, 2025 18:23

fix checkstyle

8452236

adoroszlai requested a review from errose28 February 27, 2025 16:57

errose28 reviewed Mar 7, 2025

View reviewed changes

Update hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug…

587801e

…/replicas/ReplicasVerify.java Co-authored-by: Ethan Rose <[email protected]>

ptlrs and others added 4 commits March 10, 2025 21:04

Update hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug…

c8eed93

…/replicas/ReplicasVerify.java Co-authored-by: Ethan Rose <[email protected]>

Update hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug…

bedb4f1

…/replicas/ReplicasVerify.java Co-authored-by: Ethan Rose <[email protected]>

fix review comments to implement a verifier interface

63f4195

fix review comments to implement a verifier interface

b7cbfed

errose28 reviewed Mar 12, 2025

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerifier.java Outdated Show resolved Hide resolved

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/replicas/ReplicasVerifier.java Outdated Show resolved Hide resolved

fix review comments

aa8c10f

ptlrs requested a review from errose28 March 12, 2025 17:38

fix review comments

b93672e

errose28 approved these changes Mar 12, 2025

View reviewed changes

ptlrs requested a review from adoroszlai March 13, 2025 00:34

adoroszlai reviewed Mar 13, 2025

View reviewed changes

use option group to ensure atleast one verification is performed

c911ba7

ptlrs requested a review from adoroszlai March 13, 2025 19:08

adoroszlai reviewed Mar 13, 2025

View reviewed changes

adoroszlai marked this pull request as ready for review March 13, 2025 19:18

errose28 approved these changes Mar 13, 2025

View reviewed changes

errose28 merged commit 786da39 into apache:master Mar 13, 2025
55 checks passed

ptlrs deleted the HDDS-12057-checksums branch March 13, 2025 21:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-12057. Implement command ozone debug replicas verify checksums #7748

HDDS-12057. Implement command ozone debug replicas verify checksums #7748

ptlrs commented Jan 24, 2025 •

edited

Loading

jojochuang Jan 24, 2025

sarvekshayr left a comment

adoroszlai left a comment

ptlrs commented Feb 17, 2025

sarvekshayr left a comment

sarvekshayr left a comment

adoroszlai left a comment

errose28 left a comment

errose28 left a comment

errose28 left a comment

adoroszlai Mar 13, 2025

ptlrs Mar 13, 2025

adoroszlai left a comment

HDDS-12057. Implement command ozone debug replicas verify checksums #7748

HDDS-12057. Implement command ozone debug replicas verify checksums #7748

Conversation

ptlrs commented Jan 24, 2025 • edited Loading

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

jojochuang Jan 24, 2025

Choose a reason for hiding this comment

sarvekshayr left a comment

Choose a reason for hiding this comment

adoroszlai left a comment

Choose a reason for hiding this comment

ptlrs commented Feb 17, 2025

sarvekshayr left a comment

Choose a reason for hiding this comment

sarvekshayr left a comment

Choose a reason for hiding this comment

adoroszlai left a comment

Choose a reason for hiding this comment

errose28 left a comment

Choose a reason for hiding this comment

errose28 left a comment

Choose a reason for hiding this comment

errose28 left a comment

Choose a reason for hiding this comment

adoroszlai Mar 13, 2025

Choose a reason for hiding this comment

ptlrs Mar 13, 2025

Choose a reason for hiding this comment

adoroszlai left a comment

Choose a reason for hiding this comment

ptlrs commented Jan 24, 2025 •

edited

Loading