Repository navigation
Conversation
The bloom index key check resolved each candidate base file through the table view twice per task (two timeline-server getLatestBaseFile requests on executors), and the bloom, SIMPLE and key-check reads stat every base file before opening it although its length is already known. The driver-resolved base files now ride along with BloomIndexFileInfo to the key-check stage, and the footer, bloom filter and key reads open parquet files from their known length. TestHoodieBloomIndex#testKeyCheckReadsResolvedBaseFiles and TestHoodieKeyLocationFetchHandle assert zero timeline-server base-file requests and no extra file-status calls, and TestParquetUtils covers the length-aware overloads.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #20263 +/- ##
============================================
- Coverage 80.62% 78.77% -1.86%
+ Complexity 35055 34297 -758
============================================
Files 2552 2552
Lines 143400 143476 +76
Branches 17454 17458 +4
============================================
- Hits 115615 113019 -2596
- Misses 19867 22420 +2553
- Partials 7918 8037 +119
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the issue this Pull Request addresses
closes #20262
part of #20257
The bloom index key check resolves each candidate base file through the file system view twice per handle, which on executors means two timeline-server
getLatestBaseFilerequests per (task, candidate base file), although the driver already resolved those base files when it loaded the key ranges. The bloom, key-check and SIMPLE index reads also look up each parquet file's status before opening it, although its size is known.The two lookups per handle date from #726 (0.5.0), which resolved the base file through the table view in both the
HoodieKeyLookupHandleconstructor andgetLookupResult; they became timeline-server requests when the remote view became the default (#1634, 0.5.3). #6793 (HUDI-4917, 0.13.0) already applied "the driver has the base file" to the range-info stage but not to the key check.Summary and Changelog
The key-check stage now reads the base files the driver resolved instead of asking the view for them, and parquet footer and key reads skip the status lookup when the file length is known.
BloomIndexFileInfocarries the resolvedHoodieBaseFile(not part of equality); the loaders set it, and both bloom index helpers pass a file-group-to-base-file lookup (a broadcast on Spark) toHoodieBloomIndexCheckFunction.ListBasedIndexFileFilterkeeps only the key ranges, so the base files are not shipped with the record-to-file pairing tasks.HoodieKeyLookupHandletakes an optional base file and falls back to a single view lookup when it is absent.FileFormatUtilsgainsStoragePathInfooverloads for footer, bloom filter, min/max key, key filter and key iterator reads;ParquetUtilsopens the file from its known length (with the read options ofwithHadoopReadOptions), and the Avro and Spark parquet readers and factories use them.BaseFile#getPathInfoWithKnownSizefeeds them from the read handle,HoodieIndexUtils.filterKeysFromFileandHoodieKeyLocationFetchHandle.HoodieFileReaderFactory.getFileReader(config, pathInfo, PARQUET, ...)now builds the known-length reader for its existing callers too (e.g.HoodieAvroReaderContext); only the footer, bloom filter and key reads use the length.TestHoodieBloomIndex#testKeyCheckReadsResolvedBaseFiles(BLOOM and GLOBAL_BLOOM, 9 helper and metadata configurations) asserts no timeline-server base-file request and no parquet status call during tagging, and that locations match the SIMPLE index; it fails on master.TestHoodieKeyLocationFetchHandleasserts the status calls, andTestParquetUtilscovers the new overloads.Impact
Fewer executor round trips for BLOOM, GLOBAL_BLOOM and SIMPLE index lookups, with no change to results. On a local Spark harness, executor timeline-server requests from the key check go from 64 to 0 (256 to 0 at 4x file groups per partition), and executor base-file status calls go from 152 to 80 (352 to 80 at 4x file groups); the key check makes none during tagging, and the remaining 80 come from other paths (column stats footer reads, the merge handle and write-stat sizes). On S3A,
fs.open()still issues a HEAD because parquet'sHadoopInputFile.newStreamdoes not pass the status, so S3A HEADs per candidate file drop from 2 to 1.HoodieKeyLookupHandle,HoodieBloomIndexCheckFunctionandHoodieSparkBloomIndexCheckFunctionconstructors change (internal classes). This overlaps #20095 inHoodieKeyLocationFetchHandle.fetchRecordKeysWithPositions, a trivial conflict whichever lands second.Risk Level
low: the base file comes from the same driver-side view the range stage already uses, and a wrong recorded file size fails the footer read loudly rather than returning wrong data.
Documentation Update
none
Contributor's checklist