Skip to content

Commit bcf049e

Browse files
astubbsclaude
andcommitted
merge(streams) #255: bring the classifier cache-warm down to the error-surfacing rung
Merges `feats/ks-streams-task-lifecycle` (#394), this PR's BASE, which has just taken #379's post-cut work. Six commits arrive; the only code among them is the `prepare-deps` warm for Kafka's `sources` and `test-sources` classifier jars, which closes this rung's share of the Maven Central coin flip - the failure this branch spent four commits recording sightings of. **This merges the base rung, NOT master, and that is a correction rather than a shortcut.** The task was framed as "merge origin/master so #395 stops reading CONFLICTING", but this PR's base is `feats/ks-streams-task-lifecycle`, not master - so GitHub's CONFLICTING is computed against that rung, and master cannot clear it. The evidence is direct: the sole conflicting path, `docs/inflight/ci-streams-classifier-artifacts-escape-the-cache-warming-job.md`, **does not exist on master at all**, so a master merge leaves the collision untouched while putting this rung 75 commits ahead of its own base and inflating a stacked PR's diff with work no rung below it carries. The whole spine still sits on `b2e6c190d`; changes travel DOWN it by merging the rung below, which is what the three merges arriving here did. One real conflict, and the incoming file left the instruction for it. - `docs/inflight/ci-streams-classifier-artifacts-escape-the-cache-warming-job.md` - resolved to the incoming POST-FIX version's structure, as #379 instructed in the note's own `post-merge` block: take the version that names the warm step, not the one that still calls the fix a candidate. Taken as a straight side, though, that instruction would have dropped four commits of evidence it was never written about: it names the *lifecycle* rung, whose copy was the pre-fix one unchanged, while THIS rung had since added its own sightings to it. So the branch's findings are kept, condensed into a new section written in post-merge terms and marked `post-merge: checked` - the second independent sighting that made this master-state rather than one PR's problem, that Unit and Integration fail together because they share the runner's route to Central, that a re-run is a coin flip rather than a fix, and the markdown-only control arm that settles the attribution. All three remain true for any branch that has not yet merged the warm forward, which is exactly the population the post-fix version's "What is still open" section addresses. Nothing either side wrote and still holds was dropped; what went is the superseded framing of the defect as unfixed, whose reasoning the workflow step's own comment now owns. One sentence was moved to the past tense rather than carried verbatim: the incoming copy says `feats/ks-streams-task-lifecycle` still holds the pre-fix file and will collide add/add. Both rungs that hit that collision have now resolved it as instructed, so as written it was a claim about NOW that this merge falsifies - the exact rot `bin/check-branch-self-reference.sh` exists to catch. The instruction is kept for any rung above these two that has not yet merged forward. `.github/workflows/maven.yml` and `docs/inflight/test-streamthreadtest-invalid-timestamps-flake.md` auto-merged as pure incoming additions - the warm step, and the example rung's two CI sightings. No decision this branch recorded is reversed by the incoming side. Verification. The merge changes **no Java and no pom** - `git diff --stat HEAD` against the pre-merge tip is the workflow file and two markdown notes - so no module's compilation or test outcome can move, and none was re-run on that basis rather than on assumption. `bin/check-all.sh`: 15 ran, 13 passed, 2 failed, and **both failures were reproduced on the pre-merge tip 228cebc in a detached worktree**, so neither is this merge's: - `check-file-refs.sh` - `docs/BUG_857_INVESTIGATION.md`, cited by three `docs/solutions/` write-ups, does not resolve on this branch. Identical before and after. - `check-quarantine-owners.sh` - `ProducerManagerTest.producedRecordsCantBeInTransactionWithoutItsOffsetDirect` names owner PR #262, which has merged since this branch was cut while the quarantine stayed. Live GitHub state, not tree state. `check-branch-self-reference.sh` and `check-issue-refs.sh` both pass, including the new section's `post-merge: checked` block. The repo's pre-commit gate evaluates a different worktree (#382), so this commit uses `--no-verify`; the gates above were run in this worktree instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rEzrWYFr6oEzy6porczd3
2 parents 228cebc + 9b28f2c commit bcf049e

3 files changed

Lines changed: 120 additions & 75 deletions

File tree

.github/workflows/maven.yml

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,64 @@ jobs:
5555
setup-java-Linux-x64-maven-
5656
- name: Download all dependencies
5757
run: ./mvnw --batch-mode -Pci dependency:go-offline -DincludeScope=test -U
58+
# The plugin-resolves-for-itself class, reached from a new direction: here the plugin doing its
59+
# own resolving is maven-dependency-plugin. `parallel-consumer-streams` unpacks Kafka's
60+
# `sources` and `test-sources` CLASSIFIER jars through two `dependency:unpack` executions that
61+
# name them as <artifactItems>, and go-offline resolves the DECLARED dependency graph - an
62+
# artifactItem is not in it, and neither jar is declared anywhere else. So both stay cold, and
63+
# every lane that builds this module fetches them from Central inside generate-sources, on the
64+
# CDN route lottery in
65+
# docs/solutions/build-errors/maven-central-timeout-azure-west-regions-2026-04-21.md: an
66+
# exactly-240s read timeout, and re-running does not reliably help because the runner is often
67+
# reassigned to the same region. Seen on astubbs/parallel-consumer#394 and
68+
# astubbs/parallel-consumer#395 - Unit AND Integration both red at
69+
# `unpack (unpack-kafka-streams-sources)` with ZERO tests run, so the lane called "Unit Tests"
70+
# names a subsystem that never got as far as compiling.
71+
#
72+
# Warmed by COORDINATE rather than by building the module. Building it warms the jars too, and
73+
# is what the Connect spike branch does (`-pl parallel-consumer-connect -am -DskipTests
74+
# package`) - but it costs a full build of core inside this job, and that branch records why it
75+
# cannot be shortened to a cheap phase-only walk: the module's test-scope resolution wants
76+
# core's tests-classifier jar, which does not exist until core reaches `package`. Two jars are
77+
# the whole gap here and `kafka.version` is a real root-pom property, so naming them is both
78+
# cheaper and no more prone to rot.
79+
#
80+
# Verified locally against an empty local repository: the two `get`s fetch exactly the two jars
81+
# the assertions name, after which `-pl parallel-consumer-streams -am generate-test-sources`
82+
# runs OFFLINE from a cleaned target and both unpack executions succeed - so what this warms is
83+
# sufficient for the artifactItems resolve, not merely adjacent to it.
84+
#
85+
# `kafka.version` is the default every live lane builds at. The one job that overrides it,
86+
# `test-kafka-compat`, is `if: false`; re-enabling it puts its Kafka version back outside this
87+
# warm. (astubbs#255)
88+
- name: Warm the Kafka sources jars the streams module unpacks
89+
run: |
90+
set -euo pipefail
91+
evaluate() {
92+
./mvnw -q --batch-mode -N \
93+
-Dexpression="$1" -DforceStdout \
94+
org.apache.maven.plugins:maven-help-plugin:3.5.2:evaluate 2>/dev/null | tail -1
95+
}
96+
# A renamed property makes help:evaluate print "null object or invalid expression", which
97+
# dependency:get would happily turn into a warm of the wrong coordinate - and warming
98+
# nothing is indistinguishable from a cache hit until it fails a job downstream. Require a
99+
# version-shaped answer rather than merely a non-empty one.
100+
require_version() {
101+
case "$2" in
102+
[0-9]*) : ;;
103+
*) echo "::error::$1 did not resolve to a version from the root pom (got '$2') - the Kafka sources cache warm would be a silent no-op"; exit 1 ;;
104+
esac
105+
}
106+
kafka_version=$(evaluate kafka.version)
107+
require_version kafka.version "$kafka_version"
108+
for classifier in sources test-sources; do
109+
./mvnw --batch-mode -ntp \
110+
org.apache.maven.plugins:maven-dependency-plugin:3.11.0:get \
111+
-Dartifact="org.apache.kafka:kafka-streams:${kafka_version}:jar:${classifier}"
112+
# Assert the jar is on disk: the exit code says the goal ran, only the file says the
113+
# cache this job exists to populate actually got populated.
114+
test -s ~/.m2/repository/org/apache/kafka/kafka-streams/"${kafka_version}"/kafka-streams-"${kafka_version}"-"${classifier}".jar
115+
done
58116
- name: Save Maven cache (rotating key)
59117
if: success()
60118
uses: actions/cache/save@v4
Lines changed: 55 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,89 +1,69 @@
1-
# The streams module's `sources`/`test-sources` artifacts are not pre-warmed, so every run of it fetches from Central live
1+
# The streams module's `sources` jars are warmed HERE, and nowhere else yet
22

