[opt](test) Avoid a 10s heartbeat wait in every FE unit test class - #66195
Open
morningman wants to merge 3 commits into
Open
[opt](test) Avoid a 10s heartbeat wait in every FE unit test class#66195morningman wants to merge 3 commits into
morningman wants to merge 3 commits into
Conversation
Every test class deriving from TestWithFeService spends ~12s in createDorisCluster(), of which ~10s is pure idle waiting. Daemon.run() executes one cycle before sleeping, so HeartbeatMgr's first cycle completes while createDorisCluster() has not registered any backend yet. checkBEHeartbeat() therefore has to wait for the second cycle, which is heartbeat_interval_second (10s by default) away. Two changes: - Set heartbeat_interval_second to 1 at the start of beforeAll(). It is placed before beforeCreatingConnectContext() so subclasses that set the interval explicitly (DecommissionBackendTest, BeDownCancelCloneTest) still win, and before the Env singleton is created so HeartbeatMgr picks up the new interval. It is deliberately not put into MockedFrontend's MIN_FE_CONF, because DorisFE.start() would then load it from fe.conf and silently override those subclasses. - Make checkBEHeartbeatStatus() check before sleeping and poll at 20ms instead of sleeping a fixed 1s before each check. The overall timeout budget is unchanged. Measured on 20 nereids command test classes (43 tests, all passing before and after): fork=1 wall 399s -> 211s, reported test time 283s -> 95s fork=4 wall 399s -> 67s A synthetic class that only starts the FE and asserts once goes from 12.0s to 2.83s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
morningman
marked this pull request as ready for review
July 28, 2026 15:26
Contributor
TPC-H: Total hot run time: 29158 ms |
Contributor
TPC-DS: Total hot run time: 176627 ms |
Contributor
ClickBench: Total hot run time: 24.88 s |
Every FE unit test class pays a full FE startup. Measured on the FE UT CI
agent (build 1008420), that fixed cost is about 54s per class: ~16s of JVM
fork and class loading (measured from test classes that never start an FE)
plus ~38s of FE startup.
210 of the 214 nereids test classes that start an FE finish in under 90s,
i.e. they are almost entirely that fixed cost. Merging classes that already
share the exact same fixture removes it without changing any test.
This first batch merges 12 classes that all extend SqlTestBase and override
no setup at all, so they already ran against an identical fixture (database
"test" with tables T0..T4):
sqltest/SqlPlanSuiteTest <- CascadesJoinReorderTest, InferTest,
JoinTest, MultiJoinTest, SortTest
rules/exploration/mv/MvExplorationSuiteTest <- BuildStructInfoTest, EliminateJoinTest,
HyperGraphAggTest, HyperGraphComparatorTest,
NullRejectInferenceTest
rules/rewrite/RewriteRuleSuiteTest <- PruneOlapScanTabletTest,
EliminateConstHashJoinConditionTest
Test bodies are moved verbatim; method names are unchanged so test reports
keep the same identifiers. Each merged file keeps a "from <OriginalClass>"
section header, so grepping the old class name still lands on its tests.
Verified: 53 tests before, 53 tests after, all passing.
Deliberately not merged here:
- nereids/mv (6 classes): they create materialized views with colliding names
inside their test methods, so sharing one FE makes them interfere. Needs
namespace isolation first.
- jobs/joinorder/hypergraph (4 classes): testPullUpQueryFilter exists with
different bodies in two of them.
- Classes with real work (DistributeHintTest 764s, OtherJoinTest 256s,
GraphSimplifierConsistencyTest 223s, PreMaterializedViewRewriterTest 205s)
are intentionally left alone: merging them would create a straggler that
hurts wall clock under the 12-way parallel CI run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
run feut |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
… 95% nereids/jobs/joinorder/joinhint/DistributeHintTest was the slowest class in the whole FE UT suite: 764s in CI build 1008420, 459s in 1008517. The 40% swing is not noise, the test is unseeded and the amount of work it does varies per run. It also had no assertions at all, and never had any since it was added in apache#28562: mismatches were counted into a field and printed to stdout, so no optimizer regression could ever fail it. Two of its 182 local seconds were useful; the rest was enumeration that could not report anything. Both properties it covers are worth keeping, so the tests are kept and made able to fail: - testHintJoin: a distribute hint (broadcast / shuffle) selects a physical strategy and must never change the result. This is the only coverage of distribute hints over random join graphs. Enumeration unchanged (~910 optimizations, ~5s); it now asserts. - testLeading: reordering *inner* joins is always semantics-preserving, so any leading order must give the same result as the un-hinted plan. It now builds inner-join-only graphs via the existing HyperGraphBuilderOld(Set<JoinType>) constructor, draws permutations from a fixed seed, and asserts. Enumeration is cut from ~51000 optimizations to ~280, which is what made the class slow. The previous testLeading applied a random permutation to graphs that also contained outer / semi / anti joins, where an arbitrary leading order is generally not a valid join order. Asserting on that case fails reproducibly (3/3 runs, down to 3 tables): extra null-padded rows appear. Whether Doris should reject such a hint instead of applying it is a question for the Nereids owners, so that case is kept as a @disabled test documenting the repro rather than asserting behaviour nobody has decided. Local runtime: 182s -> 9-10s over three runs, all passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
run feut |
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.
What problem does this PR solve?
Every test class deriving from
TestWithFeServicespends about 12 seconds increateDorisCluster(), and roughly 10 of those seconds are pure idle waiting.Daemon.run()runs one cycle before sleeping, soHeartbeatMgr's first cycle completes whilecreateDorisCluster()has not registered any backend yet.checkBEHeartbeat()then has to wait for the second cycle, which isheartbeat_interval_second(default 10) away.This is not a niche cost: 304 test classes in fe-core start an FE, and 215 of them (71%) live under
nereids— which is whynereidsdominates FE UT wall time.Evidence that this is a timer rather than real work: six synthetic classes that only start the FE and assert once took 11.95 / 12.07 / 12.06 / 11.93 / 12.08 / 12.08 s — a spread of ±0.08s.
What is changed?
heartbeat_interval_second = 1at the start ofbeforeAll().beforeCreatingConnectContext()so subclasses that set the interval explicitly (DecommissionBackendTest,BeDownCancelCloneTest) still override it.Envsingleton is created, soHeartbeatMgrpicks up the new interval.MockedFrontend.MIN_FE_CONF:DorisFE.start()would load it fromfe.confand silently override those subclasses.checkBEHeartbeatStatus()check before sleeping, and poll at 20ms instead of sleeping a fixed 1s before each check. The overall timeout budget is unchanged.Measurements
20
nereids.trees.plans.commandstest classes, 43 tests, all passing before and after:fork=1fork=1fork=4A synthetic class that only starts the FE and asserts once: 12.0s -> 2.83s.
Also verified separately:
DecommissionBackendTest(overrides the interval to 5) andTabletHealthTest(exercisescheckBELostHeartbeat) pass.Release note
None
Check List
🤖 Generated with Claude Code