nsivabalan opened a new pull request, #19508: URL: https://github.com/apache/hudi/pull/19508
### Change Logs > **Stacked on #19045.** This branch is based on `mdt_layout_spi`, so the diff against `master` also > contains that PR's 8 commits (the `HoodieMetadataTableLayout` SPI, `FlatMDTLayout` / > `SubDirBucketedMDTLayout`, config plumbing and its tests). **The commits belonging to this PR are > the last three:** > > - `cee7287` `fix(metadata): align MDT table-service planners with the physical bucket layout` > - `a60004a` `test(metadata): pin plan-level and key-space properties for bucketed MDT services` > - `bb5bae0` `fix(metadata): close the cleaner's file system view; tighten bucketing assertions` > > Review #19045 first; it should merge before this. Follow-up to #19045, which introduced the `HoodieMetadataTableLayout` SPI and the opt-in `SubDirBucketedMDTLayout`. That PR left the MDT table services misaligned with the bucketed on-disk layout. This aligns them. **The problem.** Under a non-flat layout the MDT write path is already keyed by *physical* partition paths: `getPartitionFileSlices` expands logical → physical before querying the file system view, and `getRecordTagger` realigns each record's partition path to its file slice's physical path. The table-service planners, however, enumerate partitions from marker discovery / `getAllPartitionPaths`, which return *logical* names — the single `.hoodie_partition_metadata` lives at the logical partition root by design. That divergence is not only a compaction-execution failure. As soon as a compaction is *requested*, the FSV's pending-compaction map is keyed logically while every write-side lookup is keyed physically, so the join misses silently: - `AbstractTableFileSystemView:250` — the phantom post-compaction file slice is not added, so MDT appends land in the slice being compacted and those log blocks end up in neither the compaction input nor the resulting slice → **RLI entry loss**. - `AbstractTableFileSystemView:1606` (`fetchMergedFileSlice`) — merged reads during a pending compaction miss the map and drop pre-compaction log files → **wrong index answers**. **The fix** — expand at the planner boundaries so the plan key space matches the already-physical write side: | Site | Bug | |---|---| | `HoodieTableMetadataUtil.expandToPhysicalPartitions` | shared helper; no-op for data tables and the flat default, idempotent for already-physical input | | `BaseTableServicePlanActionExecutor.getPartitions` | covers compaction **and** log compaction (shared chokepoint) | | `CleanPlanner.getPartitionPathsForFullCleaning` | cleaner deletes log files an in-flight compaction still needs → wedged MDT | | `CleanPlanner.hasPendingFiles` | listed the logical root directly, so it always reported "no pending files"; a partition with live pending files could be dropped wholesale | | `ListingBasedRollbackStrategy` | MDT is pinned to this strategy (DIRECT markers, rollback-using-markers disabled) and the listing is non-recursive, so rollback of a failed MDT compaction found nothing to delete and "succeeded" while the orphan base file survived | Idempotency is the property that keeps the incremental branch correct: a physical sub-path such as `record_index/000003` has no entry in the persisted per-partition file-group counts, so the count resolves to 0 and the layout returns the input unchanged. Incremental table services source partitions from write stats, which are already physical. Two supporting fixes the above make load-bearing: - `FileGroupReaderBasedMergeHandle.init` now mirrors `HoodieAppendHandle`'s metafile-suppression guard. Compaction partitions are physical now, so without it compaction writes a `.hoodie_partition_metadata` inside a bucket directory — which makes partition discovery return bucket paths and breaks the cleaner and rollback globally. The marker and the write stat still derive from the same partition string, so `reconcileAgainstMarkers` cannot mistake the freshly written base file for an orphan. - `HoodieBackedTableMetadataWriter.tagRecordsWithLocation` built a `HoodieFileGroupId` from the logical partition where its streaming twin uses `fileSlice.getFileGroupId()`; it now uses the slice's own id. **Scope.** No public contract, storage format, or config change. Every path is a no-op for data tables and for MDTs on the flat default layout, which is what every pre-existing table uses. ### Tests The bucketing tests on #19045 assert on *side effects* — bucket directories survive, HFiles appear after compaction. That is too weak for this bug class: when the key spaces diverge the corruption is silent and the on-disk layout still looks healthy. The tests here assert **plan-level and key-space** properties instead: - **Compaction** (`testMDTCompactionPlansCarryPhysicalPartitions`) — every operation in every persisted plan targets a bucket sub-path and never the logical partition root; compacted buckets are a subset of what the layout enumerates; compaction spans more than one bucket at `bucketSize=2` (so the fan-out is genuinely exercised); the marker invariant still holds afterwards. - **Cleaning** (`testMDTCleaningUnderBucketingWithFullCleaning`) — runs with incremental cleaning **disabled**, forcing the full-listing path that enumerates logical names and joins them against a physically-keyed pending-compaction map. Asserts cleaning and compaction both fired (so they overlapped on the same buckets) and that no bucket was stripped of its data files. - **Rollback** (`testRollbackEnumeratesPhysicalBucketPartitions`) — a non-recursive listing of each enumerated partition must find data files, mirroring exactly what `ListingBasedRollbackStrategy` does. This is the silent no-op. - **Unit** (`TestHoodieMetadataTableLayout`) — fan-out, idempotency for already-physical input, de-duplication of mixed input, flat-layout and non-MDT passthrough, uncounted-partition fallback, and single-bucket partitions. Two things the runs corrected in my own initial assumptions, both now encoded in tests: `SubDirBucketedMDTLayout` buckets **every** MDT partition rather than only the RLI (a single-file-group partition such as `files` lives at `files/000000`), which means the rollback listing gap applies beyond the RLI; and a completed MDT compaction appears on the timeline as a `COMMIT_ACTION`, with its plan still readable at the same instant time. Local results: `TestMDTLayoutBucketing` 6/6, `TestHoodieMetadataTableLayout` 28/28, `TestCleanPlanner` + rollback suites 75/75, `TestHoodieTableMetadataUtil` + `TestMetadataPartitionType` 47/47. ### Impact Makes MDT compaction, log compaction, cleaning and rollback correct under `SubDirBucketedMDTLayout`. No behavior change for the flat default layout or for data tables. ### Risk level: medium Touches shared table-service planner code. The risk is contained by the layout gate — `expandToPhysicalPartitions` returns its input unchanged unless the table is an MDT on a non-flat layout — so data-table and flat-MDT paths are bit-identical. `TestCleanPlanner` and the rollback suites (75 tests) pass unchanged, which is the main regression signal for the shared code. One residual worth stating plainly: `expandToPhysicalPartitions` is now load-bearing for the two FSV windows above. A future planner boundary that forgets the helper reintroduces a silent failure. The tests below pin the current boundaries, but the structural fix for that class of risk is the sibling-partition layout, which removes the logical/physical split on the service path rather than defending it. That is a larger redesign and belongs in its own RFC. ### Documentation Update None needed — no user-facing config or contract change. ### Contributor's checklist - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute) - [x] Change Logs and Impact were stated clearly - [x] Adequate tests were added if applicable - [ ] CI passed (draft — awaiting CI) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
