docs(dha): PoC finding — CNPG scale subresource has no selector; reopen actuation mechanism - #53
Conversation
…en mechanism A live PoC validated the metric encoding (Σ+target drives the right count on real data) but found the load-bearing mechanism — a stock HPA via KEDA driving the CNPG Cluster scale subresource — is not buildable on stock CNPG: the Cluster /scale exposes no status.selector (no labelSelectorPath), so the HPA controller fails with InvalidSelector for every metric type. Upstream CNPG #7923 (add the selector) is closed as not planned; not fixable by a version bump. Record the finding and two resolution options — (A) KEDA + a thin mirror shim behind a proxy object with a selector, or (B) a lean actuation controller (no KEDA) writing Cluster.spec.instances directly. Note that neither returns rev1's ownership machinery, which §3 already removed. This reopens the actuation mechanism decision. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughThe proposal records a PoC finding that prevents stock KEDA/HPA actuation for CloudNativePG. It reopens the mechanism decision and presents a mirror-shim approach and a direct controller approach. ChangesDatabase autoscaling proposal
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
design-proposals/database-horizontal-autoscaling/README.md (2)
6-7: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDo not present the superseded HPA path as the active design.
The status says that the mechanism is reopened, but the overview still describes direct KEDA/HPA-to-CNPG actuation and no new CRD. Option A adds a mirror shim and may add a CRD. Mark the later sections as the superseded pre-PoC design, or update the active overview and diagram after the mechanism decision.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/database-horizontal-autoscaling/README.md` around lines 6 - 7, Update the overview and architecture diagram in the database autoscaling proposal so they no longer present direct KEDA/HPA-to-CNPG actuation without a CRD as the active design. Either label the existing later sections as the superseded pre-PoC design, or revise them to reflect the selected post-PoC mechanism, including Option A’s mirror shim and possible CRD.
33-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftDefine one concrete proxy contract for Option A.
A CRD proxy and a placeholder workload are not interchangeable. Specify the proxy type, scale fields, selector/status behavior, source of the desired count, observed CNPG count feedback, and retry/idempotency rules. A placeholder workload also creates real pods. The option is not testable until one proxy type and its ownership contract are selected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/database-horizontal-autoscaling/README.md` at line 33, Define a single concrete proxy contract for Option A instead of presenting a CRD proxy and placeholder workload as alternatives. Specify the selected proxy type, ownership, scale fields, selector and status behavior, how the desired count is sourced, how observed CNPG instance counts feed back, and the retry/idempotency rules; if selecting a placeholder workload, document its pod implications. Update the Option A description so the design is concrete and testable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/database-horizontal-autoscaling/README.md`:
- Line 34: Expand the Option B description to define fail-safe handling for
missing, NaN, stale, and timed-out VictoriaMetrics results: never interpret
invalid metrics as zero, specify freshness and query-timeout thresholds, and
state whether the controller retains the last known good value or performs no
scaling during outages. Also document how normal scaling resumes after valid
fresh metrics return, alongside the existing bounds and stabilization behavior.
- Around line 27-29: The blocker in the README incorrectly treats the missing
selector as version-independent. Update the discussion around the HPA/CNPG
scale-subresource limitation to reflect that CloudNativePG 1.30.0 provides
Cluster.status.selector and permits HPA for custom or custom-read-replica
metrics; either scope the claims to the CloudNativePG version range supported by
cozystack v45 or validate them with a PoC on 1.30.0+. Remove or qualify
unsupported statements about version bumps and the 1.29.2 selector fix while
preserving the documented limitations of the supported version.
---
Nitpick comments:
In `@design-proposals/database-horizontal-autoscaling/README.md`:
- Around line 6-7: Update the overview and architecture diagram in the database
autoscaling proposal so they no longer present direct KEDA/HPA-to-CNPG actuation
without a CRD as the active design. Either label the existing later sections as
the superseded pre-PoC design, or revise them to reflect the selected post-PoC
mechanism, including Option A’s mirror shim and possible CRD.
- Line 33: Define a single concrete proxy contract for Option A instead of
presenting a CRD proxy and placeholder workload as alternatives. Specify the
selected proxy type, ownership, scale fields, selector and status behavior, how
the desired count is sourced, how observed CNPG instance counts feed back, and
the retry/idempotency rules; if selecting a placeholder workload, document its
pod implications. Update the Option A description so the design is concrete and
testable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 38bf1cfc-8bf1-4d8c-9093-8c523c1e079c
📒 Files selected for processing (1)
design-proposals/database-horizontal-autoscaling/README.md
| **The blocker.** That HPA never scales: it reports `ScalingActive=False, reason=InvalidSelector` — *"the HPA target's scale is missing a selector"*. The CNPG `Cluster` `/scale` subresource returns only `status: {replicas: N}` — no `status.selector` — and the CRD declares no `labelSelectorPath`. The Kubernetes HPA controller requires `scale.status.selector` unconditionally, before any metric-type branching, so this fails for **every** target type (confirmed with both `AverageValue` and `Value`). It is not a calibration detail and not fixable by a version bump: upstream CNPG issue [#7923](https://github.com/cloudnative-pg/cloudnative-pg/issues/7923), which requested exactly this selector for HPA/KEDA, is **closed as not planned**, and the CNPG 1.30 docs explicitly recommend against HPA for a `Cluster`. | ||
|
|
||
| **Consequence.** The load-bearing mechanism of this revision — a stock HPA (via KEDA) driving the CNPG `Cluster` scale subresource — cannot be built on stock CNPG; an actuation bridge is required after all. Crucially, what returns is **not** the machinery that got rev1 rejected: the ownership/enforcement layer (SSA, marker annotation, HelmRelease webhook, terminal-freeze) existed only because the chart declared `replicas`, and §3 (the chart omitting the field under autoscaling) removes it regardless of mechanism. What returns is only the small write-the-count actuator. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant proposal files =="
git ls-files | grep -E 'design-proposals/database-horizontal-autoscaling|database-horizontal-autoscaling|Cozystack|cozystack|v45|45' || true
echo
echo "== proposal excerpt =="
if [ -f design-proposals/database-horizontal-autoscaling/README.md ]; then
sed -n '1,120p' design-proposals/database-horizontal-autoscaling/README.md | nl -ba
fi
echo
echo "== search for CNPG versions/support matrix mentions =="
rg -n "CNPG|CloudNativePG|cloudnative-pg|1\.2[7-9]|1\.30|Cozystack|v45|compat|supported version|version matrix|versions?|version-bump|HPA|HelmRelease|replicas|status\.selector|labelSelectorPath" . \
-g '!node_modules' -g '!dist' -g '!build' -g '!vendor' \
| head -n 300Repository: cozystack/community
Length of output: 318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== proposal excerpt =="
sed -n '1,140p' design-proposals/database-horizontal-autoscaling/README.md
echo
echo "== version/matrix mentions =="
grep -RInE "CNPG|CloudNativePG|cloudnative-pg|1\.2[7-9]|1\.30|Cozystack|v45|compat|supported version|version matrix|versions|version-bump|HPA|HelmRelease|replicas|status\.selector|labelSelectorPath" . \
--exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build --exclude-dir=vendor | head -n 400
echo
echo "== release note/docs snippets in repository files =="
grep -RInE "status\.selector|labelSelectorPath|HPA|recommended against|not planned|7923" . \
--exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build --exclude-dir=vendor | head -n 200Repository: cozystack/community
Length of output: 50378
🌐 Web query:
cloudnative-pg 1.29 release notes status.selector HPA Cluster
💡 Result:
The status.selector field for the Cluster scale subresource, which enables integration with the Horizontal Pod Autoscaler (HPA) and Vertical Pod Autoscaler (VPA), was introduced in CloudNativePG 1.30.0, not 1.29.0 [1][2][3]. While CloudNativePG 1.29 included significant features such as dynamic network access control via podSelectorRefs and support for PostgreSQL extensions in image catalogs [4][5], the specific capability to expose a label selector via status.selector to allow HPA and VPA to map a Cluster to its instance pods was implemented in the subsequent 1.30.0 release (Pull Request #8996) [1][6][2].
Citations:
- 1: https://github.com/cloudnative-pg/cloudnative-pg/releases/tag/v1.30.0
- 2: https://github.com/cloudnative-pg/cloudnative-pg/blob/af12d5da/docs/src/release_notes/v1.30.md
- 3: https://newreleases.io/project/github/cloudnative-pg/cloudnative-pg/release/v1.30.0
- 4: https://github.com/cloudnative-pg/cloudnative-pg/releases/tag/v1.29.0
- 5: https://cloudnative-pg.io/releases/cloudnative-pg-1-29.0-released/
- 6: feat: add label selector to the Cluster scale subresource cloudnative-pg/cloudnative-pg#8996
🌐 Web query:
cloudnative-pg 1.30 HorizontalPodAutoscaler recommended against Cluster
💡 Result:
In CloudNativePG 1.30, the Cluster resource was updated to include a status.selector on its scale subresource, enabling it to be targeted by a Horizontal Pod Autoscaler (HPA) [1][2][3]. However, the official CloudNativePG documentation explicitly states that using an HPA to drive the number of instances in a Cluster is not recommended for PostgreSQL [4]. The recommendation against using HPA for a CNPG Cluster is based on the following technical reasons [4]: 1. Scaling Limitations: Increasing the number of instances in a Cluster only adds or removes standby replicas; it does not alleviate write load on the primary instance [4]. 2. Meaningless Metrics: The selector exposed by the scale subresource covers both the primary and all replicas [4]. Because these pods have fundamentally different workload profiles (write-heavy primary vs. read-only replicas), the per-pod average of CPU or memory metrics used by HPA is not a meaningful signal for scaling [4]. Adding replicas simply dilutes the average without addressing a potential bottleneck on the primary [4]. 3. Reconciliation Conflicts: HPA is generally unaware of CNPG’s internal constraints, such as synchronous replication requirements [4]. If HPA attempts to scale the cluster below the floor required by synchronous replicas (e.g., maxSyncReplicas + 1), the operator's validating webhook will reject the update, causing the HPA to continuously retry an unfulfillable state [4]. If you choose to use HPA despite these recommendations, the documentation advises that you must base the autoscaling on a custom metric that specifically reflects read-replica load and ensure that your minimum replica count (minReplicas) is set safely above the floor required by your synchronous replication configuration [4].
Citations:
- 1: https://cloudnative-pg.io/releases/cloudnative-pg-1-30.0-released/
- 2: https://github.com/cloudnative-pg/cloudnative-pg/blob/af12d5da/docs/src/release_notes/v1.30.md
- 3: feat: add label selector to the Cluster scale subresource cloudnative-pg/cloudnative-pg#8996
- 4: https://cloudnative-pg.io/docs/1.30/resource_management/
Correct the CNPG version assumptions in this blocker.
Cluster.status.selector is available in CloudNativePG 1.30.0, and CloudNativePG 1.30 documents HPA as not recommended but permitted only for custom/custom-read-replica metrics. Update the blocker to narrow the version-independent text to cozystack v45’s supported CloudNativePG range, or rerun the PoC against 1.30.0+; otherwise the “not fixable by a version bump” and “1.29.2 selector fix” claims are incorrect or unsupported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-proposals/database-horizontal-autoscaling/README.md` around lines 27 -
29, The blocker in the README incorrectly treats the missing selector as
version-independent. Update the discussion around the HPA/CNPG scale-subresource
limitation to reflect that CloudNativePG 1.30.0 provides Cluster.status.selector
and permits HPA for custom or custom-read-replica metrics; either scope the
claims to the CloudNativePG version range supported by cozystack v45 or validate
them with a PoC on 1.30.0+. Remove or qualify unsupported statements about
version bumps and the 1.29.2 selector fix while preserving the documented
limitations of the supported version.
| **Two options to resolve (decision needed).** | ||
|
|
||
| - **Option A — KEDA + a thin mirror shim.** Keep KEDA's hardened decision loop by pointing its HPA at a proxy object that *does* expose a selector (a small owned CRD, or a placeholder workload), and add a tiny controller that mirrors the proxy's computed count into `Cluster.spec.instances`. Preserves the stock decision loop, but adds a shim, a proxy object, and the platform-wide KEDA dependency for a value KEDA cannot deliver end-to-end on its own. | ||
| - **Option B — a lean actuation controller, no KEDA.** A small controller reads the read-load metric from VictoriaMetrics and writes `Cluster.spec.instances` directly, applying the `min`/`max`/quorum-floor bounds and stabilization. This is close to rev1 **minus the ownership machinery** (which §3 already eliminates) and minus the aggregated-API enforcement — a much smaller component than the rejected operator, with no KEDA platform dependency, at the cost of a modest amount of stabilization logic KEDA would otherwise provide. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Specify fail-safe metric handling for Option B.
If VictoriaMetrics returns no sample, NaN, stale data, or a timeout, the controller must not treat the result as zero. That behavior could scale CNPG to the quorum floor during a monitoring outage. Define freshness, timeout, last-known-good or no-scale behavior, and recovery behavior alongside bounds and stabilization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-proposals/database-horizontal-autoscaling/README.md` at line 34,
Expand the Option B description to define fail-safe handling for missing, NaN,
stale, and timed-out VictoriaMetrics results: never interpret invalid metrics as
zero, specify freshness and query-timeout thresholds, and state whether the
controller retains the last known good value or performs no scaling during
outages. Also document how normal scaling resumes after valid fresh metrics
return, alongside the existing bounds and stabilization behavior.
Timofei Larkin (lllamnyp)
left a comment
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
Request changes: the finding is real, but its premise is wrong — upstream already shipped the fix
The PoC observation stands: on the cluster it ran against, the KEDA-managed HPA fails with ScalingActive=False, reason=InvalidSelector, because that CNPG's Cluster scale subresource serves no status.selector. But the two claims the decision rests on — "upstream declined to add it" and "not fixable by a version bump" — are both incorrect, and with them the A/B decision this PR requests evaporates.
What upstream actually did. cloudnative-pg#7923 was not declined: it was closed by the stale-bot after 60 days of inactivity, and the only maintainer comment questioned the use case while explicitly excepting "users [who] rely heavily on PostgreSQL for read scalability" — which is this design's entire scope. Meanwhile the parallel request cloudnative-pg#2574 ("Support the scale sub-resource for Clusters") was closed completed on 2026-06-04 by cloudnative-pg#8996, "feat: add label selector to the Cluster scale subresource": it adds ClusterStatus.Selector, populated by the operator on every status update, and wires subresources.scale.labelSelectorPath: .status.selector — I verified the labelSelectorPath is present in the released CRD at tag v1.28.4. The PR's own doc text names HPA explicitly ("so an autoscaler (such as HPA or VPA) can discover the managed instance pods"). It was backported to the 1.28 and 1.29 release branches and shipped on 2026-06-29 in v1.28.4, v1.29.2, and v1.30.0.
Why the PoC hit the wall anyway. The fix was never backported to 1.27, and the PoC ran CNPG 1.27.3 (cozystack currently vendors the cloudnative-pg chart at 0.26.1 / appVersion 1.27.1). The blocker is version-bound, full stop: on any CNPG ≥ 1.28.4 the approved mechanism — chart conditional + KEDA ScaledObject + the Σ + target query — should work byte-for-byte on stock CNPG, with a real operator-maintained selector.
Consequently, neither option should be chosen. Option A adds a CRD and a mirror controller to work around a gap that no longer exists; Option B re-acquires the hand-rolled decision loop this design spent three revisions deleting. A CNPG version bump is routine vendored-chart maintenance and needs no design change beyond one line.
The ask — rework this amendment from an open decision into a closed record:
- Keep the PoC finding, scoped correctly: metrics half validated; actuation blocked on CNPG ≤ 1.27, whose scale subresource lacks a selector.
- Replace "upstream declined" with what happened: #7923 went stale as an unnoticed duplicate; #2574 was accepted and implemented by #8996.
- Record the resolution: options A and B rejected — unnecessary once the selector ships; the fix is a CNPG bump (a separate cozystack PR, ≥ 1.28.4 / 1.29.2 / 1.30.0) followed by a re-run of the same PoC as the verification gate.
- Add the version floor to the approved design as a hard precondition (e.g. in §Scope or §4: "requires CNPG ≥ 1.28.4 — the scale subresource must serve
status.selector, cloudnative-pg#8996"). - Restore the Status line — the mechanism is not reopened, so the "mechanism reopened" stamp and the "decision needed" framing go.
Merged in that shape, this PR becomes exactly the record it should be: the next person who hits InvalidSelector on an old CNPG finds the version floor and the rejected alternatives in-tree instead of re-proposing a shim.
One piece of hygiene while we're here: a one-line comment on cloudnative-pg#7923 pointing at #8996 would defuse the trap for the next searcher — that stale duplicate is precisely what sent this PR (and my own first pass at reviewing it) down the wrong path.
f67b266 to
de59928
Compare
…ersion floor Address @lllamnyp's review: the finding is real but version-bound, not a mechanism blocker. Correct the upstream story (cloudnative-pg#7923 went stale as a duplicate, not declined; #2574 was implemented by #8996, which adds Cluster.status.selector + labelSelectorPath and shipped in CNPG 1.28.4 / 1.29.2 / 1.30.0, not backported to 1.27). Record options A and B as rejected — unnecessary once the selector ships; the fix is a routine CNPG vendored-chart bump (1.28.2 -> >= 1.28.4) plus a PoC re-run as the gate. Add the CNPG >= 1.28.4 precondition to Scope, restore the Status line (mechanism not reopened), and carry the fail-safe-metric note into implementation. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
Reworked into a closed record per your review — thank you for tracing the actual upstream history; that corrected the premise.
Will post the hygiene one-liner on cloudnative-pg#7923 pointing at #8996. |
The Why-this-changed paragraph still said the PoC hit something Kubernetes cannot provide; the reworked record says the opposite (CNPG >= 1.28.4 provides it). Align the sentence with the record. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Timofei Larkin (lllamnyp)
left a comment
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
Thanks for the rework — this is now exactly the record it should be: premise corrected (#2574/#8996, version-bound blocker), options A and B explicitly rejected, the CNPG ≥ 1.28.4 floor stated as a precondition, and the Status line restored. I pushed one wording fix on top (f491243): the "Why this changed" paragraph still ended with the pre-rework claim that Kubernetes cannot provide the selector, which the record it introduces disproves.
One implementation note to carry forward from the new fail-safe paragraph: with KEDA's prometheus trigger, ignoreNullValues defaults to true, so an empty query result surfaces as metric value 0 — exactly the read-as-zero-and-scale-to-the-floor hazard the paragraph forbids. The rendered ScaledObject must set ignoreNullValues: "false" so an empty result is an error and the HPA holds. Nothing to change in this document; it belongs in the implementation and its chart tests.
Approving and merging.
|
scooby87 #56 adds decision records to this repo, one page on a decision already made and why the alternatives lost, filed under the proposal it amends. rev1's bespoke db-autoscaler died because the spike proved enforced ownership unbuildable on the aggregated apps API. Settled and merged in #44, but the reasoning is in the proposal appendix, which the next in-place revision can rewrite. Can you write it as design-proposals/database-horizontal-autoscaling/decisions/0001-*.md? Template is design-proposals/decision-template.md in #56. Your CNPG selector finding here is a good Revisit if for it. |
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
3 participants
What this PR does
Amends the merged Database Horizontal Autoscaler proposal with a blocking finding from a live PoC, and puts two resolution options on the record for a decision. This follows up the design merged in #44.
The finding
A live PoC (cozystack v45, CloudNativePG 1.27.3, Kubernetes 1.34.3) validated the metric side of the design but found that the load-bearing mechanism — a stock HPA (via KEDA) driving the CNPG
Clusterscale subresource — cannot be built on stock CNPG.Σ + targetquery returns the right number on real data (151at target150→desired = ceil(151/150) = 2 = 1 primary + 1 read replica); the series/labels exist in VictoriaMetrics; the KEDA package installs and renders theScaledObjectinto a managed HPA.ScalingActive=False, reason=InvalidSelector. The CNPGCluster/scalesubresource exposes nostatus.selector(the CRD has nolabelSelectorPath), and the HPA controller requires the selector unconditionally — so it fails for every metric type (verified with bothAverageValueandValue). Upstream CNPG #7923 (add that selector for HPA/KEDA) is closed as not planned, and the CNPG 1.30 docs recommend against HPA for aCluster— so this is not fixable by a version bump.Why this is not a return to rev1
The machinery that got the first revision rejected — SSA ownership, the marker annotation, the HelmRelease webhook, terminal-freeze — existed only because the chart declared
replicas. §3 (the chart omitting the field under autoscaling) removes that regardless of mechanism. What the finding brings back is only a small write-the-count actuator, not the enforcement layer.Decision requested (Timofei Larkin (@lllamnyp))
Cluster.spec.instances. Adds a shim + proxy object + the platform-wide KEDA dependency.Cluster.spec.instancesdirectly with the min/max/quorum bounds and stabilization. Close to rev1 minus the ownership machinery (§3 already removes it), no KEDA dependency, at the cost of some stabilization logic.Both keep §1 (validated metric encoding) and §3 (chart omits the field). The choice is where the desired-count computation lives and whether to take KEDA as a platform dependency. Your Alternative 4 (a thin actuation shim) turns out to be needed for PostgreSQL too — not because it lacks a scale subresource, but because that subresource has no selector.
The implementation branch for the code (cozystack/cozystack#3381) is held — slices 1–3 (postgres ScaledObject + cozy-lib helper, KEDA package + wiring, alerts/dashboard) and the two-phase migration are done and green, but the actuation path is frozen pending this decision.
release-note
Summary by CodeRabbit