apache / apache/hudi

Follow-ups for RLI and SI over key-less external parquet files

Open
#19,886 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
6.2k
Forks
2.5k
Avg merge
2d 8h
Merged PRs (30d)
111

Description

### Describe the problem

#19869 adds record index (RLI) and secondary index (SI) support for parquet files written outside Hudi and registered through replace commits without a known operation type. Such a table has no record key, so every row is keyed by file path and row position. Five items were left out of that PR.

`INSERT_OVERWRITE`, `INSERT_OVERWRITE_TABLE` and `DELETE_PARTITION` still assume Hudi-written files:

1. `HoodieAvroParquetReader.getRecordKeyIterator` reads `_hoodie_record_key` with no null guard, so a key-less file under these operation types throws an NPE. `BaseRecordIndexer.getRecordIndexAdditionalUpserts` reaches it through `HoodieTableMetadataUtil.readRecordKeysFromBaseFiles`. Fix: give it the positional-key fallback of `ParquetUtils`, gated on `HoodieTableConfig.hasRecordKey()`.
2. `HoodieTableMetadataUtil.readRecordKeysFromBaseFiles` builds the data file path from `baseFile.getFileName()`, which drops the directory prefix of a file registered below a subdirectory of its partition, for example `bucket-0/file.parquet`. Fix: use `HoodieBaseFile.getStoragePath()`, as `SecondaryIndexRecordGenerationUtils.readSecondaryKeysFromFileSlices` now does.
3. Neither is covered on a key-less table. Fix: add the three operation types with a prefixed file to `TestExternalFileRecordAndSecondaryIndex`. `TestExternalPathHandling` commits with `INSERT_OVERWRITE`, so the in-repo flow takes these branches; XTable sets `UNKNOWN`, which #19869 covers.

Two more items came out of the review of #19869:

4. Async index catch-up over registration commits is not covered. `RunIndexActionExecutor` builds the catch-up writer on a timeline where the replace commit is already complete, so `AbstractTableFileSystemView.resetFileGroupsReplaced` hides the replaced groups and both `getLatestBaseFile` and `getLatestMergedFileSliceBeforeOrOn` return empty. A replayed registration commit then deletes nothing and the replaced groups keep their RLI and SI entries. The sync path is unaffected, and the `INSERT_OVERWRITE` arm has the same gap on master. The `TimelineUtils.getCommitMetadata` change in #19869 only lets the catch-up tasks read the operation type. Fix: fall back to `fsView.getReplacedFileGroupsBeforeOrOn(instantTime, partition)` on both lookups, with a functional test over a completed external-file replace commit.
5. The key-less update path materializes every key of a base file three to four times in one task: `ParquetUtils.filterRowKeys` collects a `Set`, `FileFormatUtils.readRowKeys` maps it to a `Set`, the `prevFileName == null` branch of `BaseFileRecordParsingUtils` copies it into an `ArrayList`, and the caller builds the record list. Initialize streams the same data through `getRowKeyIterator`. Registered files are conversion output, typically 10^7 rows at 50-70 chars per key. No wrong results. Fix: stream both branches through `getRowKeyIterator` and keep the set only for the previous-file diff.

### Context

Review threads: [r3965129565](https://github.com/apache/hudi/pull/19869#discussion_r3965129565), [r3987612759](https://github.com/apache/hudi/pull/19869#discussion_r3987612759), [r3987612751](https://github.com/apache/hudi/pull/19869#discussion_r3987612751)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with HoodieAvroParquetReader.getRecordKeyIterator, HoodieTableMetadataUtil.readRecordKeysFromBaseFiles, BaseRecordParsingUtils, and RunIndexActionExecutor, then inspect TestExternalFileRecordAndSecondaryIndex and TestExternalPathHandling. Trace the key-less external-file paths and completed replace-commit catch-up behavior before changing the specified fallbacks and streaming path. Done means the listed operation types, prefixed files, async catch-up, and large key-less updates are covered by functional tests without incorrect index entries or failures.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
data-engineering
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.