Skip to content

fix(metadata-table): follow-ups for the zero-size file skip (#18611) - #19355

Merged
voonhous merged 2 commits into
apache:masterfrom
voonhous:follow-up-18611-zero-size-skip
Jul 23, 2026
Merged

fix(metadata-table): follow-ups for the zero-size file skip (#18611)#19355
voonhous merged 2 commits into
apache:masterfrom
voonhous:follow-up-18611-zero-size-skip

Conversation

@voonhous

@voonhous voonhous commented Jul 23, 2026

Copy link
Copy Markdown
Member

Describe the issue this Pull Request addresses

Follow-up to #18611, which added the opt-in config hoodie.metadata.skip.zero.size.files.on.initialize (skip zero-size data files while listing the data table during MDT initialization). #19243 was a duplicate submission of the same branch and collected review comments that apply to the merged code; it is now closed. This PR addresses those comments and carries over the E2E test from #19243 that never merged (cherry-picked with authorship preserved, thanks @lokeshj1703 @nada-attia).

Summary and Changelog

  • Honor the config in the restore-sync relisting. Previously update(HoodieRestoreMetadata, ...) passed skipZeroSizeFiles=false unconditionally, so a restore would diff the filesystem against MDT, see the files skipped at initialization as FS-only, and re-add them via partitionFilesToAdd -- re-introducing the failure the config exists to avoid (e.g. the column-stats restore build reading a zero-size parquet footer). With the config enabled, restore now skips them too, and zero-size files already tracked in MDT are removed by the restore sync, converging MDT to the configured semantics.
  • Document the operational trade-off in the config description: skipped files remain on storage and are not tracked by the metadata table or the cleaner; they must be removed manually, and the metadata validator will report them as inconsistencies.
  • Correct the config sinceVersion to 1.3.0 (it first ships in 1.3.0, not 1.2.0).
  • Move the metric name into HoodieMetadataMetrics.SKIPPED_ZERO_SIZE_FILES_ON_INITIALIZE_STR alongside the other metric-name constants, and only emit it when files were actually skipped (previously an unconditional 0 was emitted on every listing, including restore).
  • Reduce log noise: one aggregated WARN per partition with the skipped count; individual file paths moved to DEBUG.
  • Add a Scala E2E test (TestRecordLevelIndex.testUpsertAfterSkippingZeroSizeFileOnInitialize): after a zero-size base file is skipped during MDT bootstrap with global RLI, an upsert of the affected records succeeds, routes them to a new file group (never the zero-size one), and leaves RLI consistent with the data.

Impact

None by default. With the config enabled, restore-sync now treats zero-size files the same way initialization does instead of re-adding them to MDT. The skipped_zero_size_files_on_initialize metric is now only emitted when non-zero.

Risk Level

low. All behavior changes are gated behind the default-off config; the metric/logging changes are observability-only.

Documentation Update

None beyond the updated config description (the website config reference is generated from it).

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

lokeshj1703 and others added 2 commits July 23, 2026 11:44
…le on MDT initialize

Adds testUpsertAfterSkippingZeroSizeFileOnInitialize to TestRecordLevelIndex
to cover the scenario where SKIP_ZERO_SIZE_FILES_ON_INITIALIZE is enabled:
- corrupts the sole base file to zero-size and forces MDT rebootstrap
- verifies upsert succeeds and writes to a new file group (not the zero-size one)
- validates RLI completeness and that the zero-size fileId is absent from MDT

Uses GLOBAL_RECORD_LEVEL_INDEX so bootstrap always creates the minimum 10 file
groups for record_index even when all data files were skipped as zero-size.
… skip

- honor hoodie.metadata.skip.zero.size.files.on.initialize in the
  restore-sync relisting so restore does not re-add files that were
  skipped at initialization
- document that skipped files remain on storage untracked by MDT and
  the cleaner, and must be removed manually
- correct the config sinceVersion to 1.3.0
- move the metric name into HoodieMetadataMetrics and only emit it
  when files were actually skipped
- aggregate the per-file WARN into one WARN per partition, with the
  individual paths at DEBUG
@voonhous
voonhous force-pushed the follow-up-18611-zero-size-skip branch from 2cdb883 to c2767ce Compare July 23, 2026 03:54
@voonhous
voonhous requested a review from danny0405 July 23, 2026 03:57
@github-actions github-actions Bot added the size:M PR with lines of changes in (100, 300] label Jul 23, 2026
@hudi-bot

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@wombatu-kun

Copy link
Copy Markdown
Contributor

The test-spark-java-tests-part1 (spark3.3) failure here is the known flaky TestSparkFilterHelper.testConvertInExpression (fails with LiveListenerBus is stopped), a pre-existing test-session lifecycle issue unrelated to this PR's changes. The fix is already up and approved in #19356, so re-running that single job should bring CI green.

@voonhous
voonhous enabled auto-merge (squash) July 23, 2026 07:14
@voonhous
voonhous merged commit 5f19cf3 into apache:master Jul 23, 2026
142 of 144 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M PR with lines of changes in (100, 300]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants