From 9fea1e92fd58331115e860fc509dd1969b7d281c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Thu, 2 Jul 2026 04:59:12 +1200 Subject: [PATCH] feat(canopy): shm_size_floor spec + intent-driven floors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-land the shm-floor knob from the closed #80 on top of the CR materialisation refactor. `compute_shm_and_shared_buffers` derives shm from the pod's memory request/limit; for analytics workloads that derived value is often lower than what shared_buffers benefits from, and bumping memory request just to raise shm burns k8s scheduling budget for no real gain. Adds: - `PostgresPhysicalReplicaSpec.shmSizeFloor: Option` — the Deployment builder now takes `max(computed_shm, floor)` and scales `shared_buffers_mb` proportionally so postgres actually uses the extra shm. - `IntentConfig.shm_size_floor` (verify: 512Mi, analytics-dev / -dbt: 2Gi) — materialised into the CR spec by IntentConfig::to_replica_spec. --- README.md | 1 + crds.yaml | 11 ++++++ src/controllers/canopy/intent.rs | 11 ++++++ src/controllers/replica/scheduling.rs | 1 + src/controllers/replica/schema_migration.rs | 1 + src/controllers/replica/tests.rs | 2 ++ src/controllers/restore/builders.rs | 37 ++++++++++++++++++++- src/controllers/restore/tests.rs | 2 ++ src/types/replica.rs | 10 ++++++ tests/helpers.rs | 1 + 10 files changed, 76 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4626e95..685fead 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,7 @@ Defines a continuously-refreshed replica of a PostgreSQL database restored from | `storageSizeOverride` | `Quantity` | No | — | Override dynamic sizing with a fixed PVC size. When absent, PVC size is calculated from snapshot size. | | `storageSizeMaximum` | `Quantity` | No | `2Ti` | Maximum allowed PVC size. The restore will fail if the computed size exceeds this limit. | | `resources` | `ResourceRequirements` | No | — | CPU/memory resource requirements for the PostgreSQL pods. | +| `shmSizeFloor` | `Quantity` | No | — | Floor on the postgres pod's `/dev/shm` sizing. When set, the Deployment uses `max(computed, shmSizeFloor)` — the computed value is derived from `resources` by [`compute_shm_and_shared_buffers`]. Useful when a workload's `shared_buffers` needs more shm than the resource-derived value provides, without wanting to bump the container's memory request. | | `serviceAnnotations` | `map[string]string` | No | — | Annotations applied to the Service. | | `podAnnotations` | `map[string]string` | No | — | Annotations applied to the PostgreSQL pods. | | `affinity` | `Affinity` | No | — | Pod scheduling affinity rules. | diff --git a/crds.yaml b/crds.yaml index 1527aa6..303bb4d 100644 --- a/crds.yaml +++ b/crds.yaml @@ -704,6 +704,17 @@ spec: type: string nullable: true type: object + shmSizeFloor: + description: |- + Floor on the postgres pod's `/dev/shm` sizing. When set, the + Deployment builder uses `max(computed, shmSizeFloor)` — computed + comes from [`compute_shm_and_shared_buffers`] driven by + [`resources`]. Useful when the resource-derived value would be + smaller than what a workload's `shared_buffers` needs (analytics + / dbt) without wanting to bump the container's memory request + upward just to raise shm. + nullable: true + x-kubernetes-int-or-string: true snapshotFilter: nullable: true properties: diff --git a/src/controllers/canopy/intent.rs b/src/controllers/canopy/intent.rs index 85ccf94..b5b6387 100644 --- a/src/controllers/canopy/intent.rs +++ b/src/controllers/canopy/intent.rs @@ -41,6 +41,13 @@ pub struct IntentConfig { pub service_annotations: Option>, pub switchover_grace_period: TimeSpan, pub storage_size_override: Quantity, + /// Floor on the postgres pod's `/dev/shm` sizing. Materialised into + /// `PostgresPhysicalReplicaSpec.shm_size_floor` so the shared + /// Deployment builder picks `max(computed_from_resources, floor)`. + /// Analytics workloads (`analytics-dev` / `analytics-dbt`) want a + /// higher shm than what a 2 GiB memory request would derive, without + /// paying the k8s scheduling cost of bumping the request. + pub shm_size_floor: Quantity, } fn resources(cpu_req: &str, mem_req: &str, cpu_lim: &str, mem_lim: &str) -> ResourceRequirements { @@ -72,6 +79,7 @@ pub fn config_for(intent: &str) -> Option { service_annotations: None, switchover_grace_period: TimeSpan(Span::new().minutes(5)), storage_size_override: Quantity("20Gi".to_string()), + shm_size_floor: Quantity("512Mi".to_string()), }), "analytics-dev" => Some(IntentConfig { resources: Some(resources("500m", "2Gi", "4", "8Gi")), @@ -81,6 +89,7 @@ pub fn config_for(intent: &str) -> Option { service_annotations: None, switchover_grace_period: TimeSpan(Span::new().minutes(5)), storage_size_override: Quantity("50Gi".to_string()), + shm_size_floor: Quantity("2Gi".to_string()), }), "analytics-dbt" => Some(IntentConfig { resources: Some(resources("500m", "2Gi", "4", "8Gi")), @@ -96,6 +105,7 @@ pub fn config_for(intent: &str) -> Option { ])), switchover_grace_period: TimeSpan(Span::new().minutes(2)), storage_size_override: Quantity("50Gi".to_string()), + shm_size_floor: Quantity("2Gi".to_string()), }), _ => None, } @@ -141,6 +151,7 @@ impl IntentConfig { storage_class: None, storage_size_override: Some(self.storage_size_override.clone()), resources: self.resources.clone(), + shm_size_floor: Some(self.shm_size_floor.clone()), service_annotations: self .service_annotations .clone() diff --git a/src/controllers/replica/scheduling.rs b/src/controllers/replica/scheduling.rs index 89b73f9..fe4957f 100644 --- a/src/controllers/replica/scheduling.rs +++ b/src/controllers/replica/scheduling.rs @@ -267,6 +267,7 @@ mod tests { storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None, diff --git a/src/controllers/replica/schema_migration.rs b/src/controllers/replica/schema_migration.rs index e7a99df..3a66884 100644 --- a/src/controllers/replica/schema_migration.rs +++ b/src/controllers/replica/schema_migration.rs @@ -296,6 +296,7 @@ mod tests { storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None, diff --git a/src/controllers/replica/tests.rs b/src/controllers/replica/tests.rs index e45bf7e..a85572e 100644 --- a/src/controllers/replica/tests.rs +++ b/src/controllers/replica/tests.rs @@ -32,6 +32,7 @@ fn make_replica( storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None, @@ -191,6 +192,7 @@ fn snapshot_list_job_rotates_kopia_logs() { storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None, diff --git a/src/controllers/restore/builders.rs b/src/controllers/restore/builders.rs index 63ed9b2..9b302dc 100644 --- a/src/controllers/restore/builders.rs +++ b/src/controllers/restore/builders.rs @@ -952,7 +952,13 @@ pub fn build_deployment( replica: &PostgresPhysicalReplica, ) -> Result { let pvc_name = format!("{name}-data"); - let (shm_size, shared_buffers_mb) = compute_shm_and_shared_buffers(&replica.spec.resources); + let (computed_shm, computed_shared_buffers_mb) = + compute_shm_and_shared_buffers(&replica.spec.resources); + let (shm_size, shared_buffers_mb) = apply_shm_floor( + &computed_shm, + computed_shared_buffers_mb, + replica.spec.shm_size_floor.as_ref(), + ); let creds_secret = SecretReference { name: Some(format!("{}-creds", restore.spec.replica.name)), namespace: Some(namespace.to_string()), @@ -1001,6 +1007,35 @@ pub fn build_deployment( }) } +/// Apply the caller's `shm_size_floor` (if any) to the resource-derived +/// shm + shared_buffers pair. `shared_buffers` scales linearly with shm +/// (postgres wants ~70 % of shm), so bumping shm bumps +/// `shared_buffers_mb` proportionally. +fn apply_shm_floor( + computed_shm: &Quantity, + computed_shared_buffers_mb: u64, + floor: Option<&Quantity>, +) -> (Quantity, u64) { + let Some(floor) = floor else { + return (computed_shm.clone(), computed_shared_buffers_mb); + }; + let computed_bytes = ParsedQuantity::try_from(computed_shm.clone()) + .ok() + .and_then(|q| q.to_bytes_f64()); + let floor_bytes = ParsedQuantity::try_from(floor.clone()) + .ok() + .and_then(|q| q.to_bytes_f64()); + match (computed_bytes, floor_bytes) { + (Some(c), Some(f)) if f > c => { + let ratio = f / c; + let scaled_shared_buffers = + ((computed_shared_buffers_mb as f64) * ratio).floor() as u64; + (floor.clone(), scaled_shared_buffers.max(16)) + } + _ => (computed_shm.clone(), computed_shared_buffers_mb), + } +} + /// Shared postgres Deployment builder — no CR dependencies. Both the /// CRD path (`build_deployment`) and the canopy path /// (`controllers::canopy::builders::build_canopy_postgres_deployment`) go diff --git a/src/controllers/restore/tests.rs b/src/controllers/restore/tests.rs index 9c6ec92..37ba527 100644 --- a/src/controllers/restore/tests.rs +++ b/src/controllers/restore/tests.rs @@ -31,6 +31,7 @@ fn deployment_uses_affinity_not_node_selector() { storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None, @@ -116,6 +117,7 @@ fn test_restore_and_replica() -> (PostgresPhysicalRestore, PostgresPhysicalRepli storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None, diff --git a/src/types/replica.rs b/src/types/replica.rs index ca0941e..33524c0 100644 --- a/src/types/replica.rs +++ b/src/types/replica.rs @@ -92,6 +92,16 @@ pub struct PostgresPhysicalReplicaSpec { #[serde(default, skip_serializing_if = "Option::is_none")] pub resources: Option, + /// Floor on the postgres pod's `/dev/shm` sizing. When set, the + /// Deployment builder uses `max(computed, shmSizeFloor)` — computed + /// comes from [`compute_shm_and_shared_buffers`] driven by + /// [`resources`]. Useful when the resource-derived value would be + /// smaller than what a workload's `shared_buffers` needs (analytics + /// / dbt) without wanting to bump the container's memory request + /// upward just to raise shm. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub shm_size_floor: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub service_annotations: Option>, diff --git a/tests/helpers.rs b/tests/helpers.rs index 0254c19..d6b7f72 100644 --- a/tests/helpers.rs +++ b/tests/helpers.rs @@ -152,6 +152,7 @@ pub fn build_replica(name: &str, secret_ref: &str, opts: ReplicaOpts) -> Postgre storage_class: None, storage_size_override: None, resources: None, + shm_size_floor: None, service_annotations: None, pod_annotations: None, affinity: None,