bitflicker64 commented on PR #3132:
URL: https://github.com/apache/hugegraph/pull/3132#issuecomment-5611399757
## Campaign 2026-09-10: chart 0.1.6 defaults run against real images
The three image-side fixes this chart's defaults depend on are now on
`master`, so the
defaults committed in `2bff77a6` were run against real images rather than
reasoned about.
This is the runtime half of the checklist item whose docs half was done last
week. It also
turned up one defect in this PR's own `templates/`, at the bottom.
**What was under test.** Chart `0.1.6` from `hugegraph/hugegraph` tag
`helm-dev-20260910`
(`d8b6a0bf`). This PR's own `Chart.yaml` still declares `0.1.0`; what the
two share is the
template content and the three defaults, not the version string. Its
`templates/`, `values.schema.json` and `tests/` are byte-identical with
this PR at `2bff77a6` (the three tree hashes match), so every template-level
row applies
here too. `values-single.yaml` is byte-identical too, but `values.yaml` and
`values-cluster.yaml`
are **not**, and that matters for two rows below: the tested tree defaults
`hubble.enabled: true`, `pullPolicy: IfNotPresent` and the image tag
`helm-dev`, this PR
defaults them `false`, `Always` and `latest`. Anything that counts or scans
rendered
objects therefore differs between the two trees, and both numbers are given.
Two image sets were built from source with `docker buildx bake`, the
revision label stamped
in and read back from each running pod through its node, never from the
build host:
| Stage | Tag | Built from | Revision read back from the pods |
|---|---|---|---|
| **A** | `a0905` | tag `helm-dev-20260905` (`6ec19838`): master `36811483`
**plus #3185, #3187 and #3189 as pull-request branches** | `6ec1983889f58a...`
on all 9 pods |
| **B** | `b0910` | tag `helm-dev-20260910` (`d8b6a0bf`): **merged master
`60c8803d`** plus this chart | `d8b6a0bf256616...` on all 9 pods |
| both | `hubble:latest` | pulled, not built here; built by
hugegraph/actions from hugegraph-toolchain | `3b385c3d2f1c04...` |
Both sets therefore carry all three fixes, one as branches and one as merged
master, so
**this campaign is mostly a merge-regression check, not a
before-and-after**. There is one
exception, and it turned out to be the interesting row: #3185 gained a
further commit,
`a3b9395`, **after** the stage A tag was cut, so for the readiness handler's
own latency
stage A really is a pre-fix control. Everything else has no pre-fix control
in it. Every row
below says which stage it was measured on.
Host: a 4-node kind cluster (1 control-plane + 3 workers, Kubernetes
v1.37.0),
`values-cluster.yaml` plus a session file pinning the image tags and pull
policy, the
admin password, and the PD and Store PVC sizes at 2Gi and 5Gi rather than
the preset's 10Gi
and 50Gi, because the preset asks for 180Gi of PVCs across the six pods and
this host has
115 GB free. That last one is a deviation from the preset and the PVC
reattach row below
inherits it. **The 0.1.6 defaults were deliberately not
overridden**, which is the point: `pd.readinessPath`, `store.waitPath` and
`HG_SERVER_STARTUP_TIMEOUT_S` all came from the chart.
### The three new defaults
- **PD readiness on `/v1/ready`** (stage A install, both stages under
fault). All three PDs
were Ready when `helm install --wait` returned on stage A, with
`/v1/health` and
`/v1/ready` both answering 200 side by side. There was no stage B install:
stage B was a
`helm upgrade` of the same release, and its side-by-side 200s come from
the post-roll
check, not from an install. Under a 2-of-3 PD loss on **stage B** the
survivor answered
**503 from T+2 s to T+12 s** while `/v1/health` stayed 200 on **38 of 38**
samples, and
3/3 PD were Ready again about 5 s later.
- **The Store PD wait on `/v1/ready`** (stage A). The rendered Store
StatefulSet's init
container carries `WAIT_PATH="/v1/ready"`, and all three Stores left it,
so the majority
the gate counts is a quorum rather than a set of live listeners. (For the
record, the
Store's own three probes stay on its own `/v1/health`, and PD keeps
`/v1/health` on
startup and liveness so a PD that merely lost its leader is not restarted.
Only PD
readiness and the Store's PD wait moved.) The chart's own `NOTES.txt` still
tells the operator otherwise; see the last section.
- **A derived `HG_SERVER_STARTUP_TIMEOUT_S`** (stage B). The Pod spec
carries `"450"`,
exactly the startup probe budget (`failureThreshold: 90` x `periodSeconds:
5`). Caught
on a replacement Server three seconds after a delete, on the `b0910` image:
`/bin/bash ./bin/start-hugegraph.sh -j -Xms512m -Xmx1024m ...
-XX:+UseContainerSupport -t 450`.
A one-off Pod with an invalid value exits 1 with the image's range error
on **both**
image sets, so #3187's validation survived the merge.
### Results
| Row | Stage | Verdict | Evidence |
|---|---|---|---|
| static gates | chart only | PASS-static | lint on 3 presets; **63 unit
tests**; 15 of 15 fail-paths rejected, each with its recorded rejection text,
and 5 of 5 positive controls render (the exit code is the pass condition; their
stderr capture files are empty, so that row is thinner evidence than the
fail-paths); renders **19/17/20** on the tested tree and **16/14/16** on this
PR's own values, the difference being the Hubble objects the PR does not render
by default; kubeconform `-strict` clean on both trees at the 1.23.0 floor and
at 1.37.0 (0 invalid, 0 skipped); pluto clean on both; `ct lint` rc=0 on the
first attempt |
| policy scanners | chart only, tree-dependent | recorded, not clean | see
the section below: the headline numbers belong to the tested tree, not to this
PR, and this PR's own render was scanned separately |
| install + smoke | A | PASS-smoke | `helm install --wait` rc=0, 10/10
Ready, **0 restarts**, all ten pods at AGE 52s when it returned; `Could not
resolve allowlist entry` = 0 and `Blocked connection` = 0 on every PD; one
clean election; CRUD 202/201/201/200 |
| auth matrix | A and B | PASS | 401 / 401 / 200 on **every** Server Pod IP,
on both stages |
| `helm test` | B | PASS | `Phase: Succeeded` |
| Hubble | A and B | PASS | `operations/nodes` lists **3 PD, 3 STORE, 3
SERVER, all UP** with three distinct uptimes. Hubble itself was logged into
with the admin password the harness pinned, not with a PD credential; what the
row shows is that Hubble reached PD and enumerated the topology, and the only
PD credential in its rendered config is the one the chart wrote. That config
value was not captured to a file, so treat the chart-wrote-it half as derived |
| PD follower crash | A | PASS-hardening | 5 of 5 writes 202 during the
outage, rejoin 20 s, leadership unchanged |
| PD leader crash | A | PASS-hardening | new leader in **5 s**, pre-fault
write still readable |
| PD majority loss | A and B | PASS-hardening | see the `/v1/ready` numbers
above; stage A gave the same shape (503 at T+2, health 200 on 43 of 43) |
| Store crash + durability | A | PASS-hardening | mid-fault write 202, pre-
and mid-fault markers readable after rejoin |
| pod-IP churn | A | PASS | PD Pod IPs changed in two separate fault
scenarios and again at a full rolling replacement, with **0 blocked lines every
time** |
| PVC reattach | A | PASS-hardening | same `PersistentVolume` rebound, data
readable |
| rolling restart under load | A | PASS-hardening | 54 attempted, 52 acked,
**0 acked writes lost**, 3 of 3 uids per component |
| discovery lease, 4 claims | A | PASS-hardening | 3 rows = 3 Pod IPs at
`interval 15000`; new IP at T+10 s, old row gone by T+46 s, peak 4 rows;
`advertiseUrl` collapses to 1 row and the revert restores 3 |
| PD credential matrix | A and B | PASS | 401 **with** `WWW-Authenticate`
for a missing credential, an empty password, a wrong password, and for the two
unknown names tried (`admin`, `nobody`); 200 only for `hg`, `store`, `hubble`,
`vermeer` with the release secret; `/v1/health`, `/v1/ready` and the actuator
stay open. **Identical on both stages** |
| secret rotation | A | PASS-hardening | `checksum/auth` changed on both
upgrades and Server rolled both times; admin continuity unchanged by design |
| **image upgrade A to B** | A to B | PASS-hardening | PD 3/3, Store 3/3,
Server 3/3 rolled, **Hubble 0 of 1** because its image did not change;
`checksum/pd-auth` **unchanged**, so the roll is attributable to the image;
data written before the upgrade still readable (200 on three consecutive
reads); 12 of 12 writes through the Service afterwards |
| PD leader partition | A | **INCONCLUSIVE-confounded** | see below |
### The two policy scanners, and why their numbers are not this PR's
I ran both trees' cluster renders through the same two scanners, same
invocation, so the
comparison is like for like.
| Scanner | Tested tree (`helm-dev-20260910`) | This PR (`2bff77a6`) |
|---|---|---|
| kube-score, `--output-format ci` | **43 CRITICAL, 2 WARNING** | **36
CRITICAL, 0 WARNING** |
| polaris | **score 84**, 20 controllers | **score 83**, 16 controllers |
The gap is entirely the Hubble Deployment, which this PR's default values do
not render:
nine findings live on it, 7 CRITICAL and both WARNINGs, and 43 minus 7 is
exactly 36. The
three `ImagePullPolicy is not set to Always` findings on PD, Store and
Server disappear on
this PR, which defaults `pullPolicy: Always`, but three `Image with latest
tag` findings
appear in their place, because this PR leaves the image tag at `latest`
where the tested
tree pins it. Net zero on those, so **on the workloads this PR actually
ships, the two trees
score the same**. polaris's one point is worth a closer look, because it is
not simply the
smaller object count. The two failure profiles differ in severity, not only
in volume: the
tested tree has **33 warning-level and 6 danger-level** failures across its
20 controllers,
this PR **24 and 7** across 16. Fewer objects and *more* danger-level
findings. The cause is
that same swap: `pullPolicyNotAlways` is a warning and is satisfied on this
PR, while
`tagNotSpecified` is a danger and fires on PD, Store and Server here where
the tested tree
pins the tag. So the tag pin this PR already promises for 1.8.x removes
three danger-level
findings, and that is the more useful reading of the two scores. I have not
tried to
reproduce polaris's exact weighting, so take the two published scores as
given and the
severity counts, which come straight out of the two JSON reports, as the
explanation.
One bookkeeping note, since I quoted a different figure in an earlier draft
of this comment:
kube-score's default output groups findings by check per object and gives 27
CRITICAL on the
tested tree, where `ci` lists each finding and gives 43. Same run of the
same tool on the
same file, different granularity. Every number in this section is `ci`.
The classes themselves are the familiar ones: NetworkPolicy, root user,
ephemeral storage,
`readOnlyRootFilesystem`, pull policy and image tag, all of which map to an
existing
documented decision. Two do not fit that list and should be said out loud:
- **`Container has the same readiness and liveness probe`, 2 findings, on
this PR's render
as well as the tested tree.** This one **shrank** because of this change:
it fires on
Server and Store only, not on PD, because PD readiness left `/v1/health`
and its two
probes are no longer the same signal. That is a scanner independently
confirming the
second half of the open review-thread objection.
- **`No matching PodDisruptionBudget was found`, 1 finding, on the Server
Deployment, on
this PR's render as well.** The chart ships PDBs for PD and Store but not
for Server. I
have no documented decision to point at for that, so I am flagging it
rather than
explaining it away.
### The partition scenario, reported as inconclusive
Two attempts, neither of which supports a verdict, both recorded rather than
dropped:
- A node-level `iptables FORWARD` cut of the PD leader landed (735 packets
in, 132 out) and
raft did the right thing, moving leadership to a survivor. But **all ten
write samples
timed out at 15 s for the duration**. That is not a chart result: kubelet
probes originate
in the node netns and bypass FORWARD, so the partitioned PD stayed Ready,
stayed a
`pd-client` endpoint and stayed in every Server's static peer list. The
ambiguity was
declared before the fault.
- A cut inside the Pod's own network namespace removed that ambiguity, and
the PD did go
`podReady=false`, which is the readiness signal doing its job and is not
what a
`/v1/health` readiness would have done. Everything else about that run is
confounded: the
teardown read the container PID rather than the Pod **sandbox** PID, which
survives
container restarts, so the fault stayed in place far longer than intended,
and the host
was measurably short of memory at the time. The cluster recovered on its
own the moment
the harness rules were removed. Whether the EndpointSlice actually dropped
the address is
**not** claimed: the column meant to show it listed addresses regardless
of their ready
condition, so it measured nothing.
A clean single-node leader partition with a write oracle is therefore
**NOT-RUN**.
### Comparison with 2026-09-05
**No regressions.** The merged forms of #3185, #3187 and #3189 behave the
same as the
pull-request forms measured on 2026-09-05, with one exception in the merge's
favour:
#3185's merged form answers `/v1/ready` during an election three orders of
magnitude
faster, for the reason set out in the next section. The one-off Server
first-boot exit 1 seen then
did not reproduce: 0 restarts across all ten pods. Unit tests went 58 to 63
with the 0.1.6
guards. `ct lint` now passes on the first attempt, because the missing
`yamale` binary that
failed it on 2026-09-01 is installed on this host.
Install looks faster, and I want to be careful about that one, because the
two figures are
not the same measurement on the same machine. 2026-09-05 recorded an install
**duration** of
2 min 47 s on soul, a 24-core, 59 GB host (that campaign's `state.md`
records the specs).
Today's number is a pod **AGE** of 52 s on library, 12 cores and 15 GB by
`nproc` and
`free`, with the closest comparable figure being the 53 s span of the
install script
itself. Faster, yes, but do not read a 3x from it.
`hugegraph/hubble:latest` **now carries an
`org.opencontainers.image.revision` label**
(`3b385c3d`). Earlier campaigns recorded it as unlabelled and therefore not
attributable to
a source revision, so the whole topology is attributable at last.
### The readiness handler latency, and a correction to how I first read it
On 2026-09-05 I reported on apache/hugegraph#3185 that the `/v1/ready`
handler took 9.8 s to
answer its 503 during an election. It reproduced at **9.451460 s on the
stage A images** and
**did not reproduce on merged master**: across **eleven consecutive samples
inside the
leaderless window**, T+2 to T+12, the slowest answer was **3.055
milliseconds**.
I first wrote that up as the stall being intermittent. It is not. It is
fixed, and the fix
is in the tree under test. `a3b9395`, "serve readiness from raft callbacks,
not the node
lock", was pushed to #3185 on 2026-09-05 in response to that report, and it
reached master
in the squash `e85b36a4` on 2026-09-08. The stage A tag `helm-dev-20260905`
was cut before
it: `ProbeView` is absent from `6ec19838` and present in `60c8803d` and
`d8b6a0bf`. So the
two numbers are a genuine before and after of one commit, not two samples of
one flaky
path, and the comment that commit carries states the mechanism outright,
that jraft holds
the node lock while it reconnects to peers during an election and a reader
stalls for the
connect timeout.
That makes stage A a pre-fix control for this one row and nothing else, and
it is the only
before-and-after in the campaign.
A second correction, to my own method rather than to the code: the first
attempt at that
scenario sampled the prometheus gauge and both probes in one loop iteration.
The gauge call
stalled during the election, the loop jumped from T+5 to T+28, and the whole
leaderless
window fell into the gap, so the run would have been written up as "the
endpoint never
refused". Sampling `/v1/ready` alone at 1 s exposed it immediately. As a
consequence,
`hg_raft_has_leader` reaching 0 is **carried from 2026-09-05, not measured
today**; today's
gauge sampler was too coarse to catch a 12-second window and that cell is
inconclusive.
### Not run, stated as gaps
- **Upgrade from a released chart version: NOT-RUN.** There is still no
1.8.x release or
image tag to pin to or upgrade from, which remains the external gate on
this PR.
- **kubescape: NOT-RUN**, the binary is not on the test host.
- **A clean single-node PD leader partition with a write oracle: NOT-RUN**,
per the section
above.
- **EndpointSlice ready-condition membership under a partition: NOT-RUN.**
- Store majority loss, and the cross-namespace and node-freeze scenarios,
were not
attempted.
Two limits on what the S11 chain proves: no Server start slower than 120 s
occurred, so the
last link, the start script honouring `-t`, is still carried from #3187's
own tests rather
than observed here; and the claim that a pre-#3185 image would never turn
Ready on
`/v1/ready` is derived, not shown, because no such image exists in this
campaign.
One operational note for the docs rather than the code: an image-tag upgrade
replaces all
nine PD, Store and Server pods, and for a few minutes afterwards writes
through the Server
Service can stall even though every pod reports Ready. It cleared with no
intervention here,
and writes through both the Service and direct to pods succeeded afterwards,
but the exact
mechanism was not instrumented, only excluded: host load and memory were
fine, every PD and
Store was reachable and Up, and direct-to-pod writes were already succeeding
while
Service-routed ones were still timing out.
### Three lines to fix in this PR
Chart 0.1.6 moved the two defaults and updated most of the prose around
them, but three
places still describe the old default, and all three ship here exactly as
written.
`templates/NOTES.txt` is byte-identical between the tested tree and
`2bff77a6`. The two
READMEs are not identical overall, but the two parameter-table rows below
are word for word
the same on both.
**1. `templates/NOTES.txt`, lines 31 and 32.** Printed to the operator on
every install and
every upgrade. It appears verbatim in this campaign's install log, upgrade
log and
`helm test` output:
```
Store Pods stay in Init until a majority of PD peers answer store.waitPath
(/v1/health by default, which proves the listener is up, not a raft quorum).
```
**2 and 3. The two `README.md` parameter-table rows**, where the Default
column still reads
`/v1/health` and the description still tells the reader to set `/v1/ready`
themselves:
| Parameter | Description as shipped | Default column |
|---|---|---|
| `pd.readinessPath` | "`/v1/health` is liveness only; set `/v1/ready` on PD
images from 1.8.0 that carry it, never on older images" | `/v1/health` |
| `store.waitPath` | "`/v1/health` counts listeners; `/v1/ready` counts
quorum members but exists only on PD images from 1.8.0" | `/v1/health` |
Both now say the opposite of `values.yaml`, of this README's own bullet at
line 48 ("The
default `/v1/ready` stays 503 until a raft leader exists"), of the
Limitations section at
line 935 ("The chart defaults `pd.readinessPath` and `store.waitPath` to
`/v1/ready`"), and
of `values.schema.json`, which already describes `/v1/ready` as the default
in both places.
The rows themselves are at lines 306 and 336.
The parameter table is the reference an operator actually consults before
overriding
anything, and `NOTES.txt` is the only thing they are shown without asking.
Between them
they tell a reader that the quorum-aware gate is opt-in, which is the exact
point the open
review thread on `store-statefulset.yaml` is about, and it is no longer
true. It is three
lines, and I would rather flag them than push a fix into a PR under review
without asking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
