sync_validate: catch-up counting assumes commit action (breaks MOR/clustered targets); JDBC failure masked by NPE
- Dominant language
- Java
- Stars
- 6.2k
- Forks
- 2.5k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 111
Description
**Describe the problem**
Two bugs in `ValidateHoodieSyncProcedure` (`sync_validate`), plus a dead guard:
1. `countNewRecords` reconstructs each catch-up instant hardcoding `HoodieTimeline.COMMIT_ACTION`, but the catch-up list comes from `getCommitsTimeline` (includes deltacommits) and is resolved against `getCommitAndReplaceTimeline`. Any MOR target, or a target with a `replacecommit` (clustering / insert overwrite) in the catch-up range, fails to read the instant file. Present since #6200.
2. In the JDBC count path, `finally { conn.close() }` runs with `conn == null` when `DriverManager.getConnection` fails, so the real `SQLException` is masked by an NPE. The sibling overload in the same file guards with `if (conn != null)` -- clearly an oversight, and a regression against the CLI code this was ported from: `hudi-cli`'s `HiveUtil` uses try-with-resources (`try (Connection conn = ...; Statement stmt = ...)`) and can never touch a null `conn`.
3. Dead code: the `if (sourceLatestCommit != null ...)` guard before the branch comparison is unreachable -- `sourceLatestCommit` is either a timestamp or the `"0"` fallback, never null.
**Suggested fix**
Carry the actual `HoodieInstant` from `commitsToCatchup` instead of synthesizing a COMMIT instant; for the connection handling, port `HiveUtil`'s try-with-resources shape (which also removes the need for the null guard the other overload hand-rolls); drop the dead null check. Regression tests already exist in `TestValidateHoodieSyncProcedure` (#19161): the MOR catch-up pin (bug 1) and cause-chain pins for both JDBC modes with an unreachable URL -- `mode => 'complete'` currently asserts an NPE with no `SQLException` in the chain (bug 2; the fix flips it), `mode => 'latestPartitions'` asserts the `SQLException` surfaces.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in ValidateHoodieSyncProcedure, especially countNewRecords and the JDBC count paths; then run TestValidateHoodieSyncProcedure and inspect its MOR catch-up and unreachable-URL cause-chain cases. Done means deltacommits and replacecommits are handled, JDBC failures preserve the SQLException instead of an NPE, and the unreachable null guard is gone.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- data-engineering
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100