Repository navigation
test(utilities): use pairwise parameter matrices in TestHoodieDeltaStreamer and SchemaEvolutionQuick - #20359
Closed
databird07 wants to merge 1 commit into
Closed
test(utilities): use pairwise parameter matrices in TestHoodieDeltaStreamer and SchemaEvolutionQuick#20359databird07 wants to merge 1 commit into
databird07 wants to merge 1 commit into
Conversation
…reamer and SchemaEvolutionQuick These two classes are the most expensive in hudi-utilities CI (~1860 s and ~410 s). Most of their cost comes from full-factorial parameter matrices. This change removes only parameter rows that duplicate behaviour already covered. No test bodies, waits or continuous-mode polling/sleep intervals change. TestHoodieDeltaStreamer (198 -> 185 invocations): - testLogicalTypesWithJsonSource: 16 -> 8. Replace the full 2^4 factorial hasTransformer x orderingField x recordType x tableType with its even-parity half fraction, so every pair of factor values still runs together in at least one row. - testCOWLogicalRepair / testMORLogicalRepair: drop the SIX and EIGHT "reconcile with verdict" rows. The reconcile path does not branch on table version and still runs on CURRENT. The v6 guard rows and v8 default-path rows stay. - testUpsertsCOWContinuousMode: drop SPARK x EIGHT. v8 still runs with AVRO, and all combinations still run on the current version. Tradeoff: this was the only test in hudi-utilities where StreamSync writes Spark records to a v8 table. It is dropped deliberately under the rule that every write version runs at least once and all combinations run only on the latest. Spark-record writes are still covered on CURRENT, and v8 writes are still covered with AVRO. JaCoCo shows no lost lines in the full class. TestHoodieDeltaStreamerSchemaEvolutionQuick (52 -> 39 invocations): - testArgs: remove an exact duplicate MERGE_ON_READ row. - testReorderingColumn: 16 -> 8, using the same half fraction over rowWriterEnable x nullForDeletedCols x useKafkaSource x tableType. - testParamsWithSchemaTransformer: remove a COPY_ON_WRITE row that differs from a kept row only in rowWriterEnable. That flag has no effect in these upsert-only tests. Table versions 6, 8, 9 and 10 and both COW and MOR are still exercised. Measured locally (JDK 17, Spark 3.5, JaCoCo on, taskset 4 CPUs, 20 serial runs before and after, all exit 0): - changed DeltaStreamer methods: median 474 s -> 337.5 s, p90 490 -> 349 (48 -> 35 tests) - SchemaEvolutionQuick full class: median 268 s -> 213 s, p90 282 -> 225 (52 -> 39 tests) - full TestHoodieDeltaStreamer, one run: 1437 s -> 1284 s JaCoCo intersection across all 20 runs: Quick 25663 -> 25663 lines (no loss). For the changed DeltaStreamer methods alone, 4 HoodieSparkRecord.toIndexedRecord lines drop out of the intersection. Other tests in the full class still cover them. The remaining union-only differences are nondeterministic paths. One baseline run had a pre-existing Kafka produce race (partition leader not yet elected after createTopic) that Surefire's retry absorbed.
hudi-agent
reviewed
Oct 16, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this! The PR trims the TestHoodieDeltaStreamer and SchemaEvolutionQuick parameter matrices: it switches two 2^4 factorials to even-parity half fractions that still cover every pair of values, and drops one exact duplicate row, a few rows that only varied the table version on the reconcile path, and a row that only changed rowWriterEnable, which these upsert-only tests never use. 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 readability suggestion on the parity filter below.
cc @yihua
| for (String tableType : new String[] {"COPY_ON_WRITE", "MERGE_ON_READ"}) { | ||
| b.add(Arguments.of(tableType, rowWriterEnable, useKafkaSource, nullForDeletedCols)); | ||
| // Half fraction: every pair of factor values still appears together in at least one row. | ||
| boolean evenParity = !(rowWriterEnable ^ nullForDeletedCols ^ useKafkaSource ^ tableType.equals("MERGE_ON_READ")); |
Contributor
There was a problem hiding this comment.
🤖 nit: could you simplify the XOR chain, e.g. by counting the true flags and checking % 2 == 0, or by listing the 8 rows explicitly like logicalTypesWithJsonSourceArgs?
Contributor
Author
|
Combined into #20364 so the total impact can be reviewed in one place; this commit is included there unchanged. Closing. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20359 +/- ##
============================================
- Coverage 80.70% 78.12% -2.59%
+ Complexity 35124 34022 -1102
============================================
Files 2553 2553
Lines 143524 143629 +105
Branches 17474 17623 +149
============================================
- Hits 115830 112206 -3624
- Misses 19746 23247 +3501
- Partials 7948 8176 +228
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the issue this Pull Request addresses
TestHoodieDeltaStreamer(~1860s) andTestHoodieDeltaStreamerSchemaEvolutionQuick(~410s) are the most expensive classes in hudi-utilities CI. Most of their cost is full-factorial parameter matrices whose rows repeat behaviour that other rows already cover.Summary and Changelog
testLogicalTypesWithJsonSource: 16 -> 8 rows, using an even-parity half fraction, so every pair of factor values still runs together.testCOWLogicalRepair/testMORLogicalRepair: drop the v6/v8 "reconcile with verdict" rows. The reconcile path doesn't branch on table version and still runs on the current version.testUpsertsCOWContinuousMode: drop SPARK x v8. v8 still runs with AVRO records, and all combinations still run on the current version.testReorderingColumnthe same way, and remove one row that differs only inrowWriterEnable, which has no effect in these upsert-only tests.Impact
Test-only. DeltaStreamer 198 -> 185 invocations (full class 1437s -> 1284s). SchemaEvolutionQuick 52 -> 39 invocations, median 268s -> 213s over 20 runs. JaCoCo shows no lost lines at class level. Measured locally on JDK 17 with 4 pinned CPUs and JaCoCo on; full numbers are in the commit message.
Risk Level
low. Only parameter rows are removed. The one tradeoff (no Spark-record StreamSync write to a v8 table) is called out in the commit message.
Documentation Update
none
Contributor's checklist