Improve listing performance of Hudi tables by codope · Pull Request #17244 · prestodb/presto

codope · 2022-02-01T02:16:09Z

Integrate metadata-based listing for Hudi tables. This is
enabled by a session property.
Implement a new DirectoryLister for Hudi that uses
HoodieTableFileSystemView to fetch data files.
Bump Hudi version to 0.10.1 to use above features.
Replace hudi-common, hudi-hadoop-mr by
hudi-presto-bundle.

Test plan - (Please fill in how you tested your changes)

Synced a Hudi table to Hive and queried through Presto.
Add unit test for HudiDirectoryLister.

== RELEASE NOTES ==

Hive Changes
* Upgrade Hudi version to 0.10.1.
* Support metadata-based listing and bootstrap for Hudi tables.

codope · 2022-02-01T02:17:44Z

@arunthirupathi this is rework of #17084 with thinned presto bundle. Could you please review?
cc @vinothchandar

arunthirupathi · 2022-02-06T07:25:32Z

Can you post the mvn dependency difference before this change and after this change ?

What is the size of the shaded jar ?

I made a pass on this PR and it looks good. I need to sync up to this PR and do an internal build to ensure it does not regress things.

arunthirupathi · 2022-02-06T07:26:43Z

I will update by this Wednesday on whether this PR works with our internal dependencies correctly.

arunthirupathi · 2022-02-06T07:28:01Z

pom.xml

Why are their still excludes if the hudi-presto-bundle is shaded ?

Removed unnecessary exclusions. There are some hudi modules which we need to exclude and then there is protobuf, which if not excluded, was causing the build to fail due to some conflicting resource files.

codope · 2022-02-17T13:35:41Z

Can you post the mvn dependency difference before this change and after this change ?

What is the size of the shaded jar ?

I made a pass on this PR and it looks good. I need to sync up to this PR and do an internal build to ensure it does not regress things.

The gist contains the full dependency tree for master and this branch. Below is the diff:

sagars@Sagars-MacBook-Pro presto % git diff --no-index presto_hive_master_dep.txt presto_hive_hudi_dep.txt
diff --git a/presto_hive_master_dep.txt b/presto_hive_hudi_dep.txt
index 83b59e2092..dd99c891e9 100644
--- a/presto_hive_master_dep.txt
+++ b/presto_hive_hudi_dep.txt
@@ -19,8 +19,7 @@
 [INFO] +- com.facebook.presto:presto-expressions:jar:0.271-SNAPSHOT:compile
 [INFO] +- com.facebook.presto:presto-cache:jar:0.271-SNAPSHOT:compile
 [INFO] |  \- org.alluxio:alluxio-shaded-client:jar:2.7.0:compile
-[INFO] +- org.apache.hudi:hudi-common:jar:0.9.0:compile
-[INFO] +- org.apache.hudi:hudi-hadoop-mr:jar:0.9.0:compile
+[INFO] +- org.apache.hudi:hudi-presto-bundle:jar:0.10.1:compile
 [INFO] +- com.facebook.presto:presto-memory-context:jar:0.271-SNAPSHOT:compile
 [INFO] +- com.facebook.presto:presto-hive-common:jar:0.271-SNAPSHOT:compile
 [INFO] +- com.facebook.presto:presto-hive-metastore:jar:0.271-SNAPSHOT:compile
@@ -248,6 +247,6 @@
 [INFO] ------------------------------------------------------------------------
 [INFO] BUILD SUCCESS
 [INFO] ------------------------------------------------------------------------
-[INFO] Total time:  1.788 s
-[INFO] Finished at: 2022-02-17T18:41:17+05:30
+[INFO] Total time:  2.052 s
+[INFO] Finished at: 2022-02-17T18:56:17+05:30
 [INFO] ------------------------------------------------------------------------

The size of the shaded jar is 16MB.

codope · 2022-02-18T04:55:08Z

@arunthirupathi Could you please take a look again? I have removed unnecessary excludes. Also, was there any issue you fuond out running this internally?

arunthirupathi · 2022-02-18T06:50:27Z

There was a minor change required due to PathFilter to Optional < PathFilter > , but the build passed. I am going to kick off integration tests to see if it causes any issues.

arunthirupathi · 2022-02-19T04:11:32Z

the integration tests succeeded, so this is good. I will take final pass on Tuesday and merge this in.

codope · 2022-02-21T06:33:08Z

the integration tests succeeded, so this is good. I will take final pass on Tuesday and merge this in.

Glad to hear this. Thanks for the update.

arunthirupathi · 2022-02-23T06:52:04Z

