HDFS-13616. Batch listing of multiple directories #1725
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A rebase of the original patch by @umbrant . There are not too many conflicts when doing the rebasing. IMO the original patch is already in a very good shape. I'll start from here and see if people have any comments. cc @jojochuang |
|
Glad that this is being picked back up! Thanks Chao! |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
requested changes
Nov 21, 2019
| @@ -697,6 +697,12 @@ boolean mkdirs(String src, FsPermission masked, boolean createParent) | |||
| DirectoryListing getListing(String src, byte[] startAfter, | |||
| boolean needLocation) throws IOException; | |||
|
|
|||
| @Idempotent | |||
| BatchedDirectoryListing getBatchedListing( | |||
There was a problem hiding this comment.
I believe we want to add the annotation
@ReadOnly(isCoordinated = true)
| @Override | ||
| public BatchedDirectoryListing getBatchedListing(String[] srcs, | ||
| byte[] startAfter, boolean needLocation) throws IOException { | ||
| throw new UnsupportedOperationException("Not implemented"); |
There was a problem hiding this comment.
let's file a jira to support this API in RBF.
There was a problem hiding this comment.
Done. Filed https://issues.apache.org/jira/browse/HDFS-15029
| public byte[] getSrcPathsHash(String[] srcs) { | ||
| MessageDigest md; | ||
| try { | ||
| md = MessageDigest.getInstance("MD5"); |
There was a problem hiding this comment.
We should refine this method.
- synchronize this method.
- MessageDigest.getInstance() has big overhead so we should reuse it. (It's instantiates a new object each time)
- After use, call MessageDigest#reset().
https://stackoverflow.com/questions/13802627/when-to-use-messagedigest-reset
There was a problem hiding this comment.
Good point. Will fix this.
| return md.digest(); | ||
| } | ||
|
|
||
| BatchedDirectoryListing getBatchedListing(String[] srcs, byte[] startAfter, |
There was a problem hiding this comment.
This method is longer than I can possibly understand.
There was a problem hiding this comment.
I found a little hard to refactoring this given that several variables are used throughout the method, such as lastListing. Would it be helpful if we add more comments in the method body, explaining what each section is doing?
There was a problem hiding this comment.
ok. i'm just being lazy.
|
💔 -1 overall
This message was automatically generated. |
jojochuang
requested changes
Dec 6, 2019
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/ListingBenchmark.java
Show resolved
Hide resolved
- Caches digest and make accesses to it synchronized. - Changes `parent` to `listedPath` according to CR on JIRA itself - Added documentation for a few public methods - Added metrics for listing operation
|
One thing I'm not totally sure is the case where the input which I think is not very accurate. Is |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
approved these changes
Dec 12, 2019
jojochuang
approved these changes
Dec 12, 2019
|
💔 -1 overall
This message was automatically generated. |
RogPodge
pushed a commit
to RogPodge/hadoop
that referenced
this pull request
Mar 25, 2020
|
Hello, I encountered a bug when using the batch method, when I input a directory with more than 1000 files in it and 2 replications of each file's data block, only the first 500 files of this directory are returned and then it stops. I think it should be hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java in getBatchedListing() method to modify, as follows. The reason for this bug is mainly that the result returned by the getListingInt(dir, pc, src, indexStartAfter, needLocation) method will limit both the number of files in the directory as well as the number of data blocks and replications of the files at the same time. But the getBatchedListing() method will only exit the loop if the number of returned results is greater than 1000. |
|
@fanlinqian best to file an HDFS issue on the apache jira server. |
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 23, 2023
(cherry picked from commit d7c4f8a) Change-Id: I7b058f0761a66558907401adf0340c86de3a5993
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the dominant workloads for external metadata services is listing of partition directories. This can end up being bottlenecked on RTT time when partition directories contain a small number of files. This is fairly common, since fine-grained partitioning is used for partition pruning by the query engines.
A batched listing API that takes multiple paths amortizes the RTT cost. Initial benchmarks show a 10-20x improvement in metadata loading performance.