apache / apache/hudi

sync_validate: catch-up counting assumes commit action (breaks MOR/clustered targets); JDBC failure masked by NPE

Open
#19,635 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.