Can you please rebase and push ? There is a breaking change in our internal repo, I have submitted a PR for it. Once that is approved, I will merge this change and the other change together.

presto-hive/src/main/java/com/facebook/presto/hive/HudiDirectoryLister.java

presto-hive/src/main/java/com/facebook/presto/hive/StoragePartitionLoader.java

arunthirupathi · 2022-02-23T07:03:14Z

presto-hive/src/main/java/com/facebook/presto/hive/util/HiveFileIterator.java

On a second thought it will be easier, if you can undo this change to this file.

Where you need the filter to always work, pass in path filter where it always returns true. () -> true

This will make it easier for me to merge this PR.

PathFilter was added to DirectoryLister due to Hudi. After this patch, we don't really need it. I was planning to remove that as a follow-up to this PR if that's ok with you? Let me know if you think it's a blocker then i can make that change with this PR itself.

Let us do it with this PR itself.

got it.. will update this PR in a day or two.

updated the PR

- Integrate metadata-based listing for Hudi tables. This is enabled by a session property. - Implement a new DirectoryLister for Hudi that uses HoodieTableFileSystemView to fetch data files. - Bump Hudi version to 0.10.1 to use above features. - Add unit tests for HudiDirectoryLister. - Replace hudi-common, hudi-hadoop-mr by hudi-presto-bundle. - Remove PathFilter from DirectoryLister interface.

codope · 2022-03-01T07:37:43Z

@arunthirupathi This PR is ready now. I have removed the path filter form DirectoryLister interface and tested locally.

arunthirupathi · 2022-03-04T07:03:37Z

This change looks good, since this breaks API, I need to have a PR for our internal changes get it approved and then merge this in at the same time.

codope · 2022-03-05T03:49:20Z

Thanks @arunthirupathi for shepherding and landing this!

rohanpednekar added the awaiting-community-help label Feb 4, 2022

arunthirupathi reviewed Feb 6, 2022

View reviewed changes

codope force-pushed the presto-hudi-metadata branch 3 times, most recently from 1334c99 to 9f2225a Compare February 17, 2022 13:31

arunthirupathi self-requested a review February 19, 2022 04:11

arunthirupathi approved these changes Feb 23, 2022

View reviewed changes

presto-hive/src/main/java/com/facebook/presto/hive/HudiDirectoryLister.java Outdated Show resolved Hide resolved

presto-hive/src/main/java/com/facebook/presto/hive/StoragePartitionLoader.java Outdated Show resolved Hide resolved

arunthirupathi reviewed Feb 23, 2022

View reviewed changes

codope force-pushed the presto-hudi-metadata branch from 9f2225a to cd36a24 Compare February 28, 2022 11:47

codope force-pushed the presto-hudi-metadata branch from cd36a24 to d4f9f0b Compare March 1, 2022 05:15

arunthirupathi merged commit ef1fd25 into prestodb:master Mar 4, 2022

varungajjala mentioned this pull request Mar 22, 2022

Add release notes for 0.272 #17499

Closed

9 tasks

asjadsyed mentioned this pull request Mar 23, 2022

Add release notes for 0.272 #17510

Closed

9 tasks

asjadsyed mentioned this pull request Apr 1, 2022

Add release notes for 0.272 #17564

Merged

8 tasks

codope mentioned this pull request Apr 30, 2022

Revert "Improve listing performance of Hudi tables" on release-0.272 #17706

Merged

Conversation

codope commented Feb 1, 2022

Uh oh!

codope commented Feb 1, 2022

Uh oh!

arunthirupathi commented Feb 6, 2022

Uh oh!

arunthirupathi commented Feb 6, 2022

Uh oh!

arunthirupathi Feb 6, 2022

Choose a reason for hiding this comment

Uh oh!

codope Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

codope commented Feb 17, 2022

Uh oh!

codope commented Feb 18, 2022

Uh oh!

arunthirupathi commented Feb 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arunthirupathi commented Feb 19, 2022

Uh oh!

codope commented Feb 21, 2022

Uh oh!

arunthirupathi commented Feb 23, 2022

Uh oh!

Uh oh!

Uh oh!

arunthirupathi Feb 23, 2022

Choose a reason for hiding this comment

Uh oh!

codope Feb 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arunthirupathi Feb 23, 2022

Choose a reason for hiding this comment

Uh oh!

codope Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

codope Mar 1, 2022

Choose a reason for hiding this comment

Uh oh!

codope commented Mar 1, 2022

Uh oh!

arunthirupathi commented Mar 4, 2022

Uh oh!

codope commented Mar 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arunthirupathi commented Feb 18, 2022 •

edited

Loading

codope Feb 23, 2022 •

edited

Loading