test(sync): cover the meta-sync completion-time watermark end to end - #19726
test(sync): cover the meta-sync completion-time watermark end to end#19726Davis-Zhang-Onehouse wants to merge 2 commits into
Conversation
The completion-time watermark (last_commit_completion_time_sync) lets incremental meta sync register a commit that completes after the instant-time watermark moved past it, but nothing failed if it stopped being written, was computed with the wrong ordering, or was never read. Adds falsifiable unit tests on the Glue client, an end-to-end HiveSyncTool test of the hollow-instant rescue, and a no-op check for a table with no completed commit. The Glue fixture commit now lands in the timeline folder with an explicit completion time, so the fixture timeline is actually visible to the version-8 timeline reader.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19726 +/- ##
============================================
+ Coverage 77.90% 77.92% +0.01%
+ Complexity 33257 33256 -1
============================================
Files 2533 2533
Lines 140263 140263
Branches 16901 16901
============================================
+ Hits 109274 109298 +24
+ Misses 23359 23333 -26
- Partials 7630 7632 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The Glue watermark test seeds an existing table parameter and asserts the update merges instead of replacing, the e2e test asserts the registered partition's values, and the no-completed-commit test verifies the metastore was left untouched. The no-commit log line now names the table and the update's property map is typed to the Map interface.
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the contribution! This PR adds end-to-end and unit test coverage for the meta-sync completion-time watermark (last_commit_completion_time_sync) across the Glue and Hive sync clients, fixes the Glue test fixture to write its seed commit into .hoodie/timeline/ so the version-8 timeline actually reads it, and includes a small error-message/log wording fix in AWSGlueCatalogSyncClient. The production change is limited to cosmetic message updates and a local HashMap→Map widening, and the new tests faithfully model the out-of-order-completion scenario the feature protects. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. One minor inconsistency in the new test — HashMap used as a declared type in one spot while this same PR switches to Map everywhere else.
cc @yihua
| List<Column> columns = Collections.singletonList(GlueTestUtil.getColumn("name", "string", "person's name")); | ||
| List<Column> partitionKeys = Collections.singletonList(GlueTestUtil.getColumn("city", "string", "person's city")); | ||
| HashMap<String, String> props = new HashMap<>(); | ||
| props.put(HOODIE_LAST_COMMIT_TIME_SYNC, "101"); |
There was a problem hiding this comment.
🤖 nit: this PR already switches HashMap to Map in the production code — could you do the same here (Map<String, String> props = new HashMap<>()) for consistency?
4 participants
Describe the issue this Pull Request addresses
closes #19725
HUDI-7494 taught the meta-sync clients to persist and read
last_commit_completion_time_syncso incremental sync can register a commit that completes after the instant-time watermark has already moved past it. The mechanism has no test coverage on the sync clients: nothing fails if the completion watermark stops being written, is computed with the wrong ordering, or is never consulted on read. Separately, the Glue test fixture writes its seed commit into.hoodie/while the version-8 timeline reads.hoodie/timeline/, so every Glue test has been running against an empty active timeline.Summary and Changelog
Test-only coverage for the completion-time watermark, plus one error-message fix.
TestAWSGlueSyncClient: four new tests. The watermark test seeds a second commit whose instant time sorts below the fixture's but whose completion time is later, and asserts the two watermarks come from the two different instants; mutating the client to persist the completion time of the instant-time-latest instant fails it (expected: <20250101000001000> but was: <20250101000000000>). Also: no-op when the timeline has no completed commit, and both read-back paths ofgetLastCommitCompletionTimeSynced.TestHiveSyncToolLongRunningWriteWatermark(new): end-to-end onHiveSyncTool, parameterized over the client. A long-running INSERT_OVERWRITE completes after two sync cycles advanced the watermark past its instant time: the stock client registers the partition through the hollow-instant lookup inTimelineUtils.getCommitsTimelineAfter; an instant-time-only client silently and permanently drops it.TestHiveSyncTool:updateLastCommitTimeSyncedon a created-but-never-written table must no-op.HiveTestUtil:startInsertOverwritePartition(requested + inflight markers and data files, no completed file) andaddEmptyCommithelpers.GlueTestUtil: the fixture commit is now written into the timeline folder with an explicit completion time in the file name, so the fixture timeline is actually visible;createCommitFile/deleteCommitFilehelpers.AWSGlueCatalogSyncClient: thegetLastCommitCompletionTimeSyncedfailure message and the no-commit log line now name the database-qualified table like their siblings, and the watermark property map is typed to theMapinterface.Test code adapted from the 0.x backport of HUDI-7494.
Impact
None on production behavior; one exception message now includes the database name.
Risk Level
none
Documentation Update
none
Contributor's checklist