comphead opened a new pull request, #3515:
URL: https://github.com/apache/iceberg-rust/pull/3515

   ## Which issue does this PR close?
   
   - Part of #1130.
   
   ## What changes are included in this PR?
   
   #1130 settled on shipping both HDFS clients and letting users choose 
([consensus](https://github.com/apache/iceberg-rust/issues/1130#issuecomment-2761389382)):
 "Libhdfs is still much more mature than hdfsrs so in serious hadoop deployment 
it's still useful." #3111 added the `hdfs-native` half. libhdfs was set aside 
then to avoid the Java runtime 
([comment](https://github.com/apache/iceberg-rust/issues/1130#issuecomment-2757068697)),
 and @oven-yang reported it working through OpenDAL 
([comment](https://github.com/apache/iceberg-rust/issues/1130#issuecomment-2757182202)).
 This PR adds the libhdfs half.
   
   It replaces only the transport, and it plugs into `hdfs://` paths only:
   
   - A new `opendal-hdfs` feature, OpenDAL's `services-hdfs` (libhdfs over 
JNI), with `OpenDalStorageFactory::Hdfs` and 
`OpenDalStorage::Hdfs(HdfsStorage)`. It is experimental and not part of 
`opendal-all`.
   - The HDFS layer from #3111 moves unchanged from `hdfs_native.rs` to 
`hdfs_common.rs`: path parsing, NameNode normalization, 
`hdfs.host`/`hdfs.port`, error redaction and the per-NameNode operator cache. 
Both backends use it, so they accept the same paths and key operators the same 
way. `hdfs_native.rs` only loses the moved code. Its behavior and tests are 
unchanged.
   - What differs is what Hadoop's Java client does itself. A path authority 
reaches it as written, so `hdfs://ns1/...` resolves through the nameservice in 
the Hadoop configuration on the classpath, with HA failover, Kerberos and 
encryption zones, as in Hadoop. An authority-less path uses `hdfs.name-node` 
(one NameNode), else `hdfs.host`/`hdfs.port`, else Hadoop's `fs.defaultFS`.
   - PyIceberg's `hdfs.user` and `hdfs.kerberos_ticket`, which the 
`hdfs-native` backend ignores, select libhdfs's user and Kerberos ticket cache. 
They become the constants `HDFS_USER` and `HDFS_KERBEROS_TICKET` in 
`iceberg::io`. The private config module that holds the HDFS constants moves 
from `hdfs_native.rs` to `hdfs.rs`, and their docs now cover both backends.
   - `hadoop.*` and `hdfs.name-node.<nameservice>` are ignored, with one 
warning, because OpenDAL's libhdfs service takes no client configuration.
   - `OpenDalResolvingStorage` serves `hdfs://` with libhdfs only when it is 
the one HDFS feature built. With both features, `hdfs-native` keeps serving it, 
and `OpenDalStorageFactory::Hdfs` selects libhdfs. A property to choose at 
runtime could follow.
   
   Beyond maturity, there is an authentication reason for having both. 
OpenDAL's `services-hdfs-native` builds every client for the synthetic 
nameservice `nameservice` (`init_hdfs_config`), and `hdfs-native` picks a 
delegation token by `ha-hdfs:<nameservice>`. It therefore looks for 
`ha-hdfs:nameservice` and never finds the tokens a cluster issues, which are 
named `ha-hdfs:<nameservice>` or after a NameNode address. Spark executors and 
YARN containers authenticate with exactly those tokens. libhdfs uses the JVM's 
credentials and the real authority. I found this while wiring #3111 into 
DataFusion Comet, and checked it against `opendal-service-hdfs-native` 0.58.2 
and `hdfs-native` 0.14.7.
   
   ### Costs
   
   - Building with the feature needs a JDK: `hdfs-sys` compiles libhdfs's C 
sources and links `libjvm`, which it finds through `JAVA_HOME` or `java` on 
`PATH`. With `--all-features`, that applies to `make build`, `make 
check-clippy` and the CI build matrix. GitHub-hosted runners ship a default JDK 
with `JAVA_HOME` set.
   - `hdfs-sys` adds the JVM to the rpath with `cargo:rustc-link-arg`, which 
Cargo does not pass to dependent crates. Every test binary that links the 
feature therefore needs `$JAVA_HOME/lib/server` on the library path 
(`LD_LIBRARY_PATH`, or `DYLD_LIBRARY_PATH` on macOS). The tests job now sets 
it. Local `--all-features` test runs need the same.
   - At run time, libhdfs starts a JVM from `CLASSPATH` unless the process 
already runs one, as a Spark executor does. Building an operator connects 
synchronously, once per NameNode per storage: it attaches the calling thread to 
the JVM and stats the root.
   - libhdfs detaches a thread from the JVM when the thread exits, whoever 
attached it ([HDFS-16021](https://issues.apache.org/jira/browse/HDFS-16021)). 
That matters to hosts that attach their own threads, such as a native library 
inside a JVM.
   - New dependencies, all Apache-2.0 or MIT/Apache-2.0: `hdrs`, `hdfs-sys`, 
`java-locator`, `opendal-service-hdfs`, and `blocking` with its small async 
helpers.
   
   ## Are these changes tested?
   
   - Unit tests in `hdfs.rs`:
     - config parsing: one NameNode, nameservice names, comma lists rejected, 
`hdfs.host`/`hdfs.port`, user and ticket, the ignored-keys warning, redaction
     - NameNode resolution, batch keys, `relativize_path`, and the serde round 
trip
     - the resolving storage's precedence
   - The shared parser's tests moved with it to `hdfs_common.rs`. All HDFS unit 
tests pass.
   - Integration tests in `tests/file_io_libhdfs_test.rs` (12 tests) run 
against the existing HDFS fixture and mirror the `hdfs-native` suite:
     - exists, write and read, overwrite, metadata, range reads, streaming 
writes
     - `delete`, `delete_prefix`, `delete_stream`
     - authority-less paths through all three sources
     - a nameservice that only the Hadoop configuration declares 
(`dev/hdfs/client/hdfs-site.xml`)
   - The integration tests skip when `CLASSPATH` is unset. CI copies Hadoop's 
jars from the fixture's `apache/hadoop:3.5.0` image and sets `CLASSPATH`, 
`LIBHDFS_OPTS` and `LD_LIBRARY_PATH`.
   - I could not run the integration suite locally, since I have no Docker 
here. The same `FileIO` calls did run end to end through libhdfs against Hadoop 
3.5.0's local filesystem (`fs.defaultFS=file:///`), on macOS with JDK 17.
   - Run locally and passing: `make check-clippy`, `make check-doc`, `cargo 
fmt`, `taplo fmt --check` and `cargo machete`. `public-api.txt` is regenerated 
for both crates.
   - Not run locally: `cargo deny` and the Windows build, which compiles 
libhdfs's C sources with MSVC.
   
   ## AI Disclosure
   
   Developed with AI assistance (Claude Code). It drafted the code, tests, CI 
step and this description, and checked the OpenDAL, hdrs, hdfs-sys and 
hdfs-native sources. I reviewed the changes.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to