Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDDS-12012. Defer ozone repair prompt after subcommand validation #7653

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adoroszlai
Copy link
Contributor

@adoroszlai adoroszlai commented Jan 6, 2025

What changes were proposed in this pull request?

ozone repair shows warning prompt on startup. HDDS-11945 added logic to skip it when showing top-level usage.

This PR further improves it by deferring the prompt to subcommands, so that it is not shown for:

  • subcommand invoked without all required options and arguments
  • subcommand invoked with --help

Integration tests running ozone repair now need to pass y to stdin.

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

How was this patch tested?

$ ozone repair 
Incomplete command
Usage: ozone repair [-hV] [--verbose] [-conf=<configurationPath>]
                    [-D=<String=String>]... [COMMAND]
Advanced tool to repair Ozone. The nodes being repaired must be stopped before
the tool is run.
      -conf=<configurationPath>

  -D, --set=<String=String>

  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.
      --verbose   More verbose output. Show the stack trace of the errors.
Commands:
  cert-recover  Recover Deleted SCM Certificate from RocksDB
  om            Operational tool to repair OM.
  quota         Operational tool to repair quota in OM DB.
$ ozone repair om 
Missing required subcommand
Usage: ozone repair om [COMMAND]
Operational tool to repair OM.
Commands:
  fso-tree            Identify and repair a disconnected FSO tree by marking
                        unreferenced entries for deletion. OM should be stopped
                        while this tool is run.
  snapshot            Subcommand for all snapshot related repairs.
  update-transaction  CLI to update the highest index in transactionInfoTable.
                        Currently it is only supported for OM.
$ ozone repair om fso-tree --help
Usage: ozone repair om fso-tree [-hrV] [--force] [--verbose] [-b=<bucket>]
                                --db=<dbPath> [-v=<volume>]
...
$ ozone repair om fso-tree
Missing required option: '--db=<dbPath>'
Usage: ozone repair om fso-tree [-hrV] [--force] [--verbose] [-b=<bucket>]
                                --db=<dbPath> [-v=<volume>]
...
$ ozone repair om fso-tree --db /data/metadata/om.db
ATTENTION: Running as user hadoop. Make sure this is the same user used to run the Ozone process. Are you sure you want to continue (y/N)? n
Aborting command.
$ ozone repair om fso-tree --db /data/metadata/om.db
ATTENTION: Running as user hadoop. Make sure this is the same user used to run the Ozone process. Are you sure you want to continue (y/N)? y
Run as user: hadoop
Error: OM is currently running on this host with PID 7. Stop the service before running the repair tool.

CI:
https://github.com/adoroszlai/ozone/actions/runs/12637496667

@adoroszlai adoroszlai added the tools Tools that helps with debugging label Jan 6, 2025
@adoroszlai adoroszlai self-assigned this Jan 6, 2025
@sarvekshayr
Copy link
Contributor

Hi @adoroszlai. Please resolve the conflicts.

@adoroszlai
Copy link
Contributor Author

@sarvekshayr updated

Copy link
Contributor

@sarvekshayr sarvekshayr left a comment

Choose a reason for hiding this comment

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

Thank you @adoroszlai for the patch. The output is clear now.
LGTM.

Copy link
Contributor

@Tejaskriya Tejaskriya left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @adoroszlai , LGTM!

@adoroszlai adoroszlai requested a review from errose28 January 9, 2025 11:00
Comment on lines +29 to +30
} catch (RuntimeException e) {
throw e;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this catch block as we are not doing anything with the caught exception other than throwing it again.

Copy link
Contributor Author

@adoroszlai adoroszlai Jan 9, 2025

Choose a reason for hiding this comment

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

@nandakumar131 The goal of this catch block is to avoid wrapping RuntimeException in RuntimeException in the following block:

} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw new RuntimeException(e);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Tools that helps with debugging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants