Repository navigation
ci: self-hosted high-CPU fast-feedback workflow + PR-scoped advisory PIT - #75
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned Files
|
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
❌ Mutation Testing (PIT) ReportPIT did not produce a report. Most commonly this means a test failed in the baseline (PIT runs all tests unmodified first to establish green) and PIT aborted before mutating. See the "Run PIT mutation testing" step logs for the failing test, then either fix it or add it to |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #75 +/- ##
============================================
- Coverage 78.03% 68.64% -9.40%
- Complexity 55 851 +796
============================================
Files 81 76 -5
Lines 4039 3789 -250
Branches 372 366 -6
============================================
- Hits 3152 2601 -551
- Misses 712 993 +281
- Partials 175 195 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…same-repo only) Optional, non-gating fast feedback that runs unit, integration and performance as parallel matrix jobs on a self-hosted many-core runner (label `highcpu`), guarded to same-repo PRs so fork code never executes on it. The GitHub-hosted gate in maven.yml stays the required check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
be55515 to
d24c8c1
Compare
…rkflow # Conflicts: # .github/actionlint.yaml
…-only The high-CPU self-hosted matrix now runs unit + integration + performance + mutation as parallel jobs (a strict superset of the mac runner, which is offline indefinitely). The mac workflow is switched to workflow_dispatch-only so it stops queuing dead jobs on every PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
failingHttpCall drove an HTTP call at a dotless bogus hostname ("xxxxxxxxx") and asserted the failure
was a DNS *resolution* error ("failed resolve"). That only holds where the name fails to resolve - on any
network with a local resolver + search domain (e.g. a Pi-hole LAN) the name resolves and the failure mode
becomes "connection refused", so the test fails there while passing on GitHub's public DNS.
Point the bad request at a closed local port (127.0.0.1:1) instead: an immediate, deterministic
"connection refused" on every environment, no DNS lookup at all (and faster than one). Keep the real
failed==true check; assert the deterministic "connection refused" message. Found bringing up the
self-hosted high-CPU runner.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…with failingHttpCall) The failingHttpCall fix (127.0.0.1:1 -> connection refused) also flipped this sibling test through the shared getBadRequest() helper, so its assertion is updated to match. Caught by ce-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Load was ~18/32 with 34GB free - the box is I/O-bound (cold cache), not CPU-bound - so use all the cores. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…ateCommittedOffsetIT Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…epo guard (ce-review P2) The if: guard lives in the PR-head file, so the real RCE backstop is the repo Actions setting "require approval for all outside collaborators". Documented in the workflow header + referenced SELF_HOSTED_RUNNER.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
5ec35d4 to
5cc2e87
Compare
Unit forkCount 1C->2C, Integration 8->16 (probe the oversubscription ceiling). Mutation PIT threads 2->16 (CPU-bound, stays in the matrix). Performance split into its own job that needs the matrix, so the throughput benchmark measures on a quiet box (solo 1m54 vs 7m18 when it shares the box). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…formance Doubling was flat (unit 2C=1C) to worse (integration 16 slower than 8), so back to the knee. Performance returns to the concurrent matrix - it is a correctness gate here; noisy timing is fine and isolation wrecked total wall-clock (it queued behind a 17-min mutation). Mutation keeps threads=16. Accurate benchmarking -> separate on-demand run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Unit/Integration now also enable JUnit method-parallelism (-Dparallel-tests=true) on top of forking, forks reduced so forks x threads ~= cores - tests whether thread-parallelism is still flaky after recent fixes (revert to fork-only if red). ci-mutation-test.sh: on a PR (GITHUB_BASE_REF) mutate ONLY changed core classes, skip if none - the full internal.* sweep has never completed. Full sweep still runs for push/nightly (no base ref). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…eads finding fork×threads (parallel-tests=true) was safe (threading no longer races - unit green; integration red was the known flaky PartitionStateCommittedOffsetIT) but no faster than fork-only, so back to 1C/8. PIT: grep no-match under set -euo pipefail made the changed-class skip exit non-zero (0m19 red); wrap grep with || true so it exits 0. Recorded the thread-parallelism-vs-forking finding in inflight next to the shelved Step-2 question. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…bserve a green board TEMPORARY - not a fix. Disables the known-flaky PartitionStateCommittedOffsetIT.committedOffsetRemoved (awaitility timeout, in inflight, fails on GitHub-hosted too) purely to see a fully-green highcpu run. Revert and harden the test properly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
Now that ci-mutation-test.sh scopes PIT to changed classes on PRs (GITHUB_BASE_REF), it is fast enough for the 2-core hosted runner - so mutation runs on GitHub-hosted as well, not just the self-hosted box. Non-gating (continue-on-error). fetch-depth: 0 so the changed-class diff resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
…already passed Each GitHub-hosted test job, before running its slow 2-core suite, polls whether the self-hosted highcpu equivalent for the same commit already PASSED (ci-concede-check.sh). If so it skips and goes green; the hosted job stays the required gate but finishes faster when grumpy wins. Never depends on the self-hosted runner: offline/queued/slow/failed -> run the tests normally. Bounded wait, continue-on-error so a check bug can never fail the gate. Caveat documented: conceding trusts the self-hosted env (env-specific failures could be missed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
… the green-board experiment) Reverts the temporary experiment disable. The test is still the known flaky awaitility timeout (tracked in inflight) - harden it (adaptive await / underlying timing) rather than leave it disabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
The check name is "<workflow> / <job>", and both halves carried "High-CPU"
plus verbose filler ("Build & Test - ...", "self-hosted, optional"), pushing
past the PR merge-box truncation width.
- workflow name: "PR High-CPU Fast Feedback" -> "highcpu"
- job name: "Build & Test - <suite> (high-CPU self-hosted, optional)"
-> "<suite> (optional)" (keeps the non-gating signal)
Result: "highcpu / Unit (optional)", "highcpu / Integration (optional)", etc.
- longest ~35 chars, well under the cutoff.
Kept the concede matcher in sync (it resolves the highcpu run/jobs by name):
bin/ci-concede-check.sh WORKFLOW default -> "highcpu", JOB_PREFIX -> "${SUITE}"
(the job is now named "<suite> (optional)", so a suite-name prefix still
uniquely matches). Added a keep-in-sync comment on the job. Updated the
SELF_HOSTED_RUNNER.md reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytagqk6daL4uTtbebQNNv
… behind Mutation's slow cancel A single workflow-level concurrency group meant every new push's jobs waited for the ENTIRE previous run to complete before starting. cancel-in-progress signalled the old run immediately, but its Mutation (PIT) maven/JVM tree rides out the runner's full cancellation grace (~5 min) before force-kill - observed on PR #80: push at 22:41:05, old Mutation force-killed 22:46:06, all four new jobs started 22:46:09. Head-of-line blocking on the one job nobody needs fast feedback from. Job-level per-suite groups (highcpu-<suite>-<branch>) mean a new push supersedes each suite independently: Unit/Integration/Performance predecessors are finished or die fast, so they start within seconds; only Mutation queues behind old Mutation, which is fine for a 30-minute advisory job. No change to runner usage. actionlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
…ner' in comments Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtaCN1Re14pJ99VFW4R18
a4aad11 to
6b6383f
Compare
…(review) Two findings from the automatic review on #75: - git diff now uses --diff-filter=d so a PR whose only core change is a class DELETION does not name a non-existent class in targetClasses (pitest failWhenNoMutations=true would fail the goal instead of the intended nothing-to-mutate skip). - emit Foo,Foo$* per class instead of Foo*: PIT globs * as unbounded .*, so Foo* over-matched siblings (PartitionState* -> PartitionStateManager), mis-attributing mutants. Foo,Foo$* = the class + its nested/synthetic members only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e (review) Review finding: with set -euo pipefail, if the base-ref fetch fails or the ref is absent on a shallow checkout (e.g. the highcpu Mutation job, which has no fetch-depth:0), the git diff would hard-crash with exit 128 instead of the intended skip/full-sweep. Guard the PR-scoping on git rev-parse verifying origin/BASE_REF resolves; otherwise fall back to the full internal.* sweep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks - worked through all four inline findings on
(#1/#2 = one commit; #3 = one commit. #4 pending the above.) |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Resolve the finding that highcpu Mutation was silently PR-scoped (GITHUB_BASE_REF is set on self-hosted pull_request runs too), contradicting the full-sweep docs. Rather than pick one, run both experimentally: split the matrix Mutation entry into Mutation (PIT, scoped) and Mutation (PIT, full). The full one sets PIT_FULL_SWEEP=true, a new explicit override in ci-mutation-test.sh that ignores the PR base ref and mutates all of internal.*. Also add fetch-depth: 0 to the highcpu checkout (repo is tiny) so the scoped job reliably resolves the PR base ref. Docs updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @astubbs's task in 2m 16s —— View job Code review — PR #75Todo list:
Note: invoking the Where things standAll four findings from the previous automated review round on
I re-checked the wiring end to end:
No new issues found in this pass. One thing worth the maintainer's awareness rather than a blocking finding: the new "Mutation (PIT, full)" job runs the sweep the script's own comment describes as "impractically slow (it has never completed on CI)" under a 60-minute job timeout — that's clearly intentional/experimental per the commit message ("run scoped AND full mutation jobs side by side... see how they compare on cost/signal"), so it's flagged here only as a note, not a defect. |
|
Finding #4 resolved ( |
…a no-op) (review)
Review finding: the pom pins <forkCount>${surefire.forkCount}</forkCount>, so surefire only honours -Dsurefire.forkCount; a bare -DforkCount is silently ignored (harmless today since the ci profile already sets surefire.forkCount=1C, but the documented cap mechanism did not actually work). Use the namespaced property on the highcpu Unit line and correct the ci-unit-test.sh usage note. (Integration/failsafe is unaffected - it has no forkCount in the pom, so it reads the bare property.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve AGENTS.md (combine all four PR Discipline bullets - #75s three + #95s use-the-template rule), CHANGELOG.adoc (keep #75s renamed local line + highcpu/PIT entries alongside #95s PR-checklist entry; drop the stale macOS line the rename supersedes), and README.adoc (regenerated from the resolved CHANGELOG). Brings in #92/#95/#96 (copyright-in-mvn, PR checklist gate, track_progress) and dep bumps.
|
Claude finished @astubbs's task in 1m 37s —— View job Claude finished @astubbs's task —— View jobTodo list:
Code review — PR #75 (latest push:
|
|
5th finding (Unit |
…- merge-train prep (#83 lands after #75) Conflict resolutions (unions of both sides): - actionlint.yaml runner labels: performance, macOS, highcpu, local - bin/ci-unit-test.sh: keep this branch chaos-inclusive exclusion list (performance,chaos,quarantined - quarantine contract) + PR #75 argument passthrough ("$@") Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ALhTf1TRA2S6Ue5rkiQdK
Reply to a review comment in its own thread and mark it resolved when addressed, rather than a top-level PR comment (which leaves the conversation unresolved and blocks merge). Recording the rule that surfaced while handling PR #75 review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The gate only failed on unchecked boxes that were present, so a human PR that dropped the template entirely (e.g. gh --body-file) had zero boxes and passed vacuously - the exact bypass PR #75 slipped through. Now: real bot authors (GitHub user type Bot, e.g. Dependabot/Renovate) are exempt; every other PR must include the checklist (>=1 task-list item) and resolve each box ([x] or N/A - <reason>). A human PR with no checklist fails with a message pointing at PULL_REQUEST_TEMPLATE.md. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hecklist gate (#97) Resolutions: newer #97 checklist wording in CHANGELOG/README/AGENTS (keeping this branch's in-thread review-response policy bullet alongside); ci scripts keep the chaos-inclusive exclusion list per the quarantine contract test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ALhTf1TRA2S6Ue5rkiQdK
…double copyright headers; ledger cleanup - Remove the concede optimizer that rode in via the pre-session ci/grumpy-runner-workflow merge: maven.yml restored to master (byte-identical), bin/ci-concede-check.sh deleted. It was removed from PR #75 by a 10-reviewer ce-review as a P0 gate-spoof (a PR could add a workflow named "highcpu" with a trivially-passing job and make the REQUIRED gate skip real tests) - inflight.md documents why it stays discarded. It should never have been in this PR. - Collapse the doubled Confluent copyright header (a maven-license-plugin confluentinc#888 artifact) in the three files that carry it: BrokerIntegrationTest, KafkaClientUtils (both touched by this PR) and RetriesTest. Checker still 0 violations across 232 files. - Ledger: delete the #98 entry (self-marked, #98 merged as 6d39081); shrink #80s own now-solved committedOffsetRemoved/drain narrative to a pointer + residual open tracking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018igSVt74wPQAbRNHkXS4R8
… partitions (confluentinc#857 family) (#80) One flaky CI check (PartitionStateCommittedOffsetIT.committedOffsetRemoved[latest]) concealed two independent bugs - one real product defect in the close path, one test-harness race. Both are fixed here, each proven RED->GREEN against its captured defect. Bug 1 (product): a draining consumer busy-spins and zombie-holds its partitions. ConsumerManager.shutdownRequested was a private shadow of BrokerPollSystem.runState, raised at drain() time, so poll()'s guard short-circuited and consumer.poll() was never invoked during the drain window. The poll loop busy-spun (~10k iterations/s, a full core per closing instance), and - because rebalance participation lives inside poll() - the draining consumer became a rebalance-unresponsive zombie: heartbeats kept it a live group member holding its whole assignment (up to max.poll.interval.ms) while consuming nothing, starving same-group siblings. Fix: collapse the duplicated lifecycle state - the flag and signalStop() are deleted; ConsumerManager derives "close in progress" from runState (now volatile) via an injected signal. One source of truth, so the desync class is structurally impossible. Drain semantics preserved: the member keeps its partitions to finish in-flight work and commit, but now stays rebalance-responsive. Guards: BrokerPollSystemDrainTest (unit; characterisation flipped RED->GREEN) and DrainingMemberRebalanceIT (integration). Bug 2 (test harness): the auto.offset.reset=latest nudge race. A LATEST-reset consumer with no committed offset resolves its start position when the reset executes; runPcUntilOffset produced its single nudge before the await, so under contention the reset resolved after the nudge and positioned the consumer past every record that would ever exist - the await was unwinnable at any timeout (only the [latest] param ever failed). Fix: shared BrokerIntegrationTest#awaitWithTopicNudge produces a nudge inside each await attempt and self-diagnoses on timeout; both helpers delegate to it - no timeout enlarged, no assertion weakened. LatestResetTailNudgeIT bottles the race deterministically. Also: - Re-enable the two quarantined tests this PR owns (ChaosChurnStormIT.churnStormMeetsSlosAndBalancesLedger, PartitionStateCommittedOffsetIT.committedOffsetRemoved): their fixes are this PR, so the @Quarantined annotations and docs/QUARANTINED_TESTS.md entries are removed. - Add DEBUG-gated, behaviour-preserving diagnostics (ShardManager under-served detector, WorkManager throttle-decision log, per-PC myId log attribution, logback test config). - Docs: research write-ups under docs/solutions/test-flakiness/, a plan under docs/plans/, a CHANGELOG entry for the operator-visible drain fix, and inflight-ledger upkeep. - Repo hygiene: remove a stale CI "concede" mechanism that rode in via an old branch merge (previously rejected from #75 as a required-gate spoof; maven.yml is back to master's version); collapse three doubled copyright headers; prune closed docs/inflight.md entries. Relates to upstream confluentinc#857 (paused consumption after rebalance). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stacked-PR gap is closed: an "All branches: PR dependency gate" ruleset (~ALL, requiring only "Check PR Dependencies") now covers PRs whose base is a feature branch, which the master-only ruleset never matched. Verified live on #112. The concede optimizer is abandoned, not parked - it was removed from #75 by review, re-introduced and dropped again on #80, and highcpu stays purely advisory. A five-point revival checklist for something nobody intends to revive is the "keep it in case" habit this file is meant to resist; the findings survive in #75's and #80's review history if it ever comes back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Optional, non-gating fast feedback on a self-hosted many-core Linux runner - far faster than GitHub's 2-core gate for the Testcontainers-heavy suites - plus a couple of related CI/test fixes that landed while building it.
What
pr-highcpu-fast-feedback.yml: matrix of Unit / Integration / Performance / Mutation (PIT, scoped) / Mutation (PIT, full) onruns-on: [self-hosted, highcpu], same-repo guarded (fork PRs never run on the self-hosted box),continue-on-error+ not required so an offline runner never blocks a merge (the required gate stays GitHub-hosted,maven.yml). Triggers onpull_request+workflow_dispatch; per-suite concurrency so fast suites don't queue behind Mutation's slow cancel. Tuning (16c/32t box): unit-DforkCount=1C, integration-DforkCount=8per-broker, mutation-Dthreads=16.maven.yml):bin/ci-mutation-test.shnow scopes PIT to the core classes changed vs the PR base (wildcarded so nested/synthetic members are covered), fast enough for the hosted gate; advisory /continue-on-error.bin/ci-unit-test.shforwards extra Maven args (mirrorsci-integration-test.sh) so the self-hosted forkCount cap can be passed - no change for existing callers.pr-mac-fast-feedback.yml→pr-local-fast-feedback.yml: vendor-neutral name + a customlocalrunner label (was the automacOSlabel); staysworkflow_dispatch-only while that runner is retired.actionlint.yamlnow declaresperformance/highcpu/local.failingHttpCall/testVertxFunctionFaildeterministic - drive a closed local port (127.0.0.1:1→ "connection refused") instead of a bogus hostname that relied on DNS failing to resolve (false on any network with a local resolver). Both siblings sharegetBadRequest().SELF_HOSTED_RUNNER.md,inflight.md, andCHANGELOG.adocBuild & CI entries for the highcpu lane + PR-scoped PIT (README regenerated). Also widens the AGENTS.md changelog policy to record notable build/CI/tooling changes (this is a technical library; its contributors/agents are a primary audience) rather than exclude them as noise.AGENTS.md: add a PR Discipline section (keep PR title/body in sync without churn; duplication-report follow-up; stacked-PRdepends on #N) - mirrors the maintainer's global agent rules into the repo so contributors get them.Security
Public repo + self-hosted runner: the same-repo
if:guard keeps fork PRs' untrusted code off the box, backed by the repo setting Settings → Actions → "Require approval for ALL outside collaborators" (the in-file guard alone is editable by a fork PR). Confirmed on.Note
When the
highcpurunner is offline the checks simply pend (never gate). The generic runner infrastructure (LXC via OpenTofu + Ansible, boot control) lives in the homelab repo - not parallel-consumer-specific.Checklist
CHANGELOG.adoc, under== Unreleased) - Build & CI entries for the highcpu lane + PR-scoped PITSELF_HOSTED_RUNNER.md,inflight.md, regeneratedREADME.adocfailingHttpCall/testVertxFunctionFail)if:guard + "Require approval for ALL outside collaborators"; see the Security section above🤖 Generated with Claude Code