Skip to content

Conversation

@symious
Copy link
Contributor

@symious symious commented Mar 21, 2024

What changes were proposed in this pull request?

When a volume has more than "1024" buckets, the command of "ozone fs -ls /volume" will reach endless loop.

It's caused by incorrect parameter passing in listStatus.

What is the link to the Apache JIRA

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

How was this patch tested?

unit test.

@adoroszlai adoroszlai changed the title HDDS-10562. Fix endless loop in ozone sh -ls HDDS-10562. Fix endless loop in ozone fs -ls Mar 21, 2024

String volName = "testlistbucket";
int numBuckets = 1025;
int numBuckets = pageSize;
Copy link
Contributor

@adoroszlai adoroszlai Mar 22, 2024

Choose a reason for hiding this comment

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

Should it be pageSize + 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pageSize will also trigger the bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adoroszlai I wonder if there is a timeout wrapper on some code block in unit test, so that it can fail fast?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add @Timeout(seconds) on the test method, e.g.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adoroszlai Thanks for the advice. Updated, PTAL.

I was also wondering if there is something like

timout.wrapper(ToolRunner.run(shell, new String[]{"-ls", "/" + volName}));

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @symious for the spotting this bug.

Comment on lines +885 to +887
String startBucketPath = ofsStartPath.getNonKeyPath();
return listStatusVolume(ofsPath.getVolumeName(),
recursive, startBucket, numEntries, uri, workingDir, username);
recursive, startBucketPath, numEntries, uri, workingDir, username);
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest changing startPath parameter of listStatusVolume to OFSPath, and passing ofsStartPath. This reduces the chance for such misuse, and also avoids creating another instance of OFSPath for the same path in the method. The method may also validate that ofsStartPath.getVolumeName() equals volumeNameStr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, all the following methods are using String as parameter, maybe we can align to use String here or it might confuse people more?

listStatusBucketSnapshot(ofsPath.getVolumeName(), ofsPath.getBucketName(), uri);
listStatusVolume(ofsPath.getVolumeName(), recursive, startBucketPath, numEntries, uri, workingDir, username);
listStatusRoot(recursive, startPath, numEntries, uri, workingDir, username);

symious and others added 3 commits March 22, 2024 14:26
…ozone/shell/TestOzoneShellHA.java

Co-authored-by: Doroszlai, Attila <[email protected]>
…ozone/shell/TestOzoneShellHA.java

Co-authored-by: Doroszlai, Attila <[email protected]>
@adoroszlai adoroszlai requested a review from smengcl March 22, 2024 08:10
@adoroszlai adoroszlai merged commit 4ca8edb into apache:master Mar 24, 2024
@symious symious deleted the HDDS-10562 branch March 25, 2024 02:01
@symious
Copy link
Contributor Author

symious commented Mar 25, 2024

@adoroszlai Thank you for the review and merge.

smitajoshi12 pushed a commit to smitajoshi12/ozone that referenced this pull request Mar 27, 2024
myskov pushed a commit to myskov/ozone that referenced this pull request Apr 4, 2024
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
(cherry picked from commit 4ca8edb)

Conflicts:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
ptlrs pushed a commit to ptlrs/ozone that referenced this pull request Mar 8, 2025
…pache#7627) (apache#190)

* CDPD-77427. HDDS-11997. Duplicate snapshot purge request causes NPE (apache#7627)

(cherry picked from commit ad108c8)
Change-Id: I02dae0a1994bafa6dc95bd2a9a38ecb82ac45a07

* CDPD-76953. HDDS-11893. Fix full snapshot diff fallback logic because of DAG pruning (apache#7549)

(cherry picked from commit 853d657)
Change-Id: Idbdcb724c2b2da23614c2be6a4e2a88f7a4f5a71
(cherry picked from commit 4cff8f827a9cfc810f48c1be767963dad428f1c5)

* CDPD-77091. HDDS-11908. Snapshot diff DAG traversal should not skip node based on prefix presence (apache#7567)

(cherry picked from commit 66ccc25)
Change-Id: Id1960cfadc9a983c89b379ccb87a4fa6d2586203
(cherry picked from commit b10b7b0d0a1b4c7fc3d0bbeda457a8ff0a0e2313)

* CDPD-77085. HDDS-11914. Snapshot diff should not filter SST Files based by reading SST file reader (apache#7563)

(cherry picked from commit 7a46080)
Change-Id: I7788f8b962985c12ba2cec9b8cab537a392a5b65
(cherry picked from commit b8601fe948a993632f8028a8947ee53dde6e4a5e)

* CDPD-67962. HDDS-10562. Fix infinite loop in ozone fs -ls /volume (apache#6416)

(cherry picked from commit 4ca8edb)
Change-Id: I015f524950ebf15491bcd8136caa11deea60027a
(cherry picked from commit 206353c44479761c6d416704c1b01aebd66b9922)

* CDPD-77818. HDDS-12064. Optimize bootstrap logic to reduce loop while checking file links (apache#7676)

Change-Id: Ibee016b338abdbb0bee6e62af89cc2560fb8f350

---------

Co-authored-by: Swaminathan Balachandran <[email protected]>
Co-authored-by: Swaminathan Balachandran <[email protected]>
Co-authored-by: Symious <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants