Skip to content

HDDS-11747. Add debug CLI to show full path of keys present in specific containers #7465

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

Closed
wants to merge 11 commits into from

Conversation

sumitagrawl
Copy link
Contributor

@sumitagrawl sumitagrawl commented Nov 21, 2024

What changes were proposed in this pull request?

This provide a CLI option to get key full path in FSO case for matching criteria like container. DB will be opened in read-only mode.

ozone debug retrieve-key-fullpath --db=<db path with db name> --containers=<container separated by comma>

ozone debug retrieve-key-fullpath --db=<db path with db name> --container-file=<file having list of container separated by new line>

example:

ozone debug retrieve-key-fullpath --db=om.db --containers=1,2,4

Sample output:

vol/bucket1/dir1/file1
vol/bucket1/dir1/file2

What is the link to the Apache JIRA

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

How was this patch tested?

  • Unit and Integration test is added

@errose28
Copy link
Contributor

For the CLI I would propose something like this:

ozone debug om container-to-key --db=<db path with db name> [--container-file=<file having list of container separated by new line>] [--containers=<comma separated list of container IDs>] [--output <out file name having file full path>]

We should also support reading container IDs from stdin to support piping. This gives three ways to get container IDs into the command. IMO we should not make them exclusive and read from all sources to get the final set of container IDs to operate on. At least one container ID must be present from somewhere for the command to succeed.

--output would also be optional and output would go to stdout otherwise. To support redirection please make sure all output that is not part of the final key list goes to stderr.

@sumitagrawl sumitagrawl requested a review from errose28 November 22, 2024 05:43
@sumitagrawl sumitagrawl marked this pull request as ready for review November 26, 2024 04:58
@ashishkumar50
Copy link
Contributor

@errose28, Would you like to take another look on this?

@errose28
Copy link
Contributor

errose28 commented Dec 2, 2024

Thanks, yes I would like to do another round of review, but have been busy with other tasks and was off for the holiday. I should get to it later this week.

@adoroszlai adoroszlai changed the title HDDS-11747. CLI retrieve full key path for matching containers HDDS-11747. Add debug CLI to show full path of keys present in specific containers Dec 3, 2024
@adoroszlai adoroszlai added the tools Tools that helps with debugging label Dec 12, 2024
Comment on lines +196 to +198
StringBuilder sb = new StringBuilder();
key1.getOzoneKeyLocations().forEach(e -> sb.append(e.getContainerID()));
key3.getOzoneKeyLocations().forEach(e -> sb.append(",").append(e.getContainerID()));
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not produce the expected output if key1 has more than one location.

      String containers = Stream.concat(key1.getOzoneKeyLocations().stream(), key3.getOzoneKeyLocations().stream())
          .mapToLong(OzoneKeyLocation::getContainerID)
          .mapToObj(String::valueOf)
          .collect(Collectors.joining(","));

key1.getOzoneKeyLocations().forEach(e -> sb.append(e.getContainerID()));
key3.getOzoneKeyLocations().forEach(e -> sb.append(",").append(e.getContainerID()));
String dbFile = OMStorage.getOmDbDir(conf).getPath() + "/om.db";
cmd = new CommandLine(new OzoneDebug()).addSubcommand(new CommandLine(new KeyPathRetriever())).setOut(pstdout);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not add subcommand manually.

Suggested change
cmd = new CommandLine(new OzoneDebug()).addSubcommand(new CommandLine(new KeyPathRetriever())).setOut(pstdout);
cmd = new OzoneDebug().getCmd().setOut(pstdout);

Comment on lines +206 to +207
assertThat(stdout.toString()).contains(keyPrefix + "file1").contains(keyPrefix + "file2")
.doesNotContain(keyPrefix + "file3");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: please wrap.

Suggested change
assertThat(stdout.toString()).contains(keyPrefix + "file1").contains(keyPrefix + "file2")
.doesNotContain(keyPrefix + "file3");
assertThat(stdout.toString())
.contains(keyPrefix + "file1")
.contains(keyPrefix + "file2")
.doesNotContain(keyPrefix + "file3");

TestDataUtil.createKey(bucket, "dir1/file2", "test");
TestDataUtil.createKey(bucket, "dir1/file3", "test");
OzoneKeyDetails key1 = bucket.getKey("dir1/file1");
OzoneKeyDetails key3 = bucket.getKey("dir1/file2");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: key3 vs. file2 is confusing, especially in final assertion, where we expect file2, but not file3.

@adoroszlai adoroszlai marked this pull request as draft February 17, 2025 22:07
@adoroszlai
Copy link
Contributor

/pending conflicts, review comments

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

conflicts, review comments

Copy link

Thank you very much for the patch. I am closing this PR temporarily as there was no activity recently and it is waiting for response from its author.

It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.

It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.

If you need ANY help to finish this PR, please contact the community on the mailing list or the slack channel."

@github-actions github-actions bot closed this Apr 30, 2025
@sumitagrawl sumitagrawl reopened this May 5, 2025
@sumitagrawl
Copy link
Contributor Author

@errose28 please check and review, after that will handle conflicts and fix other test case review comments.

@adoroszlai
Copy link
Contributor

@sumitagrawl please reopen if you update the patch, not to ping reviewers

@adoroszlai adoroszlai closed this May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending tools Tools that helps with debugging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants