feat: add PostgreSQL parameter tuning with memory-aware defaults#307
feat: add PostgreSQL parameter tuning with memory-aware defaults#307xgerman wants to merge 2 commits intodocumentdb:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds PostgreSQL parameter tuning and resource configuration to the DocumentDB Kubernetes Operator by introducing a spec.postgresParameters override map, optional CPU/memory limits, and memory-aware parameter defaults that are propagated into the underlying CloudNativePG (CNPG) Cluster.
Changes:
- Add memory-aware + static default PostgreSQL parameter computation with protected parameters and user overrides.
- Wire
spec.resource.memory/cpuinto CNPG Clusterspec.resourcesusing Guaranteed QoS (requests == limits). - Add docs + mkdocs navigation for PostgreSQL tuning guidance, and unit tests for defaults logic.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| operator/src/internal/controller/documentdb_controller.go | Adds patching logic to update CNPG cluster for parameter/resource changes. |
| operator/src/internal/cnpg/cnpg_cluster.go | Uses merged parameter defaults and builds CNPG resource requirements from CRD CPU/memory fields. |
| operator/src/internal/cnpg/pg_defaults.go | Implements memory-aware defaults, static defaults, protected params, and merge logic. |
| operator/src/internal/cnpg/pg_defaults_test.go | Adds unit tests covering defaults and merge behavior. |
| operator/src/api/preview/documentdb_types.go | Extends the CRD Go types with postgresParameters, resource.memory, and resource.cpu. |
| operator/src/api/preview/zz_generated.deepcopy.go | Adds deep-copy support for the new PostgresParameters map. |
| operator/src/config/crd/bases/documentdb.io_dbs.yaml | Updates generated CRD schema to include new fields (and modifies an XValidation rule). |
| operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml | Mirrors CRD schema updates for Helm distribution. |
| mkdocs.yml | Adds nav entry for the new PostgreSQL tuning doc. |
| docs/operator-public-documentation/postgresql-tuning.md | Adds tuning documentation for defaults, overrides, and resource configuration. |
- Fix smart quotes in XValidation CEL rule that broke CRD installation - Replace resource.MustParse with ParseQuantity to prevent controller panics - Change JSON patch op from 'replace' to 'add' for spec/resources and spec/postgresql/parameters - Fix work_mem documentation values (7MB -> 6MB for 8Gi) to match implementation - Renumber step comments after inserting new Step 3 - Add unit tests for parseMemoryToBytes and buildResourceRequirements (100% coverage) - Apply go fmt formatting fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: German <geeichbe@microsoft.com>
| // defaults are used for memory-aware parameters. | ||
| // Examples: "2Gi", "4Gi", "8Gi" | ||
| // +optional | ||
| Memory string `json:"memory,omitempty"` |
There was a problem hiding this comment.
Should we call it MemoryLimit then?
There was a problem hiding this comment.
Good question! I kept it as memory (rather than memoryLimit) for two reasons:
-
It sets both
requests.memoryANDlimits.memory(Guaranteed QoS) — so calling it "limit" would be misleading since it is equally the "request". It is really the memory allocation for the pod. -
Follows the CNPG convention — CloudNative-PG uses
spec.resources.limits.memorybut the user-facing concept is "how much memory should this database pod have."
Happy to rename if you feel strongly about it — since this is a preview API, now is the time to get the name right. Let me know!
- Add postgresParameters map to CRD for user overrides - Add memory and cpu resource fields to CRD (default unlimited) - Implement memory-aware defaults (shared_buffers=25%, effective_cache_size=75%, etc.) - Add static best-practice defaults (max_connections=300, autovacuum, WAL, IO) - Add protected parameters (cron.database_name, replication slots, prepared txns) - Wire CPU/memory into CNPG cluster with Guaranteed QoS (requests=limits) - Add controller patching for parameter and resource changes - Add 30 unit tests for pg_defaults.go (87% coverage) - Add PostgreSQL tuning documentation guide Closes documentdb#255 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: German <geeichbe@microsoft.com>
- Fix smart quotes in XValidation CEL rule that broke CRD installation - Replace resource.MustParse with ParseQuantity to prevent controller panics - Change JSON patch op from 'replace' to 'add' for spec/resources and spec/postgresql/parameters - Fix work_mem documentation values (7MB -> 6MB for 8Gi) to match implementation - Renumber step comments after inserting new Step 3 - Add unit tests for parseMemoryToBytes and buildResourceRequirements (100% coverage) - Apply go fmt formatting fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: German <geeichbe@microsoft.com>
Closes #255