33
<!-- inflight-type: bug -->
44
<!-- inflight-impact: misdirection -->
55

6-
<!-- post-merge: checked-begin -->
7-
`parallel-consumer-streams` (astubbs#255). First seen as a red Unit lane on the task-lifecycle rung,
8-
astubbs/parallel-consumer#394, which had **nothing to do with the tests** on it. Seen again, with the
9-
same signature, on the error-surfacing rung stacked above it, astubbs/parallel-consumer#395 - two
10-
independent branches, which is what makes it master-state rather than one PR's problem.
11-
<!-- post-merge: checked-end -->
12-
13-
## What it looks like
14-
15-
The Unit lane goes red with the streams module `FAILURE` and every other module `SUCCESS`, at
16-
`maven-dependency-plugin:unpack (unpack-kafka-streams-sources)`:
17-
18-
```
19-
Could not transfer artifact org.apache.kafka:kafka-streams:jar:sources:3.9.2
20-
from/to central (https://repo1.maven.org/maven2/): Read timed out
21-
```
22-
23-
**No test ran.** The module dies in `generate-sources`, so a reader looking for a failing test finds
24-
none, and the lane's name says "Unit Tests". That is the misdirection: the signal names the wrong
25-
subsystem.
26-
27-
## Why the cache-warming job does not cover it
28-
29-
`prepare-deps` in `.github/workflows/maven.yml` warms the cache with `dependency:go-offline`, which
30-
resolves the **declared dependency graph**. This module additionally fetches artifacts by explicit
31-
`artifactItems` in two `dependency:unpack` executions - Apache Kafka's `sources` and `test-sources`
32-
classifiers - and those are not part of that graph. So they are downloaded live from Maven Central on
33-
every run of this module, in exactly the phase where the region-dependent timeout class already
34-
documented in
6+
`parallel-consumer-streams` (astubbs#255) fetches Kafka's `sources` and `test-sources` classifier
7+
jars through `dependency:unpack` `<artifactItems>`, which `dependency:go-offline` does not resolve -
8+
so they were fetched live from Maven Central inside `generate-sources` on every lane, in exactly the
9+
phase where the region-dependent read timeout in
3510
[`docs/solutions/build-errors/maven-central-timeout-azure-west-regions-2026-04-21.md`](../solutions/build-errors/maven-central-timeout-azure-west-regions-2026-04-21.md)
36-
bites.
37-
38-
That write-up's own conclusion is the relevant one: **re-running does not reliably help**, because the
39-
runner is often reassigned to the same region, and the fix that worked was pre-warming so nothing is
40-
fetched from Central during the build. This module is the one place that was left outside the fix.
11+
bites. It presented as Unit **and** Integration red at `unpack (unpack-kafka-streams-sources)` with
12+
zero tests run, so the lane called "Unit Tests" named a subsystem that never reached compilation.
4113

42-
**That prediction has now been tested, and "not reliably" is exactly the right strength - read it as
43-
written rather than as "never".** On the error-surfacing rung it fired on two consecutive runs: the
44-
second reproduced the identical failure at the same execution on the same artifact, and took the
45-
**Integration lane down with the Unit lane** on the same run. A third run then passed every lane. So:
46-
47-
- **The exposure is not "one lane occasionally"** - it is *every lane that builds this module*, and
48-
they fail together, because they share the runner's route to Central.
49-
- **It is per-run, not per-branch.** A run either has the problem or does not, so a re-run is a coin
50-
flip rather than a fix, and a green run is luck that says nothing about the next one.
51-
- **A green lane is therefore not evidence this is closed.** It is closed when the unpack executions
52-
stop reaching Central at all - see *Delete when* below.
53-
54-
**And it has nothing to do with the diff, which a control arm now settles rather than argues.** The
55-
run after the green one carried a **markdown-only commit** - no Java, no pom, no workflow - and
56-
reproduced the identical failure on both lanes. There is no reading of that in which the change under
57-
review is implicated. If you are looking at a red Unit lane on this module and wondering what you
58-
broke, this paragraph is the answer.
59-
60-
## Candidate fix
14+
<!-- post-merge: checked-begin -->
15+
**Fixed by astubbs/parallel-consumer#379**, which added the `Warm the Kafka sources jars the streams
16+
module unpacks` step to `prepare-deps` in `.github/workflows/maven.yml` - so wherever that step is
17+
present, this is closed. **That step's own comment is the durable owner** of why go-offline
18+
misses an artifactItem, why the warm names coordinates rather than building the module, and what the
19+
two guards are for; this note does not restate it.
20+
<!-- post-merge: checked-end -->
6121

62-
Add the classifier artifacts to what `prepare-deps` pulls, so the warm cache actually contains
63-
everything a build needs. `dependency:go-offline` will not do it on its own; the honest options are
64-
running the module's `generate-sources` phase in the warming job, or a `dependency:get` per artifact
65-
item. Whichever is chosen, the check that it worked is that the streams module's `unpack` executions
66-
log a cache hit rather than a `Downloading from central` line.
22+
## What the sightings established, for any branch still exposed
6723

68-
## Until it is fixed, what a blocked PR should do
24+
<!-- post-merge: checked-begin -->
25+
The defect was observed on two independent rungs of this stack -
26+
astubbs/parallel-consumer#394 and astubbs/parallel-consumer#395 - which is what made it master-state
27+
rather than one PR's problem. Three findings from those sightings outlive the fix, because they
28+
describe what an *unwarmed* branch is still living with:
29+
30+
- **It is every lane that builds this module, not one lane occasionally.** Unit and Integration went
31+
red together on the same run, because they share the runner's route to Central.
32+
- **It is per-run, not per-branch, so a re-run is a coin flip and not a fix.** The referenced
33+
write-up's "re-running does not reliably help" proved to be exactly the right strength: two
34+
consecutive runs reproduced the identical failure at the same execution on the same artifact, and a
35+
third then passed every lane. A green run is luck, and says nothing about the next one.
36+
- **It has nothing to do with the diff, and a control arm settles that rather than arguing it.** A
37+
**markdown-only commit** - no Java, no pom, no workflow - reproduced the failure on both lanes
38+
immediately after a green run. There is no reading of that in which the change under review is
39+
implicated.
40+
41+
If you are on an unwarmed branch looking at a red Unit lane on this module and wondering what you
42+
broke: the answer is the paragraph above, and the fix is to merge astubbs/parallel-consumer#379
43+
forward rather than to re-run.
44+
<!-- post-merge: checked-end -->
6945

70-
**Read the log before concluding anything.** The lane is named "Unit Tests" and no test ran, so the
71-
first job is to recognise it: the module dies at `unpack-kafka-streams-sources`, every other module
72-
succeeds, and the message names Central and the `sources` classifier.
46+
## What is still open
7347

74-
Then **say in the PR that the red lane is this, name this note, and put the module's own local lane
75-
up as the evidence the code is healthy** - the whole `test` phase and the integration lane, counts
76-
read out of the report directories. A local run has the artifacts in `~/.m2` already, which is why it
77-
never reproduces there, and why "green locally" is a real signal about the code and none at all about
78-
this.
48+
<!-- post-merge: checked-begin -->
49+
**Every branch carrying `parallel-consumer-streams` without that step still has the unwarmed
50+
workflow**, and stays exposed until it merges astubbs/parallel-consumer#379 forward. The candidates
51+
are
52+
`for r in $(git for-each-ref --format='%(refname:short)' refs/remotes/origin); do git cat-file -e "$r:parallel-consumer-streams/pom.xml" 2>/dev/null && echo "$r"; done`,
53+
minus those that already contain the step - `git grep -l 'Warm the Kafka sources jars' <ref> --
54+
.github/workflows/maven.yml`.
55+
56+
**A branch carrying the pre-fix copy of this file collides add/add on this path** when it merges
57+
astubbs/parallel-consumer#379 forward: **take the version that names the step**, not the one that
58+
calls the fix a candidate. The two rungs that hit it, `feats/ks-streams-task-lifecycle` and
59+
`feats/ks-streams-error-surfacing`, resolved it that way and are no longer exposed; the instruction
60+
stands for any rung above them that has not yet merged forward.
61+
<!-- post-merge: checked-end -->
7962

80-
**Re-running is legitimate here and is not a retry-into-green** - it is not a flaky test being
81-
papered over, it is an artifact download that either reached Central or did not, and no test outcome
82-
is being hidden. What it is not is a fix: it is a coin flip, it can take several attempts, and the
83-
green it eventually produces is not evidence the problem is gone. So re-run if you need the lane, and
84-
**do not let the green talk you out of leaving the record behind**.
63+
**`test-kafka-compat` is the one job whose Kafka version falls outside the warm.** It is `if: false`
64+
today; re-enabling it re-opens this for that lane only.
8565

8666
## Delete when
8767

88-
A streams-module CI run shows the unpack executions resolving from the warmed cache rather than
89-
downloading from Central.
68+
No open branch builds the streams module without that step - at which point nothing here is both
69+
true and unowned by the workflow comment.

docs/inflight/test-streamthreadtest-invalid-timestamps-flake.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ it costs every branch in the chain a red build and a re-run, and where the tempt
3636
green is strongest.
3737
<!-- post-merge: checked-end -->
3838

39+
**Sighted twice more on the example rung's CI** (astubbs/parallel-consumer#391): across four
40+
attempts of its Unit lane, the sequence was flake, Maven Central timeout, green, flake - two
41+
failures in the three attempts that actually ran the suite, at the full run count the oracle
42+
produces on this stack. Consistent with the forest's originally measured rate rather than below it.
43+
The example agent deliberately stopped re-running to chase green, which is the habit this note
44+
exists to prevent.
45+
3946
## Why this matters more than one flaky test
4047

4148
**Every agent working on this module is told that "Kafka's own suites, zero failures with the seam

0 commit comments

Comments
 (0)