nsivabalan opened a new pull request, #20246: URL: https://github.com/apache/hudi/pull/20246
### Describe the issue this Pull Request addresses **This is a WIP draft and is intentionally red.** It adds no production change. It adds a functional suite that reproduces two consistency gaps in the async indexer, so that a fix has something to prove itself against. Two of the seven tests fail on master by design; they are the bugs. Context is @yihua's analysis on #19802 (https://github.com/apache/hudi/pull/19802#discussion_r4213531275): the async indexer implementation has drifted from RFC-45, and the drift lets a concurrent writer's commit go unindexed into a newly built metadata partition. Two root causes, both confirmed in code: 1. **The writer's coordination signal is written outside any lock.** `buildMetadataPartitions` sets `hoodie.table.metadata.partitions.inflight` at `HoodieBackedTableMetadataWriter#839`, reached from `RunIndexActionExecutor#149`. Scheduling is locked (`BaseHoodieWriteClient#1209`), but `index()` at `#1226` takes no lock at all. Regular writers choose which metadata partitions to update by reading that same config (`HoodieBackedTableMetadataWriter#802-809`) inside their own commit lock — but mutual exclusion requires both sides to participate. 2. **Catch-up verifies that a metadata deltacommit exists, not what it contains.** `AbstractIndexingCatchupTask#run` skips an instant on `metadataInstant.isPresent()` alone. A deltacommit written by a writer that only knew about `files` is indistinguishable from one that wrote the new partition. RFC-45 designates catch-up as the correction for exactly this race: *"the inflight writer may not see an indexing request that was just added and proceed to commit without that. We will correct this during indexing action completion."* Its prose requires catch-up to verify that each completed instant "added entries per its indexing plan". The implementation kept the step-by-step shorthand ("has corresponding deltacommit then continue"), which is only equivalent when the writer obligation always holds — which is what TOCTOU breaks. So the mechanism assigned to correct the race is the one waving it through. ### Summary and Changelog Adds `TestAsyncIndexerConcurrency` (hudi-utilities), seven functional tests that hold a regular writer and the indexer in pinned interleavings using `CountDownLatch` pairs rather than relying on timing. Every test asserts on **index content** rather than on the presence of a metadata deltacommit, since the deltacommit's existence is the assumption under test. | Test | On master | |---|---| | Commit between schedule and execute | passes | | **Commit straddling the start of the run** | **fails — 50/50 record keys lost** | | **Files-only deltacommit not treated as indexed** | **fails — 3/3 base files have no column stats** | | Instant inflight at schedule time | passes | | Out-of-order completion | passes | | Concurrent table-service scheduling | passes | | Partition marked complete after a successful run | passes | The two failures: ``` testCommitOverlappingRunStartIsIndexed keys from a commit overlapping the indexer run are missing from the record index: 50 of 50 keys testFilesOnlyDeltacommitIsNotTreatedAsIndexed column stats are missing for files written by a commit whose metadata deltacommit predates the new partition; catch-up accepted deltacommit existence as proof of indexing: [(2016/03/15, 9aa65ec1-...parquet), (2015/03/16, 98e14227-...parquet), (2015/03/17, 32f76d7c-...parquet)] ``` Both are deterministic across 4/4 runs. Worth noting that column stats fail the same way RLI does, not the milder way. The straddling commit loses every entry, not just some, so the data-skipping exposure is wider than the "a commit between schedule and execute is missed" framing suggests. **Two things these tests rule out**, which narrows where the problem lives: - *Index scheduling vs. table-service scheduling is safe.* Both take the same `txnManager` state-change lock around plan-and-persist (`BaseHoodieTableServiceClient#739`, `BaseHoodieWriteClient#1209`), so they serialize. Either ordering leaves the table-service instant in the catch-up range. - *Inflight-at-schedule handling is correct in principle.* `getContiguousCompletedWriteTimeline` (`BaseHoodieTimeline#206-213`) cuts the plan's base instant below the earliest pending instant, so such an instant is deliberately excluded from bootstrap and delegated to catch-up, matching the RFC. So the base-instant selection and the no-holes rule work as designed. Everything converges on catch-up's verification. Separately, in the same area: `getInstantsToCatchup` computes the catch-up range with `findInstantsBefore`, which compares requested time, while the timeline is ordered by completion time (`InstantComparatorV2#orderingComparator`). For a table-service instant that completes late those disagree, so the computed range can miss it. Not covered by a test here. ### Impact No production change. Test-only. Why the existing coverage did not catch this: `TestHoodieIndexer` runs the indexer to completion and then rewinds instants to inflight to simulate concurrency. That exercises rollback handling of metadata deltacommits, but never places a writer and the indexer in flight at the same time, so the orderings that decide whether a commit lands in a new partition are not reachable. ### Risk Level none Test-only. Checkstyle clean. No existing test modified. ### Documentation Update none ### Contributor's checklist - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute) - [x] Enough context is provided in the sections above - [x] Adequate tests were added if applicable 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
