[I] Consolidate the native bindings around one C ABI [hudi-rs]

["yihua (via GitHub)" <gi...@apache.org>]

yihua opened a new issue, #848:
URL: https://github.com/apache/hudi-rs/issues/848

   hudi-rs now exposes its reader through four non-Rust interfaces, and new reader features increasingly have to be wired through each of them. I'd like to agree on a direction for 0.7.0 and beyond so binding work converges on one place.
   
   Nothing here blocks current work: open PRs against any of the bindings can go in as they are in the interim.
   
   ## Where we are
   
   | Interface | Crate | Consumers | Published |
   |---|---|---|---|
   | Python (PyO3) | `python/` | PyPI wheel | yes |
   | DataFusion FFI | `python/` | `datafusion-python` | through the wheel |
   | C++ (cxx) | `cpp/` (`hudi-cpp`) | demo app | no |
   | C ABI | `crates/jvm-ffi` (`hudi-ffi` after #847) | JVM through JNA; C++ engines with #834 | no |
   
   The same concerns are being solved once per binding:
   
   - Filters: Python takes `(field, op, value)` tuples, #767 adds `HudiFilter` to the cxx bridge, and #834 adds `HudiFfiFilter` to the C ABI.
   - Runtimes: the cxx bridge creates a current-thread Tokio runtime per reader, #834 moves the C ABI to one process-wide runtime, and Python has its own (#829).
   - Panic guards landed separately: #700 for the C ABI, #766 for the cxx bridge.
   - Read options are parsed and documented per binding, and they already differ in which keys apply per read.
   
   ## Proposal
   
   1. Keep Python as the high-level binding. PyO3 is the right tool there.
   2. Make the C ABI (`hudi-ffi`) the one native boundary for every other caller: JVMs, C++ engines like DuckDB, and anything else with a C FFI. Plain C plus the Arrow C Data Interface has no C++ toolchain coupling, which matters for engines built across many platforms, and a JVM can bind it where it cannot bind cxx.
   3. Stop maintaining `hudi-cpp` as a second implementation of the reader bindings. Two ways to get there:
      - Deprecate and remove it; C++ callers use the C ABI directly.
      - Keep a C++ API with the same entry points (`new_file_group_reader_with_options`, `read_file_slice`, ...), but as a thin header-only wrapper over the C ABI (RAII handles, exceptions raised from `hudi_ffi_last_error`). Existing C++ callers keep working with small changes, such as cxx types like `rust::Box` becoming plain C++ types, while hudi-rs maintains only the C ABI behind it.
   4. Move shared parsing into `hudi-core` (filter tuples already live there; `key=value` options could join them) so each binding only converts types.
   
   ## Rough timeline
   
   - 0.7.0: publish `hudi-ffi` with a C header and a stated ABI compatibility policy; mark `hudi-cpp` deprecated.
   - 1.0.0: remove `hudi-cpp`, or ship it only as the wrapper over the C ABI.
   
   ## Open questions
   
   - Who uses `hudi-cpp` today, and would the C ABI cover those cases? @codope, you've been closest to the cxx bridge.
   - @linliu-code, does the Gluten work load the library by name, and is JNA still the plan for the JVM side?
   - Header: hand-written, or generated with `cbindgen`?
   - Distribution: crates.io only, or prebuilt shared libraries per platform?
   
   Comments welcome. If there's agreement, I'll summarize the outcome on dev@ so the decision is recorded there too.
   


-- 
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: commits-unsubscribe@hudi.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Consolidate the native bindings around one C ABI [hudi-rs]

["codope (via GitHub)" <gi...@apache.org>]

codope commented on issue #848:
URL: https://github.com/apache/hudi-rs/issues/848#issuecomment-6531047845

   Thanks @yihua, +1 on making the C ABI the one native boundary.
   
   ### Who uses `hudi-cpp` today
   
   As far as I know, only the in-repo demo. It isn't published, and nothing depends on cxx-specific behavior, so deprecating it in 0.7.0 costs little. I'd keep a thin header-only C++ wrapper over the C ABI rather than drop C++ entirely: engines end up writing RAII handles and exception translation anyway, and one canonical header is better than each engine writing its own.
   
   The C++ engine that would have mattered most has gone another way for now. The native Hudi MOR effort for Presto C++ (facebookincubator/velox#18530, prestodb/rfcs#66) proposes a pure C++ connector in Velox. RFC-0066 considered hudi-rs via C FFI and set it aside for three reasons:
   
   1. a Rust toolchain in the Velox/Prestissimo build
   2. buffers allocated outside Velox's `MemoryPool`, invisible to its arbitrator
   3. a Tokio runtime inside a synchronous reader path
   
   The RFC keeps its connector layer independent of the reader, so hudi-rs could be swapped in later. I think those three points are a good bar for `hudi-ffi`. cc: @jja725
   
   ### What a Velox-class engine needs
   
   In engines like Prestissimo and Spark on Gluten+Velox, a JVM coordinator plans and native workers read splits. The per-row read path wants to stay inside the engine. Its own Parquet reader pushes filters into decoding, loads columns lazily, keeps dictionary encoding and uses the engine's cache. And Arrow batches don't import into Velox for free, since strings, timestamps and nested types are laid out differently. So the most useful C ABI pieces for these engines are:
   
   1. **Plan-only (your point 5).** I'd rank it higher. It lets the engine read base-file-only slices natively, which is most of the bytes for most tables.
   2. **Log-decode-only.** Return a slice's log records, with positions and delete markers when the blocks carry them, and do no merge, so the engine can apply them over its own base-file read.
   3. **Host-supplied limits at open.** A memory budget, the runtime's thread count, and bounded batch sizes. Together with plan-only, that addresses objections (2) and (3).
   
   Two smaller points:
   
   - #767 found that a per-read time window was silently ignored, because it's applied from the reader's configs. It would help if the C ABI said, per option, whether it applies at open, at plan or per read.
   - RFC-0066 plans golden fixtures from Hudi Java as its correctness oracle, and hudi-rs already checks its fixtures against Spark gold snapshots. Sharing one fixture set across hudi-rs and Velox would keep the readers from drifting.
   
   Also, I think #830 (byte-range split reads) should target the C ABI rather than the cxx bridge.
   


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org