Improve HadoopCatalog performance/scalability #2124#2125
Merged
rdblue merged 5 commits intoapache:masterfrom Jan 28, 2021
Merged
Improve HadoopCatalog performance/scalability #2124#2125rdblue merged 5 commits intoapache:masterfrom
rdblue merged 5 commits intoapache:masterfrom
Conversation
Move to RemoteIterator for scanning directories. It's not as elegant as using the java8 streaming, but it works with the prefetching that the s3a and (soon) abfs connectors do, as well as bailing out more efficiently. Also added check for access errors to also look for AccessDeniedException
steveloughran
force-pushed
the
outgoing/2124-HadoopCatalog
branch
from
eb8408f to
4195a3a
Compare
* corrected the assertEquals() for the correct error messages * only using listIterator on the two operation which is doing nested file I/O underneath; this is where speedups could be observed.
rdblue
reviewed
Jan 22, 2021
| try { | ||
| return fs.listStatus(metadataPath, TABLE_FILTER).length >= 1; | ||
| } catch (FileNotFoundException e) { | ||
| } catch (FileNotFoundException e) { |
Contributor
There was a problem hiding this comment.
Accidental whitespace change?
rdblue
approved these changes
Jan 22, 2021
Contributor
|
Looks good to me overall. Any reason why it is a draft? |
Contributor
|
Looks like there are checkstyle issues to fix: |
Contributor
Author
cos for some reason some of the tests are failing, and I'm just getting through your build process before I start bothering people for reviews. one of the listIterator changes isn't working, and I want to understand why |
Contributor
|
Tests look like they're passing to me? |
Contributor
Author
|
updated to fix the checkstyle. Also did a quick fix for azure exception reporting...this patch will continue to work with branches with and without the hadoop changes |
Contributor
|
A couple more checkstyle errors: The JDK 11 test failure is a flaky test with Hive that we need to track down. |
Contributor
Author
|
ok. I thought those imports were in use. Will fix; just a bit distracted today. |
Contributor
|
Looks good. Thanks for fixing this! |
Contributor
Author
|
thanks! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Move to RemoteIterator for scanning directories.
It's not as elegant as using the java8 streaming, but it works with
the prefetching that the s3a and (soon) abfs connectors do, as well
as bailing out more efficiently.
Because each directory is probed with its own getFileStatus and list calls, the overhead of the outer list could be entirely swallowed by those inner probes -at least if there is >1 page of results in the listing and the implementation is prefetching.
Also added check for access errors to also look for AccessDeniedException; that's to support other filesystems and to prepare for HADOOP-15710