Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.06%. Comparing base (f09f8af) to head (adfacc7).
Report is 25 commits behind head on main.

@@              Coverage Diff              @@
##               main    #1377       +/-   ##
=============================================
- Coverage     56.12%   39.06%   -17.06%     
- Complexity      976     2071     +1095     
=============================================
  Files           119      263      +144     
  Lines         11743    60746    +49003     
  Branches       2251    12909    +10658     
=============================================
+ Hits           6591    23733    +17142     
- Misses         4012    32530    +28518     
- Partials       1140     4483     +3343     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@parthchandra @andygrove @kazuyukitanimura @mbutrovich

From the org of datafusion-contrib, I see many hdfs crates, which one is best for comet?

From the org of datafusion-contrib, I see many hdfs crates, which one is best for comet?

I know it was brought up in the original issue, but just plugging my pure Rust implementation: https://github.com/datafusion-contrib/hdfs-native-object-store. It's already integrated in delta-rs and delta-kernel-rs. Though this arguably could be the one time a JNI based implementation could make sense since you're guaranteed to have Java installed and probably your classpath already set correctly since you're running Spark.

Doesn't seem like something that should have the implementation copied into the Comet repo, as it seems out of scope.

From the org of datafusion-contrib, I see many hdfs crates, which one is best for comet?

I know it was brought up in the original issue, but just plugging my pure Rust implementation: https://github.com/datafusion-contrib/hdfs-native-object-store. It's already integrated in delta-rs and delta-kernel-rs. Though this arguably could be the one time a JNI based implementation could make sense since you're guaranteed to have Java installed and probably your classpath already set correctly since you're running Spark.

Doesn't seem like something that should have the implementation copied into the Comet repo, as it seems out of scope.

Hey @Kimahriman its nice to see you here, we checked the contribution crate https://github.com/datafusion-contrib/hdfs-native-object-store and https://github.com/datafusion-contrib/datafusion-objectstore-hdfs.

I really like that the crate you mentioned because it has less memory footprint and has no JVM dependency and therefore no JVM roundtrips. Can't wait this crate to grow up and use it.

For now libhdfs on JVM provides more HDFS client configuration which critical on production sites comparing to https://github.com/Kimahriman/hdfs-native?tab=readme-ov-file#supported-hdfs-settings where it is hard to configure cluster network configuration, namenode retries, etc.

Can we have a README that covers any changes we have made (if any), and what versions of hdfs client are supported (and on what platforms)? Also, if we have made changes, can we add a comment in the code itself to mark the areas where the code has been updated?

This is a good point.

For now libhdfs on JVM provides more HDFS client configuration which critical on production sites comparing to https://github.com/Kimahriman/hdfs-native?tab=readme-ov-file#supported-hdfs-settings where it is hard to configure cluster network configuration, namenode retries, etc.

If there's custom settings you rely on please file an issue! We've been using this internally a decent amount and haven't seen many issues at all.

Also curious how Comet handles custom Hadoop configs for accessing object stores (i.e. fs.s3a.*) type settings on the native path.

For now libhdfs on JVM provides more HDFS client configuration which critical on production sites comparing to https://github.com/Kimahriman/hdfs-native?tab=readme-ov-file#supported-hdfs-settings where it is hard to configure cluster network configuration, namenode retries, etc.

If there's custom settings you rely on please file an issue! We've been using this internally a decent amount and haven't seen many issues at all..

Thats a nice point, I would hope the crate matures as fast as possible and it is good idea to create feature requests, I'll do it.

I followed up on some comments, otherwise LGTM

For now libhdfs on JVM provides more HDFS client configuration which critical on production sites comparing to https://github.com/Kimahriman/hdfs-native?tab=readme-ov-file#supported-hdfs-settings where it is hard to configure cluster network configuration, namenode retries, etc.

If there's custom settings you rely on please file an issue! We've been using this internally a decent amount and haven't seen many issues at all.

Also curious how Comet handles custom Hadoop configs for accessing object stores (i.e. fs.s3a.*) type settings on the native path.

All of Comet's hdfs interaction is in the Parquet FileReader which is in Java. We simply use the hadoop client to access any hdfs compliant store including those accessed via hadoop-aws. Since Spark recognizes all hadoop configs and passes them on to the FileReader, we are able to access anything a Java application can.
Doing all this natively is not going to be easy so for the next phase where we are moving more of our parquet code to native, we will have a hook back to the jvm class when we cannot access hdfs or an object store natively.

Doing all this natively is not going to be easy so for the next phase where we are moving more of our parquet code to native, we will have a hook back to the jvm class when we cannot access hdfs or an object store natively.

Yeah that's all I was referring to, was wondering how Hadoop configs would get translated to datafusion scan/object store configs. I thought maybe the whole thing would still go through the JNI for hadoop file system interactions. I guess you could actually use the JNI based object store in this PR for any Hadoop file system

A small question that is it possible to use this crate to access other hadoop compatible protocol filesystem? (the libhdfs.so can do this)

Doing all this natively is not going to be easy so for the next phase where we are moving more of our parquet code to native, we will have a hook back to the jvm class when we cannot access hdfs or an object store natively.

Yeah that's all I was referring to, was wondering how Hadoop configs would get translated to datafusion scan/object store configs. I thought maybe the whole thing would still go through the JNI for hadoop file system interactions. I guess you could actually use the JNI based object store in this PR for any Hadoop file system

Right now, Comet is losing the configs passed in to hadoop which is something we will address (soon?). Converting those configs to Datafusion configs will have to be done as and when we encounter them. To start with, the configs from hadoop-aws would probably be the first to be handled. I haven't dug any deeper into this yet.

@kazuyukitanimura @parthchandra @andygrove appreciate if you can have another look?

Thanks everyone

Hey, is there a reason why not using hdfs-sys or hdrs? it looks like it support many more versions of hdfs (versions 2.2 to 3.3) in a single crate instead of 2

https://github.com/Xuanwo/hdfs-sys/blob/main/Cargo.toml

thanks @rluvaton
2 and 3 are main HDFS versions afaik
hdfs-sys crate does not seem to be maintained for 2 years and there is no object_store implementation.

The best alternative I can see now is https://github.com/datafusion-contrib/hdfs-native-object-store when the hdfs client parameter support has been extended