Repository navigation
chore(ci): reduce GitHub CI test runtime without losing coverage - #20364
Open
databird07 wants to merge 14 commits into
Open
databird07 wants to merge 14 commits into
databird07 wants to merge 14 commits into
Conversation
…ly and share writes across view types
TestHoodieFileSystemViews#testFileSystemViewConsistency writes a ~28-commit
history (inserts, upserts, inline compaction/clustering, clean, archival and a
rollback) and checks that a configured file system view matches the in-memory
baseline view. It ran 12 cases: table type x table version (6, 8) x {MDT off
with the spillable view, MDT on with the in-memory view, MDT on with the
spillable view}. Each MDT-on table was written twice, once per view type, even
though the reader-side view type only decides how the finished table is read.
This made it the most expensive class in the "Java UT part 2" GitHub job.
Changes:
- Every table is now written with the metadata table enabled, which is the
mode that matters going forward. The four MDT-off cases are removed.
Turning the metadata table off on a table that has one
(HoodieTable#maybeDeleteMetadataTable) stays covered by
TestHoodieBackedMetadata#testTurnOffMetadataTableAfterEnable and
TestHoodieTable.
- Each table is written once and compared against both the in-memory and the
spillable view at every checkpoint (after the first inserts, mid-history,
after the rollback and at the end).
- The view storage type used by the writer (and its embedded timeline
server, which drives small-file, compaction, clustering, clean and rollback
planning) is still varied: every older supported write version (6, 8, 9;
see HoodieWriteConfig.WRITE_TABLE_VERSION) writes with SPILLABLE_DISK, and
the current version (10) writes with both MEMORY and SPILLABLE_DISK, for COW
and MOR. That is 10 cases instead of 12, and versions 9 and 10 are now
tested here for the first time.
- Views are closed in a finally block, and assertion failures name the view
storage type that diverged.
Locally (4 pinned CPUs, Spark 3.5, JDK 17, JaCoCo on) all 20 runs passed
with 10/10 tests (median 340s on a noisy VM). In 3 interleaved rounds against
the old test on the same machine, the class took 318-332s versus 285-329s
before, so it costs about the same (roughly +10%) while covering about 750
more lines in every run, mostly table-version 9/10 write and read paths.
Lines covered in every run of the old test are all still covered, except:
- the 9 lines of the MDT-off writer path above;
- the partition-not-in-store branch of
AbstractTableFileSystemView#getAllFileGroupsStateless (only reached with
the metadata table off), still covered by
TestHoodieTableFileSystemView#testViewForGetAllFileGroupsStateless in
hudi-hadoop-common.
The only other lines that differ are reached by chance in both versions: the
timeline server port retry (only when its port is already taken), the remote
view fallback after a DNS failure, and second-granularity instant-time
matching in CommittedInstants.
…strapIndex#testBootstrapIndexConcurrent testBootstrapIndexConcurrent starts 20 threads that each open their own HFileBootstrapIndex reader over the same index files and validate the full bootstrap mapping 50 times (1,000 full validations). It took ~85s of the "UT - hudi-hadoop-common" GitHub step, almost all of it repeating the same reads after the readers were already open. Change numRequestsPerThread from 50 to 10. The test still runs 20 threads concurrently, each with an independent reader (200 full validations), so it still exercises concurrent reader creation and lookups against the same files. Sharing one reader across threads was considered and rejected: HFileBootstrapIndexReader only synchronizes lazy reader creation, while getAllKeys / getSourceFileMappingFor* move a stateful HFile cursor without locking, and production code opens one reader per use. Measured locally (JDK 17, 4 pinned CPUs, JaCoCo on), 20 runs each: - before: 20/20 passed (4 tests), median 50.2s, p90 52.6s - after: 20/20 passed (4 tests), median 14.5s, p90 15.3s - JaCoCo across all hudi modules: 3,128 lines / 489 branches covered before and after, no line lost or gained. Tradeoff: fewer iterations per run lowers the per-run chance of hitting a rare race in concurrent reader opening.
…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.
…heck send acks
Problem: KafkaTestUtils.createTopic returned as soon as the CreateTopics
request finished. sendMessages called producer.send() and flush() but never
looked at the returned futures. A test that created a topic and produced to
it right away could hit a partition with no elected leader yet. The broker
answered NOT_LEADER_OR_FOLLOWER, and the idempotent producer's retries ended
in OUT_OF_ORDER_SEQUENCE_NUMBER until the default delivery.timeout.ms
(120 s) expired, dropping those records. Because nothing checked the
futures, the test went on with missing data and failed later on an
unrelated-looking count assertion. Observed in
TestHoodieDeltaStreamer#testLogicalTypesWithJsonSource: "expected 1000,
found 974", with a 121.9 s failed attempt, in 1 of 20 local runs. Surefire's
rerun hid it with exit code 0.
Fix (test code only):
- createTopic now polls describeTopics (100 ms interval, 60 s timeout
measured with System.nanoTime()) until every partition reports a leader,
so producers no longer race leader election.
UnknownTopicOrPartitionException during metadata propagation is retried.
The lookup uses describeTopics(...).values().get(topic).get(), the same
pattern as KafkaConnectUtils, rather than allTopicNames(). This keeps the
code compatible with kafka-clients 2.8.1, which the spark3.3 profile uses
and which has no allTopicNames().
- createTopic's catch block restores the thread's interrupt flag when it
catches an InterruptedException.
- Both sendMessages overloads keep every send future and call get() on each
after flush(). A failed send now throws
IllegalStateException("Failed to send a record to Kafka topic <topic>")
with the Kafka error as the cause, instead of silently losing records.
Verification (local, JDK 17, Spark 3.5, taskset 4 CPUs, serial):
- 20 runs of the Kafka-source DeltaStreamer tests that use these helpers:
TestHoodieDeltaStreamer#testLogicalTypesWithJsonSource* (16 rows),
testJsonKafkaDFSSource*, testKafkaTimestampType*,
testParquetSourceToKafkaSource*, and
TestHoodieMultiTableDeltaStreamer#testMultiTableExecutionWithKafkaSource.
All 20 runs exited 0 with 22 tests each, 0 failures, 0 errors, 0 skips and
0 flaky reruns. Median wall time was 189 s. No NOT_LEADER_OR_FOLLOWER or
OUT_OF_ORDER_SEQUENCE_NUMBER appeared in the logs.
- Final revision, with the review changes above: test-compile passes on the
default profile and on -Dspark3.5 -Dscala-2.12 -Pjava17, with 0 checkstyle
violations. KafkaTestUtils compiles with javac against kafka-clients 2.8.1.
The earlier allTopicNames() version did not. 5 more runs of the same 22
tests all exited 0 with no failures, errors, skips or flakes (188-190 s).
- A throwaway test, not committed, sent a 2 MiB record (over the default
max.request.size). sendMessages threw IllegalStateException caused by
RecordTooLargeException, which confirms that failed sends are now reported.
…te real table versions in TestColumnStatsIndexWithSQL TestColumnStatsIndexWithSQL (82 invocations, ~816s in CI) rebuilt the same 3-commit COW/MOR table (input-table-json -> another-input-table-json -> update-input-table-json) for every parameter case, then only ran SQL and data-skipping assertions against it. The parameter case carries tableVersion (6, 8, 9, current=10) and shouldReadInMemory, but the class never put tableVersion into the write options, so every case wrote a v10 table and versions 6/8/9 were never exercised by this class. What changes: - tableVersionOpts(testCase) adds WRITE_TABLE_VERSION and HoodieTableConfig.VERSION = tableVersion, and TIMELINE_LAYOUT_VERSION = 1 for v6 and 2 otherwise (as TestHoodieFileSystemViews does), to the write options of all 7 methods that take a ColumnStatsTestCase. Every write in those methods (setupTable, the in-memory-index insert, the DELETE write, the scheduleCompaction write client, the re-upserts) uses these options, and ColumnStatIndexTestBase now asserts for every write that the table has the requested version. MDT stays enabled in every case. - useSharedTable(recipe, writeOpts, copy) caches a built table per class, keyed by the recipe name plus the sorted write options (so each version gets its own table), in a temp root that an AfterAllCallback deletes. Each build starts with initMetaClient(), like the per-test harness, so the Overwrite-onto-existing-table path is still exercised. - Read-only methods (WithSQL, WithLimitedIndexes, WithInMemoryIndex, CompactionWithSQL) read the shared table directly. Mutating methods (DeletionWithSQL, ScheduledCompactionWithSQL, GetPrunedPartitionsAndFileNames) get a full private copy in a unique directory under their own @tempdir. Other tests are unchanged. - Shared builds that validate column stats validate both index read modes (in-memory and Spark), so a case reads a table validated in its own mode whichever case built it. - The recipe name covers every non-option input of the build (shouldValidate, useShortSchema), so a case that skips validation can never hand its table to one that validates. A build that throws deletes its half-built directory and caches nothing. - Isolation guards: after each test, the shared table's completed instants, completed MDT instants and each file's path, size and modification time must match the snapshot taken when it was built. A changed table is evicted before the assertion fails, so only the offending test fails. Each test must get a new SparkSession (previous one tracked through a WeakReference); the tbl view is dropped, the catalog cache cleared and changed SQLConf keys restored. Files.walk streams are closed. The class is @execution(SAME_THREAD) because the cache is not thread-safe. Expected results: the column-stats fixtures this class compares against (column-stats-index-table.json, updated-*, *-short-schema, cow/mor-updated2-*) describe data content and are the same for every version; all four versions pass them unchanged. The only version-specific fixtures in the directory (mor-bootstrap1/2-column-stats-index-table-v10.json) belong to the TestColumnStatsIndex bootstrap tests, not this class. Nothing is skipped or loosened for any version, and no test or parameter case is removed: all 82 invocations still run. Measured, each run pinned to 4 CPUs with taskset, JaCoCo agent on: - Upstream baseline (e9da226): 20 runs, all exit 0, 82 tests, 0 failures/errors/skips; median 910s, p90 1102s (host had varying CPU steal). - This change: 20 runs, runs 11-20 with MethodOrderer$Random; all exit 0, 82 tests, 0 failures/errors/skips; median 555s, p90 658s (510-706s). - Same time window (10 runs each side, concurrently on the other 4 CPUs): upstream median 681s vs this change 555s, paired ratio median 0.82. Without the version wiring the shared fixtures alone measured 0.62; the difference is the cost of building real v6/v8/v9 tables (40 shared builds instead of 10). - JaCoCo lines covered in all 20 runs: 28209 -> 30174; in any run: 28212 -> 30178. No line covered in any baseline run is lost. 1965 lines are newly covered in every run, from the older-version paths: v1 timeline (ActiveTimelineV1, TimelineArchiverV1, InstantFileNameGeneratorV1), v6 MDT writer (HoodieBackedTableMetadataWriterTableVersionSix), Avro/HFile log blocks and log format writer/reader used below v10 (HoodieInlineLogAppendHandle, HoodieAvroDataBlock, HoodieDeleteBlock, HoodieHFileDataBlock), and the V1 incremental relations.
This was referenced Oct 16, 2026
…orks
What
- bot.yml: test-spark-java17-java-tests-part1 now passes -DforkCount=2
and -Djacoco.agent.dest.filename=jacoco-fork${surefire.forkNumber}.exec
to its test step. The root pom default (forkCount=1, reuseForks=true)
and every other job stay as they are.
- pom.xml: surefire exports hoodie.test.fork.number=fork${surefire.forkNumber}
so tests can find out which fork they run in. The "fork" prefix is
needed because a value made only of ${surefire.forkNumber} arrives
empty.
- HoodieTestUtils: getTestForkNumber() and toForkUniquePort(port). They
add 2000 per fork and wrap into [1024, 32768) so ports stay below the
ephemeral range. Fork 1 (and any non-forked run) keeps the original
port numbers.
- ZookeeperTestService client port 2828 and the timeline-server start
port in HoodieWriterClientTestHarness and
SparkClientFunctionalTestHarness are now fork-unique. The
timeline-server retry logic is unchanged.
- HoodieSparkClientTestHarness: optional reuse of one SparkSession and
JavaSparkContext per test class (-Dhoodie.test.spark.reuse.context=true,
off by default and not enabled in CI). The context is reset after each
test: streaming queries, running jobs, local properties, SQLConf,
Hadoop conf, catalog/database, temp and global temp views, databases,
tables, user functions, cache and persisted RDDs, and listeners the
test added. Classes can opt out with reuseSparkContextAcrossTests().
A failed reset stops the context and fails the class in @afterall.
Why
part1 is the longest Hudi CI job (~73 min on GitHub, test step ~66 min).
Its tests spend most of their time waiting on per-commit I/O and
single-threaded work, so a second JVM on the same 4 vCPUs nearly doubles
throughput. The only things two JVMs shared were fixed ports and the
JaCoCo output file, and this change makes those per fork.
Numbers (local, taskset -c 0-3, JDK 17, Maven 3.9.9, -Xmx4g like CI,
exact part1 command and profiles)
- Full part1 test step: baseline 3329/3479/3611 s (median 3479).
forkCount=2: 2052/2079/2078/2159 s (median 2078, -40%).
forkCount=2 + shared Spark context: 1978/2002/2024 s (median 2002, -42%).
Every run: 4211 tests, 0 failures, 0 errors, 65 skipped, 340 classes.
Peak RSS per fork 5.5-6.3 GB (~12 GB for both, 16 GB runner).
- Flakiness, 31-class subset (30 slowest classes + the multi-writer
test, 701 tests), 20 runs each, 10 of them with
MethodOrderer$Random: baseline median 2696 s / p90 2742 s, forkCount=2
1640 / 1827 s, + shared Spark 1578 / 1719 s. 0 failures, 0 errors,
0 reruns in all 60 runs. The only count difference (699 vs 701) is a
pre-existing random-order dependence in
TestHoodieFileGroupReaderOnSpark's @MethodSource that also happens at
baseline.
- JaCoCo (per-fork exec files merged): with the shared Spark context the
covered lines match the baseline intersection and union exactly
(20 runs each). With forkCount=2 alone, the lines not covered in every
run are thread-race lines from TestMultiWriterWithPreferWriterIngestion
(SimpleConcurrentFileWritesConflictResolutionStrategy:148 and
WriteOperationType:99). They are covered in only some runs even at
baseline (5/20 subset), and TestSimpleConcurrentFileWritesConflict-
ResolutionStrategy in test-common-and-other-modules covers them
deterministically. The remaining differences also vary between
baseline runs: random insert buckets, partition TTL against the wall
clock, and which reader hits a deliberately corrupted file first.
SparkPartitionTTLActionExecutor:59 (no expired partition) depends on
a 1-second TTL in TestHoodieSparkSqlWriterPartitionTTL, which the
slower forked run exceeds. TestPartitionTTLManagement in the
spark-client job covers it deterministically.
The shared Spark context saves nothing measurable on this job: creating
a local context is cheap next to each test's own work. That is why it
is left off.
The harnesses' static port fields now call HoodieTestUtils, which lives in the
hudi-common test jar. hudi-examples-spark loads SparkClientFunctionalTestHarness
(TestHoodieSparkQuickstart) without depending on that jar, so it failed with
NoClassDefFoundError; add hudi-common:tests as a test dependency there. No other
module uses these harnesses without already depending on hudi-common:tests.
… test jobs
Every test job of bot.yml ran its own `mvn clean install -DskipTests` before its
test steps (about 95 of ~620 job-minutes of a master push). Jobs that build the
same profile set now restore one shared build instead.
What:
- build-jdk11-spark35 (scala-2.12, spark3.5, flink2.2, -Phudi-platform-service,
-Pthrift-gen-source, in the hudi-ci docker image) serves
test-spark-client-and-hadoop-common, test-utilities,
test-common-and-other-modules and test-hudi-hadoop-mr-and-hudi-java-client.
- build-java17-spark42 (java17, scala-2.13, spark4.2) serves the five
test-spark-java17-* jobs.
- build-flink (flink2.2 with avro 1.11.4 / parquet 1.15.2) serves test-flink-1/2.
- validate-bundles, validate-bundle-spark4 and integration-tests keep their own
builds: they use other profiles (bundle deploy, -Pintegration-tests, other
Spark/Flink versions), so their output is not the same build.
How:
- .github/actions/save-hudi-build packs every module target/ directory and
~/.m2/repository/org/apache/hudi (pax tar + zstd, identical target/lib jars
hard-linked so they are stored once) and uploads them as a workflow artifact
named hudi-build-<sha>-<jdk>-<profiles...>.
- .github/actions/restore-hudi-build downloads that artifact, checks that its
manifest names this commit and this profile set, replaces the local
org/apache/hudi repository with it, unpacks the target/ directories and
re-stamps them (compress_mtimes.py) so Maven sees them as up to date: every
distinct original millisecond keeps its order and gets its own millisecond
just after the newest checked-out source, so neither javac, maven-jar-plugin
nor scala-maven-plugin (which recompiles when a source is not older than its
timestamp file) treats the restored outputs as stale.
- The test jobs drop their build step and restore instead; every test step
(module lists, profiles, -Dtest / wildcardSuites filters, excludes) is
unchanged. Docker based jobs mount the workspace at the same absolute path
in the build and test containers, since Maven metadata stores absolute paths.
Review follow-ups: the save action chowns the packed target/ dirs and
org/apache/hudi to the runner user before hard-linking and packing (builds
in the hudi-ci container leave root-owned files), artifact retention is 7
days so "re-run failed jobs" keeps working for a week, and the three build
jobs get a 120-minute timeout instead of 60.
Measured locally (JDK 17, Maven 3.10.0, taskset -c 0-3, 20 runs each of
the "Java UT 1 - Common & Spark" step of test-spark-java17-java-tests-part1
narrowed to -Dtest=TestHoodieAvroUtils, JaCoCo on):
- in-job build (today): 20/20 exit 0, 103 tests, 0/0/0 fail/err/skip,
median 126.3 s, p90 127.6 s
- fresh checkout + restored artifact: 20/20 exit 0, 103 tests, 0/0/0,
0 recompiled sources, median 126.9 s, p90 128.0 s
- JaCoCo (hudi-common + hudi-spark): 1021 covered lines in every run of
both series; intersection and union identical, no line lost or gained.
- Without the checkstyle cache refresh the restored runs took median
143.4 s (checkstyle re-checked every source of the fresh checkout).
Artifact for the Java 17 / Spark 4.2 group: 1.07 GB (1.04 GB targets after
hard-linking duplicate target/lib jars, was 3.76 GB; 35 MB of
org/apache/hudi). Upstream run 38362333401 spent 63.5 of 621.8 job-minutes
in the 11 builds removed here; with three shared builds and the restores
the projected saving is about 30 job-minutes per push, while the critical
path gets about 3 minutes longer because test jobs wait for a build job.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20364 +/- ##
============================================
+ Coverage 80.73% 80.75% +0.01%
- Complexity 35201 35313 +112
============================================
Files 2553 2562 +9
Lines 143629 144143 +514
Branches 17504 17572 +68
============================================
+ Hits 115964 116398 +434
- Misses 19703 19767 +64
- Partials 7962 7978 +16
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
devin-ai-integration
Bot
force-pushed
the
perf/ci-runtime-reduction
branch
from
October 16, 2026 07:11
f100abb to
21e4c5c
Compare
3 tasks done
Java tests part 1 already runs two surefire JVMs (-DforkCount=2), which cut
its test step by about a third on GitHub Actions. This extends the same setup
to Java tests parts 2 and 3 and to the spark-client job.
- pom.xml: the functional-tests, functional-tests-b and functional-tests-c
profiles hard-coded <forkCount>1</forkCount>, which overrode -DforkCount on
the command line. They now use ${forkCount}, defined as 1 in the root
properties, so local builds behave as before and CI can pass
-DforkCount=2.
- bot.yml: the Maven test commands in those jobs pass -DforkCount=2 and
write one JaCoCo file per JVM
(jacoco-fork${surefire.forkNumber}.exec), as part 1 already does, so the
coverage upload still sees every JVM's data.
- HiveTestService: HiveServer2 used a fixed port 9999, so two JVMs running
Hive-backed tests would collide. The port now goes through
HoodieTestUtils.toForkUniquePort, the same per-JVM offset ZooKeeper and the
timeline server already use. With a single JVM it is still 9999. Kafka
already runs in a Testcontainers broker on a random port, and the schema
registry and HTTP stubs in these tests don't bind fixed ports.
- TestHoodieDeltaStreamerWithMultiWriter: its lock config pointed at a fixed
ZooKeeper port 2828, while ZookeeperTestService already starts on the
per-JVM port. The config now uses HoodieTestUtils.toForkUniquePort(2828)
too, so the second JVM's writers reach their own ZooKeeper.
The common/other job is unchanged for now: it runs Maven inside a Docker
container, so its arguments need separate quoting.
hudi-utilities stays on one JVM. Tried on GitHub Actions, two JVMs made
its unit-test step slower (28:13 -> 30:40 min): every class took about 1.7x
longer, so the job is bound by CPU and I/O, not by running classes one at a
time. The per-JVM HiveServer2 and ZooKeeper ports above are kept, so tests
that use them are safe to run in parallel later.
Measured on GitHub Actions against the same branch without this change,
with every job passing: Java tests part 2 went from 50.2 to 35.4 min, part 3
from 42.4 to 23.7 min, and the spark-client unit-test step from 17:32 to
13:53 min.
devin-ai-integration
Bot
force-pushed
the
perf/ci-runtime-reduction
branch
from
October 16, 2026 15:35
06ee19a to
b42f5ce
Compare
… pairs HadoopStorageConfiguration is Java-serialized into Spark task closures (it is held by HoodieTableMetaClient, engine contexts, write configs, ...), so it is written on the driver/DAG-scheduler thread and read on executors for every task. It delegated to Hadoop's Configuration#write/readFields, which writes each property's source list with WritableUtils.writeCompressedStringArray: every source string gets its own GZIP stream, and reading each one ends in a thrown and caught EOFException from GZIPInputStream. An async-profiler CPU profile of TestHoodieDeltaStreamer (185 tests, 19 min) attributed about 400s of its 1,609s total CPU (~25%) to this serde: 266s in readObject on executor threads and 138s in writeObject on the driver, including the single-threaded dag-scheduler-event-loop. A microbenchmark of a default Configuration (254 properties) measured about 1.1ms to write and 1.0ms to read with Configuration#write, versus 0.08ms and 0.15ms with plain pairs. writeObject now writes a marker byte (-1, which can never start the legacy payload because its VInt property count is non-negative), a format version, the property count, and each key and value with Text.writeString (no 64KB limit). readObject sets them on a Configuration(false); Configuration#set applies the same deprecated-key handling that readFields uses. The only dropped information is the per-property "source" bookkeeping (Configuration#getPropertySources), which is diagnostic only; final-parameter markers were never serialized by Configuration#write either. Compatibility: when the marker is absent, readObject falls back to reading the legacy Configuration#write payload, so objects serialized by older Hudi versions (e.g. in Flink operator/coordinator state restored after an upgrade) still deserialize. serialVersionUID is unchanged so that fallback is reachable. The reverse direction cannot be fixed in old code: an older Hudi reading the new format sees a negative property count and gets an empty Configuration, so downgrading a Flink job onto state written by this version, or mixing Hudi versions between driver and executors, is not supported. TestHadoopStorageConfiguration gains a round-trip test covering all default properties, a value longer than 64KB, a non-BMP unicode value and a key set through its deprecated name, plus a test that reads a payload serialized by the previous implementation (followed by another object, to check the stream stays aligned).
…urable In continuous mode, when a write is rejected with a HoodieClusteringUpdateException because it conflicts with a pending clustering plan, StreamSyncService.handleUpsertException slept a hard-coded 60 seconds before retrying. Add HoodieStreamer.Config#clusteringConflictRetryIntervalSecs (--clustering-conflict-retry-interval-seconds), defaulting to 60 so the production behavior is unchanged, and use it for that sleep. A value of 0 retries immediately; every retry that hits the conflict again rolls back another attempt. TestHoodieDeltaStreamer#testAsyncClusteringServiceWithConflicts sets it to 2 seconds. The test still hits the rejection-and-retry path (the "Write rejected due to conflicts" warning is logged once per run) and still asserts a commit after rollback; it no longer spends a minute idle before the retry. Also fix the Spark UI key in UtilitiesTestBase: it set "spark.ui.enable", which Spark ignores, so every utilities test started the Spark UI. It now sets "spark.ui.enabled".
… class
testAdditionalFilesInMetadata runs 10 parameter cases (lastNFileSlices in
{-1, 1, 3, 4, 5} x ignoreFailed). Every case used to write the same 7-commit
MERGE_ON_READ table (one overwrite plus six appends of the same 10 records,
with inline compaction every 2 delta commits) before running the validator.
The table does not depend on the test arguments, so the first case to run
now writes it as before and copies it into a class-level @tempdir, then
writes a READY marker file there. The other cases copy that table into
their own base path with the raw local file system, which copies the files'
contents (the validator compares file names and sizes, not modification
times). Each case still moves and deletes log files in its own copy and
runs the same validator assertions, so no case shares mutable state with
another, and any case can be the one that writes the table.
The marker file is used instead of a static flag because Surefire's
rerunFailingTestsCount reruns a failed case in the same JVM with a fresh
static @tempdir; the rerun then finds no marker and writes the table again
instead of copying from a directory that no longer exists.
Locally the full class goes from 341s to 244s (median of 3 alternating
runs, 47 tests passing in each). The write path is still run once per
class, so the code it covers is unchanged.
…ites Profiled the slowest suites in the test-spark-java17-scala-other-tests job (TestSecondaryIndex, TestExpressionIndex, TestClusteringProcedure, TestSpark3DDL, TestCreateTable, HoodieBackedTableMetadataIndexLookupV8/V9 Test1Fg/Test10Fg) with async-profiler 4.1 (cpu 10ms + wall 20ms, quiet warn-log logging, JaCoCo off). Test-thread wall time is dominated by metadata-table work (33-66%) and writes (23-41%); Spark sessions are already created once per suite (lazy, stopped in afterAll). Three avoidable costs showed up and are fixed here: 1. HFileReaderFactory: HFiles below the in-memory threshold are read fully into a byte[] of the file size, but the stream was opened with the storage default buffer size (the block size, 32MB on the local file system). RawLocalFileSystem/ChecksumFileSystem allocate that buffer up front, so every MDT HFile open allocated and zeroed 32MB (Unsafe.allocateUninitializedArray / BufferedInputStream.<init> on the test thread). readFully into a file-sized array bypasses the stream buffer, so the stream is now opened with max(1, min(fileSize, storage.getDefaultBufferSize())). The read itself is unchanged; object stores ignore the hint. 2. HoodieSparkSqlTestBase: sets hoodie.time.generator.max_expected_clock_skew_ms=1 for the suite's SparkSession. SkewAdjustingTimeGenerator sleeps this long after reading the clock, under the lock, on every instant generation. Most SQL write paths already infer 1ms (InProcessLockProvider), but procedures (run_clustering/run_compaction via HoodieCLIUtils.getLockOptions) and tables configured with FileSystemBasedLockProvider used the 200ms default; this was 7.9% of TestClusteringProcedure test-thread wall time. All tests run in one JVM with a single clock, so 1ms still keeps instant times unique and ordered (the next lock holder reads at least ts+1). 3. TestHoodieBackedTableMetadataIndexLookup: the SQL conf key "set hoodie.metadata.global.record.level.index.max.filegroup.count" had a stray "set " prefix and was silently dropped. Fixed to the real key. This does not change behaviour: the table is created with min = max = N and estimateFileGroupCount already clamps to the minimum, which was confirmed via MDT initialization logs (the 10Fg variants create 10 RLI and 10 SI file groups both before and after). Measurement: 20 paired runs of all 9 suites in one JVM each, before and after running concurrently on disjoint pinned CPU sets (taskset 0-3 / 4-7, alternating), suite order shuffled on every other run, JaCoCo agent on, quiet log4j config (as -Pwarn-log). 0 failures/errors/cancellations in all 40 runs, 132 tests per run on both sides. Median seconds (before -> after, 20 runs each): TestSecondaryIndex 243.2 -> 211.8 (-12.9%) 10 tests TestExpressionIndex 237.5 -> 203.6 (-14.3%) 26 tests TestSpark3DDL 206.9 -> 184.9 (-10.7%) 16 tests TestClusteringProcedure 179.6 -> 149.8 (-16.6%) 13 tests TestCreateTable 172.3 -> 157.1 (-8.8%) 55 tests ...LookupV9Test10Fg 22.4 -> 21.1 (-5.9%) 3 tests ...LookupV9Test1Fg 15.1 -> 14.8 (-2.0%) 3 tests ...LookupV8Test10Fg 14.2 -> 13.8 (-2.8%) 3 tests ...LookupV8Test1Fg 11.2 -> 10.7 (-4.3%) 3 tests sum of the 9 suites 1115.2 -> 967.6 (-13.2%) Attribution (3 extra paired runs of TestSecondaryIndex, TestExpressionIndex and TestClusteringProcedure without JaCoCo, full change vs. the change without the HFileReaderFactory part): dropping the HFile buffer fix makes the sum 16% slower (570.7s -> 661.8s median), so it carries most of the gain; the clock-skew setting accounts for the rest of the TestClusteringProcedure improvement (its 200ms sleeps were 7.9% of that suite's test-thread wall time). Coverage (JaCoCo, lines covered in every one of the 20 runs, after mapping shifted line numbers): 44116 before, 44115 after. The only line no longer hit is HoodieStorage.openSeekable(path, wrapStream), the default-buffer convenience overload that HFileReaderFactory no longer calls; it is still used by HFileBootstrapIndexReader and BatchedBlobReader and covered by TestHoodieStorageBase. Union of covered lines is identical (44144).
Three test-only changes, chosen from async-profiler wall/CPU profiles of
the slowest classes in the Java 17 Spark functional-test jobs.
1. HoodieWriterClientTestHarness: stop waiting out heartbeat expiry.
testRollbackFailedCommits, testRollbackFailedCommitsToggleCleaningPolicy
and testParallelInsertAndCleanPreviousFailedCommits slept/polled until
the failed writers' heartbeats expired (interval 3s x 10 tolerable
misses = 30s+ each wait), which was ~33% of the test-thread wall time
of TestHoodieClientOnCopyOnWriteStorage. A new expireHeartbeats()
helper instead moves the heartbeat file's modification time of each
failed (already closed) writer back by interval * (misses + 1) via
HoodieStorage.setModificationTime, and asserts that the heartbeat
client now reports every one of those instants as expired. The live
writer in the parallel test is never backdated. Semantics are the
same as waiting: the expiry check only reads the file mtime.
2. TestMORDataSource#testCount: table version 8 repeated all 16
read/write combinations that also run on the current version. Every
combination still runs on the current table version; version 8 keeps
representative cases covering each merge mode and both read/write
paths, and the version-6 cases are unchanged (34 -> 22 cases). The
version parameter is set as hoodie.write.table.version and checked
against the table config.
3. TestColumnStatsIndex: testMetadataColumnStatsIndex,
testMetadataColumnStatsIndexInitializationWithUpserts and
testMetadataColumnStatsIndexInitializationWithRollbacks ran the full
table type x read mode / partitioning matrix on versions 6, 8, 9 and
10. New providers columnStatsIndexParams / tableTypePartitionTypeParams
keep the full matrix on the current version, and run both table types
on each older version with the read mode / partitioning alternating,
so every older version still covers both values of each dimension.
Partitioned MOR additionally runs on version 6: it is the only
older-version case whose rollback appends a rollback command block
(the log-block path of RollbackHelperV1); JaCoCo showed 123 lines
lost without it (104 -> 88 tests).
Measurement (local, Java 17, -Pjava17 -Dscala-2.13 -Dspark4.2 -Pwarn-log,
taskset-pinned 4 CPUs, JaCoCo on, before/after runs alternating, every
third pair in MethodOrderer$Random order, 0 failures/errors/retries in
every run):
TestHoodieClientOnCopyOnWriteStorage (97 tests both sides):
median 377.8s -> 281.7s (-25.4%), ranges 364.9-416.2s / 273.1-295.1s
TestMORDataSource (115 -> 103 tests):
median 497.6s -> 400.3s (-19.5%), ranges 484.5-539.8s / 385.0-430.7s
TestColumnStatsIndex (104 -> 88 tests):
median 343.1s -> 272.4s (-20.6%), ranges 333.8-380.3s / 256.9-295.9s
No before/after range overlap in any class; CPU steal per run was
similar on both sides.
Coverage: JaCoCo lines covered in every one of the 20 before runs vs every
one of the 20 after runs:
TestHoodieClientOnCopyOnWriteStorage: 28,486 -> 28,488, 0 lines lost.
TestMORDataSource: 35,195 -> 35,195, 0 lines lost.
TestColumnStatsIndex: 30,153 -> 30,151. The 2 lines missing from the
after intersection, RollbackHelper#deleteFiles lines 146/149 (delete
of an already missing file), were still covered in 19 of 20 after runs:
unpartitioned MOR in the rollback test picks the first .log file from a
recursive listing, which is sometimes a metadata-table log, so the
branch is listing-order dependent. Both lines are covered in all 20
before and after runs of TestHoodieClientOnCopyOnWriteStorage.
…tests Without the Hadoop native library (as on the CI runners), RawLocalFileSystem.setPermission forks a chmod process for every file and directory it writes. Profiles put those forks at 6-18% of test-thread wall time in the Java Spark functional classes, and about 10% in the utilities tests. Add a test-only NioPermissionLocalFileSystem to the hudi-hadoop-common test sources. Its raw file system sets ordinary rwx modes with Files.setPosixFilePermissions. It falls back to Hadoop's implementation when native IO is available, when the mode has bits outside 0777, when the target is a directory with setuid/setgid bits, and when POSIX attributes aren't supported. A review compared it with stock Hadoop over 1,569 mode/permission probes and found no differences. The harnesses build their Hadoop configuration with new Configuration(false), so a test core-site.xml is never read, and Hadoop caches the first file:/// instance per user. So the file system is installed in code. NioPermissionLocalFileSystem.install(conf) checks what the file scheme resolves to without an override: - LocalFileSystem: uses the base class. - Hive's ProxyLocalFileSystem (classpaths with hive-exec, such as hudi-utilities, hudi-spark-client and hudi-spark): uses the nested WithHiveRename, which keeps Proxy's "rename onto an existing file fails" contract. - anything else: left alone, conf unchanged. It then replaces the current user's cached local file system and looks it up again so the cache holds the new instance. Closing the replaced instance deletes its deleteOnExit paths, as the Javadoc says. install() is called from UtilitiesTestBase and from HoodieClientTestUtils.overrideSparkHadoopConfiguration, the shared setup of SparkClientFunctionalTestHarness and HoodieSparkClientTestHarness. hudi-examples-spark gets the hudi-hadoop-common test-jar it would otherwise miss once a test there uses that harness setup. Measured locally: the whole utilities CI job (TestHoodie* unit-test step plus functional-test step), 2 alternating pairs, JaCoCo on, cut the unit step from 2022s/1954s to 1764s/1714s (about -12.5%). Every run had 440 unit tests (0 failures, 21 skipped) and 48 functional tests. Covered lines are the same in every Hudi module except HoodieMetadataTableValidator:1299. That line is in an RDD reduce that merges task results in completion order; reruns of the class hit and missed it on the same tree, so it is timing-dependent. In spark-client and hudi-spark test subsets, test counts are identical and chmod forks drop from 5,028/1,808/8,528/14,485 to 617/0/0/0. Only test code changes.
This branch has not been deployed
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
2 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
A GitHub CI run of
bot.ymlcosts about 620 job-minutes per push (reference: run 38362333401). About 508 of those minutes are test steps and about 102 are builds. This PR collects measured changes that cut that cost without deleting test areas or losing coverage. Each change is a separate commit with a self-contained message (what, why, measurements), so any one of them can be dropped or reverted on its own.Summary and Changelog
Every change was measured locally (JDK 17, 4 pinned CPUs, JaCoCo on): 20 runs before and 20 after, with random method order where relevant, plus JaCoCo intersection/union diffs. Every line that lost coverage was traced to another test that still covers it.
test(spark)TestHoodieFileSystemViewstest(common)TestBootstrapIndextest(utilities)DeltaStreamer matricestest(utilities)KafkaTestUtilstest(spark)TestColumnStatsIndexWithSQLtableVersionto the writer, so v6/8/9 are really written nowchore(ci)two surefire forkstest-spark-java17-java-tests-part1runs two test JVMs with per-fork ports and JaCoCo fileschore(ci)two surefire JVMs, parts 2/3 + spark-clientfunctional-tests[-b/-c]profiles take${forkCount}(default 1); CI passes 2 for Java tests parts 2/3 and spark-client. HiveServer2 and a DeltaStreamer ZooKeeper lock port made per-JVM. hudi-utilities stays at 1 JVM (two were slower on CI)perf(common)HadoopStorageConfiguration serdeConfiguration#write(which gzips every property's source list). Payloads written by older Hudi versions are still read via a fallbackperf(utilities)streamer clustering-conflict retry--clustering-conflict-retry-interval-seconds(default 60, the old hard-coded sleep); the conflict test sets 2s and still hits the reject-and-retry path once. Also fixesspark.ui.enable->spark.ui.enabledin UtilitiesTestBasetest(utilities)TestHoodieMetadataTableValidator fixtureperf(test)slow Spark SQL suitesmin(fileSize, defaultBufferSize)buffer instead of the 32MB local-FS block size allocated on every open (production code).HoodieSparkSqlTestBasesetsmax_expected_clock_skew_ms=1, removing 200ms lock-provider sleeps on clustering/compaction procedure paths. Fixes the straysetprefix on the max file-group key in TestHoodieBackedTableMetadataIndexLookuptest(spark)slow Spark functional teststestNIO local-FS permissionsNioPermissionLocalFileSystem(hudi-hadoop-common tests) sets rwx modes viaFiles.setPosixFilePermissionsinstead of forkingchmodper file when Hadoop native IO is absent (as on CI); falls back to Hadoop for special bits/native IO/non-POSIX.install(conf)keeps the classpath's rename contract (HiveProxyLocalFileSystem->WithHiveRename) and replaces the cachedfile:///instance, since the harnesses usenew Configuration(false). Wired intoUtilitiesTestBaseandHoodieClientTestUtils.overrideSparkHadoopConfigurationchore(ci)build oncetarget/+ hudi artifacts; test jobs restore instead of rebuildingTotal: about 100 job-minutes per push (~16-19%). Part1, the longest Java job, drops from ~74 to ~45 minutes. Wall-clock time is still bounded by the Scala "other" job (~74 min), which Maven forking doesn't help.
Passing GitHub Actions run (Java CI run 38865662941 on 21e4c5c vs master run 38839885993 on c167f7b): all 20 jobs passed. It used 540 job-minutes against 632 on master (-92, -15%). Part1 went from 75.5 to 46.5 min; utilities, spark-client and Scala DML jobs also ran shorter, partly from build-once and partly from runner variance. The longest job (Scala other) took 63.7 min against 72.2, and is still the critical path. Codecov: project coverage 80.73% -> 80.74%, with 11 more lines hit and all modified lines covered.
First GitHub Actions run (run 38851759537 vs master run 38839885993): every job passed except the part2 Quickstart step, fixed below. Restoring the shared builds took 0.5-2.6 min per job, with no recompilation. The part1 test step went from 68.7 to 46.7 min (-32%, 4211 tests), and the part1 job from 75.5 to 48.4 min. The run totalled 496 job-minutes against 632 on master; adding back the ~60 min part2 job that the failure skipped gives ~554 (-12%). The jdk11/Spark 3.5 build artifact is 3.5 GB, larger than the 1 GB I measured locally.
The Quickstart failure (
NoClassDefFoundError: HoodieTestUtilsinhudi-examples-spark) came from the two-fork commit: the test harnesses' port fields now callHoodieTestUtilsfrom the hudi-common test jar, whichhudi-examples-sparkdid not depend on. That commit now also addshudi-common:testsas a test dependency there.Run with the parts 2/3 + spark-client JVM commit (Java CI run 38909557846 vs 38865662941, same base): 540 -> 509 job-minutes, all jobs passing, so about 509 vs 632 on master (-19%). Single-run job times vary by up to ~10 min on jobs this commit doesn't touch (Scala DML +10.4 min, common/other -7.1 min), so per-job numbers above come from the changed jobs only. hudi-utilities was tried with two JVMs in that run and got slower (unit-test step 28:13 -> 30:40; each class ~1.7x longer), so it stays on one JVM.
Run on the 8-commit head without the serde commit (Java CI run 38925188336 on b42f5ce): all jobs passed, 536 job-minutes against 632 on master (-15%). That's above the ~507 I expected from the previous run; utilities (43.7 min), part2 (48.9) and common/other (58.0) ran longer than in run 38909557846, which again shows ~10 min per-job variance between single runs.
Not included (follow-ups): Scala suite parallelism, and Kafka-source tests that use their own producers without checking send results.
Impact
Mostly CI and test code. The one production change is
HadoopStorageConfiguration's Java serialization format. It speeds up every Spark task and Flink operator that ships a storage configuration, and keeps the same property values, including deprecated-key handling; only Hadoop's diagnostic per-property source list is dropped. Payloads from older Hudi versions still deserialize. An older Hudi reading the new format gets an empty configuration, so downgrading a Flink job onto state written by this version, or mixing Hudi versions between driver and executors, isn't supported. The test matrix changes are listed per commit. The one deliberate coverage tradeoff is no Spark-record StreamSync write to a v8 table (v8 still runs with Avro records; all combinations still run on the current version). The MDT-off file-system-view cases are removed because MDT-on is the supported mode, and the MDT-off writer paths stay covered byTestHoodieBackedMetadataandTestHoodieTableFileSystemView.Risk Level
medium. The two workflow changes are the risky part. Parallel test JVMs can expose shared-resource assumptions; this is mitigated by per-fork ports, 4 full-job runs and 60 repeated subset runs with random ordering. Build-once depends on restored outputs being treated as up to date (manifest checks on commit and profile set, re-stamped mtimes), and this PR's CI run is its first run on GitHub Actions. Both are single commits that can be dropped independently. The test-only commits are low risk.
Documentation Update
none
Contributor's checklist