fix(ci): stop smoke-boot.sh false-positive on jOOQ "tip of the day"#6336
Open
mengw15 wants to merge 8 commits into
Open
fix(ci): stop smoke-boot.sh false-positive on jOOQ "tip of the day"#6336mengw15 wants to merge 8 commits into
mengw15 wants to merge 8 commits into
Conversation
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6336 +/- ##
============================================
+ Coverage 66.04% 66.06% +0.01%
- Complexity 3380 3382 +2
============================================
Files 1142 1142
Lines 44980 44980
Branches 4948 4948
============================================
+ Hits 29709 29715 +6
+ Misses 13631 13626 -5
+ Partials 1640 1639 -1
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 386 | 0.236 | 25,734/33,734/33,734 us | 🔴 +23.6% / 🔴 +132.5% |
| ⚪ | bs=100 sw=10 sl=64 | 822 | 0.502 | 118,195/142,161/142,161 us | ⚪ within ±5% / 🔴 +41.7% |
| 🔴 | bs=1000 sw=10 sl=64 | 940 | 0.573 | 1,063,025/1,142,245/1,142,245 us | 🔴 +6.3% / 🔴 +18.0% |
Baseline details
Latest main afa8af2 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 386 tuples/sec | 432 tuples/sec | 831.69 tuples/sec | -10.6% | -53.6% |
| bs=10 sw=10 sl=64 | MB/s | 0.236 MB/s | 0.264 MB/s | 0.508 MB/s | -10.6% | -53.5% |
| bs=10 sw=10 sl=64 | p50 | 25,734 us | 20,821 us | 11,870 us | +23.6% | +116.8% |
| bs=10 sw=10 sl=64 | p95 | 33,734 us | 41,642 us | 14,509 us | -19.0% | +132.5% |
| bs=10 sw=10 sl=64 | p99 | 33,734 us | 41,642 us | 17,863 us | -19.0% | +88.9% |
| bs=100 sw=10 sl=64 | throughput | 822 tuples/sec | 848 tuples/sec | 1,078 tuples/sec | -3.1% | -23.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.502 MB/s | 0.517 MB/s | 0.658 MB/s | -2.9% | -23.7% |
| bs=100 sw=10 sl=64 | p50 | 118,195 us | 117,196 us | 93,851 us | +0.9% | +25.9% |
| bs=100 sw=10 sl=64 | p95 | 142,161 us | 139,814 us | 100,335 us | +1.7% | +41.7% |
| bs=100 sw=10 sl=64 | p99 | 142,161 us | 139,814 us | 109,028 us | +1.7% | +30.4% |
| bs=1000 sw=10 sl=64 | throughput | 940 tuples/sec | 956 tuples/sec | 1,100 tuples/sec | -1.7% | -14.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.573 MB/s | 0.583 MB/s | 0.671 MB/s | -1.7% | -14.7% |
| bs=1000 sw=10 sl=64 | p50 | 1,063,025 us | 1,046,062 us | 927,449 us | +1.6% | +14.6% |
| bs=1000 sw=10 sl=64 | p95 | 1,142,245 us | 1,074,196 us | 968,203 us | +6.3% | +18.0% |
| bs=1000 sw=10 sl=64 | p99 | 1,142,245 us | 1,074,196 us | 998,749 us | +6.3% | +14.4% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,517.91,200,128000,386,0.236,25734.42,33733.99,33733.99
1,100,10,64,20,2432.72,2000,1280000,822,0.502,118194.58,142160.53,142160.53
2,1000,10,64,20,21287.62,20000,12800000,940,0.573,1063024.78,1142245.40,1142245.40
smoke-boot.sh grepped the whole boot log for bare linkage-error class names, so jOOQ's random "tip of the day" banner -- which names NoClassDefFoundError / ClassNotFoundException in prose -- was read as a boot crash, failing the check at random for any jOOQ-using service on any PR even when the service booted fine. Match the exception as an actual thrown type instead (a fully-qualified java.lang.* name or an "Exception in thread" header), which real linkage failures produce but informational prose does not. Add a regression test (.github/scripts/test_smoke_boot.sh, discovered by the infra job) that reads crash_re out of smoke-boot.sh and asserts the jOOQ tip is not flagged while real thrown errors still are. Closes apache#6332.
mengw15
force-pushed
the
fix/6332-smoke-boot-jooq-false-positive
branch
from
3f6e16a to
92dced6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate flaky CI failures in the boot smoke checks caused by jOOQ’s random “tip of the day” mentioning exception class names in prose, by tightening the crash-detection grep pattern and adding a shell regression test to prevent reintroducing the false-positive.
Changes:
- Tighten
.github/scripts/smoke-boot.shcrash detection regex to match thrown linkage errors (and keep the Jackson Databind conflict sentinel) without matching prose mentions. - Add
.github/scripts/test_smoke_boot.shregression tests for the crash-detection regex. - Expand shell test discovery in
.github/workflows/build.ymlto include.github/scripts/.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/build.yml |
Runs shell test_*.sh suites from both bin/ and .github/scripts/ so the new regression test executes in CI. |
.github/scripts/test_smoke_boot.sh |
Adds regression coverage for smoke-boot.sh crash regex (including the jOOQ tip false-positive case). |
.github/scripts/smoke-boot.sh |
Updates crash_re to avoid matching bare exception names in informational logs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Meng Wang <mengw15@uci.edu>
Comment on lines
+59
to
+65
| # check exists to catch. Match the exception as an actual *thrown* type -- a | ||
| # fully-qualified java.lang.* name, or an "Exception in thread" header -- not the | ||
| # bare class name, which also shows up in harmless informational log prose (e.g. | ||
| # jOOQ prints a random "tip of the day" banner naming NoClassDefFoundError / | ||
| # ClassNotFoundException). Regression test: .github/scripts/test_smoke_boot.sh. | ||
| # See https://github.com/apache/texera/issues/6332. | ||
| crash_re='java\.lang\.(NoClassDefFoundError|ClassNotFoundException|LinkageError|NoSuchMethodError|AbstractMethodError|ExceptionInInitializerError|IncompatibleClassChangeError)|Exception in thread|requires Jackson Databind' |
Contributor
There was a problem hiding this comment.
can we in addition also check the return code from the process? if the process crashed, we get non 0 return code right?
Contributor
There was a problem hiding this comment.
The way to check exception names in stdout/stderr is too fragile. it could easily be flaky if in the future we print out some of the exception name.
Contributor
Author
There was a problem hiding this comment.
Done in cf1da71 — reworked to exactly this. smoke-boot.sh now decides from the process: reaching LISTEN is a healthy boot; exiting before LISTEN is a crash, and it reports the exit code; timeout is a hang. The crash_re stdout/stderr scan is gone, so log text — the jOOQ tip, or any future exception-name prose — can't trip it.
The one case this intentionally gives up is a service that logs a linkage error but still reaches LISTEN; I don't think that's worth a fragile string match — a health-check on the admin endpoint would be a more robust signal if we ever want it.
The regression test now drives smoke-boot.sh end-to-end (a healthy service that logs the jOOQ tip passes; a crasher and a hang fail).
Deciding a boot crash by grepping the service log for exception class names was fragile: any library that merely prints an exception name in prose (jOOQ's random "tip of the day" names NoClassDefFoundError / ClassNotFoundException) tripped it, failing the check at random. Drop the log scan. Base the verdict on the process itself: reaching LISTEN is a healthy boot; exiting before LISTEN is a crash (report its exit code); neither within the timeout is a hang. That is what actually distinguishes a boot crash and can't be fooled by log text. Rework the regression test to drive smoke-boot.sh end-to-end with fake launchers: a healthy service that logs the jOOQ tip must pass, a crasher must fail, a hang must time out. Closes apache#6332.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Meng Wang <mengw15@uci.edu>
Two robustness fixes from review of the process-based boot check: - Port ownership: the wait loop treats any LISTENer on the port as a healthy boot, so a leftover/unrelated listener could mask a crashed service. Fail fast if the port is already in use before launching, so a listener detected afterwards is the service we started. - Reap on early exit: stop_service returned without `wait` when the service exited during the SIGTERM grace period, leaving a short-lived zombie. wait it before returning. The regression test adds a case for the busy-port guard.
… cleanup Resolve the launcher glob to exactly one match and error otherwise, so a dist dir that accumulates multiple versions can't silently smoke-test the wrong (lexicographically-first) binary. Probe 127.0.0.1 instead of "localhost", so an IPv6-first name resolution can't report a service that binds IPv4 0.0.0.0 as "not listening". Add an EXIT trap that stops the service and removes the log on any exit, so an aborted run can't orphan the JVM or leak temp files.
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
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 changes were proposed in this PR?
The boot smoke check (
.github/scripts/smoke-boot.sh, from #6220) decided a service had crashed on boot by grepping its whole log for exception class names (NoClassDefFoundError,ClassNotFoundException, …). jOOQ prints a random "tip of the day" banner naming those exceptions in prose, so whenever it drew that tip the check reported a crash even though the service booted fine — a random failure on any jOOQ-using service, on any PR (e.g.config-serviceon #6331). Closes #6332.smoke-boot.shnow bases its verdict on process behaviour: reaching LISTEN is a healthy boot; exiting before LISTEN is a crash (it reports the process's exit code); neither within the timeout is a hang. This is what actually distinguishes a boot crash and can't be fooled by log text — no more grepping for exception-name strings..github/scripts/test_smoke_boot.shdrivessmoke-boot.shwith fake launchers: a healthy service that logs the jOOQ tip must pass, a crasher must fail, a hang must time out. Theinfrajob's shell-test discovery is broadened to.github/scripts/so it runs.Any related issues, documentation, discussions?
Closes #6332. Boot check from #6220; the false positive surfaced through the
platform-integrationboot checks (#6274). Per review feedback, the fragile log scan is replaced by a process-exit check.How was this PR tested?
.github/scripts/test_smoke_boot.shpasses locally and is deterministic across repeated runs: a healthy service that prints the jOOQ tip → OK; a service that exits on boot → FAIL (with its exit code); a service that never listens → FAIL (timeout).smoke-boot.shreports the crash's exit code, e.g.exited on boot (exit code 3) before listening on :<port>.infrajob discovers the test viafind bin .github/scripts -name 'test_*.sh';smoke-boot.shkeeps its executable bit.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-opus-4-8)