Design proposal: Database Horizontal Autoscaler#32
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
📝 WalkthroughWalkthroughThis PR adds a new design proposal document describing a "Database Horizontal Autoscaler" (DHA) for Cozystack. It defines a ChangesDesign Proposal Document
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal for a Database Horizontal Autoscaler (db-autoscaler) in Cozystack to automatically scale read replicas of managed databases based on load. The review feedback highlights several areas in the design that require clarification or adjustment: detailing the mechanism and RBAC permissions for draining read connections, clarifying whether replica limits refer to total instances or strictly read replicas, addressing false-positive replication lag freezes during write-idle periods, and adding the necessary RBAC permissions to read tenant quotas and resource presets.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| 4. Query VictoriaMetrics for the driver metric and the replication lag. | ||
| 5. Compute `desired = ceil(currentReplicas * currentMetric / targetMetric)` with a tolerance dead-zone. | ||
| 6. Apply guardrails (see below): clamp to `[min,max]`, quorum floor, lag brake, stabilization windows, step limit, tenant quota. | ||
| 7. If `desired != current` and the decision passes → PATCH `spec.replicas` on the `Application` (with `RetryOnConflict`); on scale-down, drain read connections first. |
There was a problem hiding this comment.
The proposal mentions that on scale-down, the autoscaler will "drain read connections first". However, the exact mechanism for this is not described. Since the db-autoscaler only has read/patch permissions on applications.apps.cozystack.io and read-only permissions on pods (with no write/patch access to Pods, Services, or Endpoints), it is unclear how it can actively drain connections or de-register a pod from the read-only service before patching the replica count.
If the draining relies entirely on the underlying database operator's native graceful shutdown handling, the DHA does not need to perform any action "first". If the DHA is expected to actively manage connection draining (e.g., by labeling pods to exclude them from the service), the RBAC permissions must be updated to include write/patch access to Pods or Services, and the design should clarify how this avoids fighting the database operator's reconciliation.
| metadata: { name: db, namespace: tenant-acme } | ||
| spec: | ||
| targetRef: { kind: Postgres, name: db } # apiGroup defaults to apps.cozystack.io | ||
| minReplicas: 2 |
There was a problem hiding this comment.
There is a potential ambiguity between "total instances" and "read replicas". In database operators like CloudNativePG, the instances (or replicas in Helm values) controls the total number of instances (primary + replicas). If the DHA's minReplicas and maxReplicas map directly to this value, they represent total instances. However, the proposal describes the autoscaler as scaling "read replicas".
Please clarify in the spec description whether minReplicas and maxReplicas refer to the total instance count (including the primary) or strictly the number of read-only standby replicas. If it represents total instances, a warning or validation should prevent users from setting minReplicas to 1, which would leave them with only a primary and 0 read replicas.
| scaleDown: { stabilizationWindowSeconds: 1800, step: 1 } | ||
| constraints: | ||
| respectQuorum: true | ||
| maxReplicationLagSeconds: 30 |
There was a problem hiding this comment.
Using a time-based replication lag threshold (e.g., maxReplicationLagSeconds: 30) is highly susceptible to false positives during periods of write idle. In PostgreSQL, standard lag metrics like now() - pg_last_xact_replay_timestamp() will continuously increase when there are no new transactions on the primary, which would falsely trigger the lag brake and freeze scaling.
It is recommended to specify how the PromQL query will handle write-idle periods (e.g., by checking if the primary's LSN is advancing, or using byte-based lag like pg_wal_lsn_diff as a fallback/supplement) to prevent false-positive scaling freezes.
|
|
||
| ## Security | ||
|
|
||
| - **RBAC.** The operator needs: read DHA and read/patch `applications.apps.cozystack.io`, read `workloadmonitors.cozystack.io`, read `pods`, and read-only HTTP to vmselect. It has no direct access to engine operator CRs or Flux `HelmRelease` objects — only the aggregated apps API. |
There was a problem hiding this comment.
The proposed RBAC permissions do not include access to ResourceQuota or custom Tenant resources, nor do they include access to any platform configuration ConfigMaps defining resource presets. However, the "Tenant quota" guardrail states that the autoscaler must validate whether the new replica count × preset resources fits within the tenant quota.
To perform this validation, the operator will need RBAC permissions to read these quota and preset resources. Please update the RBAC section to include these permissions, or clarify how the quota validation will be performed without them.
![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
🤖 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`:
- Around line 142-143: Clarify the enforced `replicas` ownership policy for the
DHA so it is not just advisory: specify which controller or writer is the
explicit owner when a DHA is active, and how conflicting declarative updates are
blocked or resolved. Update the `Ownership` section to describe the concrete
conflict path using the relevant DHA autoscaler behavior, and state whether this
is enforced via a lock/marker, admission rejection, or a single explicit winner.
Make it clear that `RetryOnConflict` alone is insufficient and should not be
presented as the ownership mechanism.
- Around line 40-43: The proposal is inconsistent about the owned write target
for autoscaling, switching between patching the Application values and PATCHing
spec.replicas. Pick one single owned path and use the same term everywhere in
the README so the controller write target is unambiguous; update the wording
around Application and the scaling flow to consistently refer to that chosen
field/path.
🪄 Autofix (Beta)
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
Run ID: 3fdc3ad6-15e6-4257-aa9d-9d14b06d30bb
📒 Files selected for processing (1)
design-proposals/database-horizontal-autoscaling/README.md
| - Apply all decisions by patching the `Application` values (Flux-compatible write path), never the operator CR directly. | ||
| - Reuse existing telemetry (VictoriaMetrics + `WorkloadMonitor`); introduce no new exporters. | ||
| - Be safe for stateful workloads: respect the replica quorum, brake on replication lag, drain read connections on scale-down, use long stabilization windows, and honor tenant quotas. | ||
| - Provide HPA-like observability: status conditions, events, and a `dryRun` mode. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file structure first
wc -l design-proposals/database-horizontal-autoscaling/README.md
ast-grep outline design-proposals/database-horizontal-autoscaling/README.md --view expanded
# Inspect the cited areas and nearby context
sed -n '1,130p' design-proposals/database-horizontal-autoscaling/README.md | cat -n
# Find every mention of Application values / replicas / patching to compare wording
rg -n "Application values|spec\.replicas|PATCH|write path|Flux|operator CR|owned" design-proposals/database-horizontal-autoscaling/README.mdRepository: cozystack/community
Length of output: 13908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the Application API shape and any replicas/value fields in this repo
rg -n "type Application struct|spec\.replicas|replicas value|HelmRelease\.Spec\.Values|Values" -g'*.go' -g'*.md' .
# Show the most relevant Application definitions if present
fd -a "application*.go" .
fd -a "application*.yaml" .Repository: cozystack/community
Length of output: 2858
Clarify the owned Application path.
The proposal alternates between “patching the Application values” and PATCH spec.replicas on the Application; pick one owned field/path and use the same name everywhere so the write target is unambiguous.
🤖 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 40 -
43, The proposal is inconsistent about the owned write target for autoscaling,
switching between patching the Application values and PATCHing spec.replicas.
Pick one single owned path and use the same term everywhere in the README so the
controller write target is unambiguous; update the wording around Application
and the scaling flow to consistently refer to that chosen field/path.
| - **Ownership.** While a DHA is active, `replicas` is considered owned by the autoscaler. This is mutually exclusive with managing `replicas` from a tenant GitOps repo for the same application; documented explicitly. | ||
| - **Rollback.** Deleting the DHA stops all autoscaling immediately and leaves the application at its current `replicas`. Disabling the package removes the operator; no data migration is involved and the change is fully reversible. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline design-proposals/database-horizontal-autoscaling/README.md --view expanded || true
printf '\n== Relevant lines around 120-180 ==\n'
sed -n '120,180p' design-proposals/database-horizontal-autoscaling/README.md | cat -n
printf '\n== Search for ownership/conflict-related terms in proposal ==\n'
rg -n --no-heading -i 'ownership|own(ed|er|s)|conflict|exclusive|race|flux|lease|annotation|admission|webhook|field manager|field ownership|replica' design-proposals/database-horizontal-autoscaling README.md . || trueRepository: cozystack/community
Length of output: 50377
Define enforced replicas ownership. The ownership policy is still open, and RetryOnConflict only handles API write conflicts; it does not stop Flux or another declarative writer from repeatedly reconciling the same field. Spell out the conflict path here: lock/marker, admission rejection, or an explicit winner.
🤖 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 142
- 143, Clarify the enforced `replicas` ownership policy for the DHA so it is not
just advisory: specify which controller or writer is the explicit owner when a
DHA is active, and how conflicting declarative updates are blocked or resolved.
Update the `Ownership` section to describe the concrete conflict path using the
relevant DHA autoscaler behavior, and state whether this is enforced via a
lock/marker, admission rejection, or a single explicit winner. Make it clear
that `RetryOnConflict` alone is insufficient and should not be presented as the
ownership mechanism.
1 participant
Summary
Adds a design proposal for a Database Horizontal Autoscaler (
db-autoscaler) — a dedicated operator that automatically scales the number of read replicas of managed databases (postgres,mariadb,redis,mongodb) based on load.Proposal:
design-proposals/database-horizontal-autoscaling/README.mdHighlights
DatabaseHorizontalAutoscaler(autoscaling.cozystack.io/v1alpha1).Applicationvalues — Flux/GitOps-safe, never fights reconciliation.WorkloadMonitor; no new exporters.dryRun.Scope
Horizontal (read replicas) only. Vertical and storage autoscaling are deferred to sibling proposals; write-path/sharding rebalance is out of scope.
Status: Draft — feedback welcome, especially on the default driver metric and scale-down defaults (see Open questions).
Summary by CodeRabbit