Follow-ups for RLI and SI over key-less external parquet files
- 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