perf(deletes): Implement filtering position delete files by file path - #2936
perf(deletes): Implement filtering position delete files by file path#2936brgr-s wants to merge 8 commits into
Conversation
03347ff to
d6abcca
Compare
d6abcca to
8008ece
Compare
c-thiel
left a comment
There was a problem hiding this comment.
I left a few comments. I also see think we have at least two remaining efficiency gaps vs Java for which we should open Issues or directly follow-up PRs:
1. Equality deletes are not pruned by column statistics
Java's canContainEqDeletesForFile skips an equality delete file when the data file's and delete file's bounds for the equality fields don't overlap, or when null counts prove no rows can match. PopulatedDeleteFileIndex::get_deletes_for_data_file only applies the
sequence-number check, so every equality delete in the partition is returned, then loaded and applied.
2. Buckets are linearly scanned instead of binary-searched
Java keeps each bucket sorted by data sequence number and binary-searches the first applicable delete (findStartIndex), giving O(log n + matches). Rust filters linearly per data file.
not "never applied" but "skipped for data files with known sequence number" Co-authored-by: Christian <Christian.Thiel@outlook.com>
Co-authored-by: Christian <Christian.Thiel@outlook.com>
|
@c-thiel thanks for the review, changes are incoming. |
Which issue does this PR close?
What changes are included in this PR?
The PR introduces (uncomments)
pos_deletes_by_pathinPopulatedDeleteFileIndexand populates theHashMap. To determine if a position delete file belongs in the map, it uses a helperposition_delete_targetthat investigatesreferenced_data_fileset? if yes, we know the delete file applies to that pathlower == upperfor the reserved fieldRESERVED_FIELD_ID_DELETE_FILE_PATH? If yes, we know that all entries point to the same pathAs a small ergonomic imrpovement it changes
eq_deletes_by_partitionandpod_deletes_by_partitionto take a(i32, Struct)as key, where thei32is thepartition_spec_idof the delete file.get_deletes_for_data_filecan then usepos_deletes_by_pathto lookup delete files that match that specificdata_file.PR also adds a debug trace that could be usefull to determine if a read on a table is slow or OOMs (see linked issue) due to a large number of created delete file references.
Are these changes tested?
I have added unit tests.
The change was also tested locally in conjuncture with PR #2620. The task was compacting a merge-on-read table with a huge number of delete files. The test scenario can be reproduced without PR #2620, see Issue #2935.
AI Disclosure
I used AI to investigate how Spark handles delete files, to review my changes and add more tests. I also used AI to write parts of the Issue and double-check the math.