diff --git a/config/dev/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml b/config/dev/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml index ff275a5f73..4632a0ae15 100644 --- a/config/dev/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml +++ b/config/dev/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml @@ -2572,6 +2572,15 @@ spec: items: type: string type: array + pgBackRestInfoThrottleMinutes: + default: 10 + description: |- + Minimum number of minutes between pgBackRest info collection runs when + OpenTelemetry metrics are enabled. Lower values update backup metrics more + frequently but can increase cloud egress from object storage-backed repos. + format: int32 + minimum: 0 + type: integer type: object resources: description: Resources holds the resource requirements for the diff --git a/config/dev/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/dev/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index da98a03c56..d9043d1989 100644 --- a/config/dev/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/dev/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -13218,6 +13218,15 @@ spec: items: type: string type: array + pgBackRestInfoThrottleMinutes: + default: 10 + description: |- + Minimum number of minutes between pgBackRest info collection runs when + OpenTelemetry metrics are enabled. Lower values update backup metrics more + frequently but can increase cloud egress from object storage-backed repos. + format: int32 + minimum: 0 + type: integer type: object resources: description: Resources holds the resource requirements for the @@ -33379,6 +33388,15 @@ spec: items: type: string type: array + pgBackRestInfoThrottleMinutes: + default: 10 + description: |- + Minimum number of minutes between pgBackRest info collection runs when + OpenTelemetry metrics are enabled. Lower values update backup metrics more + frequently but can increase cloud egress from object storage-backed repos. + format: int32 + minimum: 0 + type: integer type: object resources: description: Resources holds the resource requirements for the diff --git a/internal/controller/postgrescluster/metrics_setup.sql b/internal/controller/postgrescluster/metrics_setup.sql index 2d282643ce..45876161d0 100644 --- a/internal/controller/postgrescluster/metrics_setup.sql +++ b/internal/controller/postgrescluster/metrics_setup.sql @@ -30,6 +30,13 @@ CREATE TABLE monitor.pg_stat_statements_reset_info( reset_time timestamptz ); +DROP TABLE IF EXISTS monitor.pgbackrest_info_cache; +-- Table to cache pgBackRest info output and avoid frequent cloud egress. +CREATE TABLE monitor.pgbackrest_info_cache( + collected_at timestamptz DEFAULT now() NOT NULL, + data json NOT NULL +); + DROP FUNCTION IF EXISTS monitor.pg_stat_statements_reset_info(int); -- Function to reset pg_stat_statements periodically CREATE FUNCTION monitor.pg_stat_statements_reset_info(p_throttle_minutes integer DEFAULT 1440) @@ -89,7 +96,9 @@ DROP FUNCTION IF EXISTS get_pgbackrest_info(); --- get_pgbackrest_info is used by the OTel collector. --- get_pgbackrest_info is created as a function so that no ddl runs on a replica. --- In the query, the --stanza argument matches DefaultStanzaName, defined in internal/pgbackrest/config.go. -CREATE FUNCTION get_pgbackrest_info() +--- To match legacy postgres_exporter behavior, pgbackrest info output is refreshed +--- at most once every 10 minutes and cached in monitor.pgbackrest_info_cache. +CREATE FUNCTION get_pgbackrest_info(p_throttle_minutes integer DEFAULT __PGBACKREST_INFO_THROTTLE_MINUTES__) RETURNS TABLE ( last_diff_backup BIGINT, last_full_backup BIGINT, @@ -102,6 +111,9 @@ RETURNS TABLE ( oldest_full_backup BIGINT, repo TEXT ) AS $$ +DECLARE + v_collected_timestamp timestamptz; + v_throttle interval; BEGIN IF pg_is_in_recovery() THEN RETURN QUERY @@ -117,16 +129,33 @@ BEGIN 0::bigint AS oldest_full_backup, 'n/a' AS repo; ELSE - DROP TABLE IF EXISTS pgbackrest_info; - CREATE TEMPORARY TABLE pgbackrest_info (data json); - COPY pgbackrest_info (data) - FROM PROGRAM 'export LC_ALL=C && printf "\f" && pgbackrest info --log-level-console=info --log-level-stderr=warn --output=json --stanza=db && printf "\f"' - WITH (FORMAT csv, HEADER false, QUOTE E'\f'); + IF p_throttle_minutes < 0 THEN + p_throttle_minutes := 0; + END IF; + + v_throttle := make_interval(mins := p_throttle_minutes); + + SELECT max(collected_at) + INTO v_collected_timestamp + FROM monitor.pgbackrest_info_cache; + + IF v_collected_timestamp IS NULL OR ((CURRENT_TIMESTAMP - v_collected_timestamp) > v_throttle) THEN + DROP TABLE IF EXISTS pgbackrest_info_tmp; + CREATE TEMPORARY TABLE pgbackrest_info_tmp (data json); + COPY pgbackrest_info_tmp (data) + FROM PROGRAM 'export LC_ALL=C && printf "\f" && pgbackrest info --log-level-console=info --log-level-stderr=warn --output=json --stanza=db && printf "\f"' + WITH (FORMAT csv, HEADER false, QUOTE E'\f'); + + DELETE FROM monitor.pgbackrest_info_cache; + INSERT INTO monitor.pgbackrest_info_cache(collected_at, data) + SELECT CURRENT_TIMESTAMP, data FROM pgbackrest_info_tmp; + END IF; RETURN QUERY WITH all_backups (data) AS ( - SELECT jsonb_array_elements(to_jsonb(data)) FROM pgbackrest_info + SELECT jsonb_array_elements(to_jsonb(data)) + FROM monitor.pgbackrest_info_cache ), stanza_backups (stanza, backup) AS ( SELECT data->>'name', jsonb_array_elements(data->'backup') FROM all_backups diff --git a/internal/controller/postgrescluster/pgmonitor.go b/internal/controller/postgrescluster/pgmonitor.go index 44f38acb6e..0d0008445c 100644 --- a/internal/controller/postgrescluster/pgmonitor.go +++ b/internal/controller/postgrescluster/pgmonitor.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "os" + "strconv" "strings" "github.com/pkg/errors" @@ -145,7 +146,21 @@ func (r *Reconciler) reconcileExporterSqlSetup(ctx context.Context, // we can assume that postgres_exporter is enabled and we should // use that if collector.OpenTelemetryMetricsEnabled(ctx, cluster) { - return metricsSetupForOTelCollector, nil + throttleMinutes := int32(10) + if cluster.Spec.Instrumentation != nil && + cluster.Spec.Instrumentation.Metrics != nil && + cluster.Spec.Instrumentation.Metrics.PGBackRestInfoThrottleMinutes != nil { + throttleMinutes = *cluster.Spec.Instrumentation.Metrics.PGBackRestInfoThrottleMinutes + } + + withThrottle := strings.Replace( + metricsSetupForOTelCollector, + "__PGBACKREST_INFO_THROTTLE_MINUTES__", + strconv.FormatInt(int64(throttleMinutes), 10), + 1, + ) + + return withThrottle, nil } // pgMonitor will not be adding support for postgres_exporter for postgres diff --git a/internal/controller/postgrescluster/pgmonitor_test.go b/internal/controller/postgrescluster/pgmonitor_test.go index aed23ab0d8..8c35241c7b 100644 --- a/internal/controller/postgrescluster/pgmonitor_test.go +++ b/internal/controller/postgrescluster/pgmonitor_test.go @@ -867,6 +867,8 @@ func TestReconcileExporterSqlSetup(t *testing.T) { Image: "image", } + throttleFive := int32(5) + testCases := []struct { tcName string postgresVersion int32 @@ -874,6 +876,7 @@ func TestReconcileExporterSqlSetup(t *testing.T) { otelMetricsEnabled bool errorPresent bool setupEmpty bool + expectedThrottle string expectedNumEvents int expectedEvent string }{{ @@ -892,6 +895,17 @@ func TestReconcileExporterSqlSetup(t *testing.T) { otelMetricsEnabled: true, errorPresent: false, setupEmpty: false, + expectedThrottle: "DEFAULT 10", + expectedNumEvents: 0, + expectedEvent: "", + }, { + tcName: "ExporterDisabledOtelEnabledCustomThrottle", + postgresVersion: 17, + exporterEnabled: false, + otelMetricsEnabled: true, + errorPresent: false, + setupEmpty: false, + expectedThrottle: "DEFAULT 5", expectedNumEvents: 0, expectedEvent: "", }, { @@ -901,6 +915,7 @@ func TestReconcileExporterSqlSetup(t *testing.T) { otelMetricsEnabled: true, errorPresent: false, setupEmpty: false, + expectedThrottle: "DEFAULT 10", expectedNumEvents: 0, expectedEvent: "", }, { @@ -919,6 +934,7 @@ func TestReconcileExporterSqlSetup(t *testing.T) { otelMetricsEnabled: true, errorPresent: false, setupEmpty: false, + expectedThrottle: "DEFAULT 10", expectedNumEvents: 0, expectedEvent: "", }, { @@ -928,6 +944,7 @@ func TestReconcileExporterSqlSetup(t *testing.T) { otelMetricsEnabled: true, errorPresent: false, setupEmpty: false, + expectedThrottle: "DEFAULT 10", expectedNumEvents: 0, expectedEvent: "", }, { @@ -956,7 +973,12 @@ func TestReconcileExporterSqlSetup(t *testing.T) { ctx := feature.NewContext(ctx, gate) if tc.otelMetricsEnabled { - cluster.Spec.Instrumentation = instrumentationSpec + cluster.Spec.Instrumentation = instrumentationSpec.DeepCopy() + if tc.tcName == "ExporterDisabledOtelEnabledCustomThrottle" { + cluster.Spec.Instrumentation.Metrics = &v1beta1.InstrumentationMetricsSpec{ + PGBackRestInfoThrottleMinutes: &throttleFive, + } + } } if tc.exporterEnabled { @@ -970,6 +992,9 @@ func TestReconcileExporterSqlSetup(t *testing.T) { assert.NilError(t, err) } assert.Equal(t, setup == "", tc.setupEmpty) + if tc.expectedThrottle != "" { + assert.Assert(t, strings.Contains(setup, tc.expectedThrottle)) + } assert.Equal(t, len(recorder.Events), tc.expectedNumEvents) if tc.expectedNumEvents == 1 { diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go index 139786f3c4..6be4b3dd5c 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go @@ -117,6 +117,15 @@ type InstrumentationMetricsSpec struct { // +optional CustomQueries *InstrumentationCustomQueriesSpec `json:"customQueries,omitempty"` + // Minimum number of minutes between pgBackRest info collection runs when + // OpenTelemetry metrics are enabled. Lower values update backup metrics more + // frequently but can increase cloud egress from object storage-backed repos. + // --- + // +kubebuilder:validation:Minimum=0 + // +default=10 + // +optional + PGBackRestInfoThrottleMinutes *int32 `json:"pgBackRestInfoThrottleMinutes,omitempty"` + // The names of exporters that should send metrics. // --- // +kubebuilder:validation:MinItems=1 diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go index 6b9125b5c0..e565f00099 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go @@ -416,6 +416,11 @@ func (in *InstrumentationMetricsSpec) DeepCopyInto(out *InstrumentationMetricsSp *out = new(InstrumentationCustomQueriesSpec) (*in).DeepCopyInto(*out) } + if in.PGBackRestInfoThrottleMinutes != nil { + in, out := &in.PGBackRestInfoThrottleMinutes, &out.PGBackRestInfoThrottleMinutes + *out = new(int32) + **out = **in + } if in.Exporters != nil { in, out := &in.Exporters, &out.Exporters *out = make([]string, len(*in))