Repository: cloudnative-pg/cloudnative-pg

Issue:
[Feature]: Set `STANDBY_TCP_USER_TIMEOUT` default
### Is there an existing issue already for this feature request/idea?

- [x] I have searched for an existing issue, and could not find anything. I believe this is a new feature request to be evaluated.

### What problem is this feature going to solve? Why should it be added?

Currently, the `STANDBY_TCP_USER_TIMEOUT` operator-level configuration option is set to 0 by default, which is the system's default value (see #7119 and PR #7036).

### Describe the solution you'd like

To improve the default installation of CNPG, I propose setting it to a 10 seconds by default. The scenario is the one depicted by @fcanovai in his [proposal of patch for PostgreSQL](https://www.postgresql.org/message-id/CAJNLuXDoMhyTvQT9d3WkxQmOuc-=PQfB0khGcmHDzibp1ti7zQ@mail.gmail.com).

### Describe alternatives you've considered

N/A

### Additional context

_No response_

### Backport?

No

### Are you willing to actively contribute to this feature?

Yes

### Code of Conduct

- [x] I agree to follow this project's Code of Conduct

Discussion:
@mnencia: In our E2E, we have been setting STANDBY_TCP_USER_TIMEOUT to 5 seconds for a long time. It is a reasonable value and should work well for the majority of the deployments. I'm fine with enabling it by default to 5 or 10 seconds, as long as we clearly communicate the change and explain how to revert to the previous behavior.

Solution patch:
diff --git a/docs/src/operator_conf.md b/docs/src/operator_conf.md
index c322fee6..de9aadfa 100644
--- a/docs/src/operator_conf.md
+++ b/docs/src/operator_conf.md
@@ -59,7 +59,7 @@ Name | Description
 `PGBOUNCER_IMAGE_NAME` | The name of the PgBouncer image used by default for new poolers. Defaults to the version specified in the operator.
 `POSTGRES_IMAGE_NAME` | The name of the PostgreSQL image used by default for new clusters. Defaults to the version specified in the operator.
 `PULL_SECRET_NAME` | Name of an additional pull secret to be defined in the operator's namespace and to be used to download images
-`STANDBY_TCP_USER_TIMEOUT` | Defines the [`TCP_USER_TIMEOUT` socket option](https://www.postgresql.org/docs/current/runtime-config-connection.html#GUC-TCP-USER-TIMEOUT) for replication connections from standby instances to the primary. Default is 0 (system's default).
+`STANDBY_TCP_USER_TIMEOUT` | Defines the [`TCP_USER_TIMEOUT` socket option](https://www.postgresql.org/docs/current/runtime-config-connection.html#GUC-TCP-USER-TIMEOUT) in milliseconds for replication connections from standby instances to the primary. Default is 5000 (5 seconds). Set to `0` to use the system's default.
 `DRAIN_TAINTS` | Specifies the taint keys that should be interpreted as indicators of node drain. By default, it includes the taints commonly applied by [kubectl](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/), [Cluster Autoscaler](https://github.com/kubernetes/autoscaler), and [Karpenter](https://github.com/aws/karpenter-provider-aws): `node.kubernetes.io/unschedulable`, `ToBeDeletedByClusterAutoscaler`, `karpenter.sh/disrupted`, `karpenter.sh/disruption`.
 
 Values in `INHERITED_ANNOTATIONS` and `INHERITED_LABELS` support path-like wildcards. For example, the value `example.com/*` will match
diff --git a/docs/src/postgresql_conf.md b/docs/src/postgresql_conf.md
index a1128cd1..1c6d0246 100644
--- a/docs/src/postgresql_conf.md
+++ b/docs/src/postgresql_conf.md
@@ -168,19 +168,20 @@ role within the cluster. These parameters are effectively applied only when the
 instance is operating as a replica.
 
 ```text
-primary_conninfo = 'host=<PRIMARY> user=postgres dbname=postgres'
+primary_conninfo = 'host=<PRIMARY> user=postgres dbname=postgres tcp_user_timeout=5000'
 recovery_target_timeline = 'latest'
 ```
 
-The [`STANDBY_TCP_USER_TIMEOUT` operator configuration setting](operator_conf.md#available-options),
-if specified, sets the `tcp_user_timeout` parameter on all standby instances
-managed by the operator.
-
-The `tcp_user_timeout` parameter determines how long transmitted data can
-remain unacknowledged before the TCP connection is forcibly closed. Adjusting
-this value allows you to fine-tune the responsiveness of standby instances to
-network disruptions. For more details, refer to the
-[PostgreSQL documentation](https://www.postgresql.org/docs/current/runtime-config-connection.html#GUC-TCP-USER-TIMEOUT).
+!!! Important
+    By default, every standby sets `tcp_user_timeout` to **5 seconds**, as shown
+    above. This parameter defines how long transmitted data may remain
+    unacknowledged before the TCP connection is forcibly closed. Adjusting it lets
+    you control how quickly a standby reacts to network issues.
+    If the default value does not meet your requirements, you can override it
+    for all standbys managed by the operator using the
+    [`STANDBY_TCP_USER_TIMEOUT` operator configuration option](operator_conf.md#available-options).
+    For additional details on `tcp_user_timeout`, refer to the
+    [PostgreSQL documentation](https://www.postgresql.org/docs/current/runtime-config-connection.html#GUC-TCP-USER-TIMEOUT).
 
 ### Log control settings
 
diff --git a/internal/configuration/configuration.go b/internal/configuration/configuration.go
index 66fe83df..e0076d88 100644
--- a/internal/configuration/configuration.go
+++ b/internal/configuration/configuration.go
@@ -165,7 +165,9 @@ type Data struct {
 	// added as a tcp_user_timeout option to the primary_conninfo
 	// string, which is used by the standby server to connect to the
 	// primary server in CloudNativePG.
-	StandbyTCPUserTimeout int `json:"standbyTcpUserTimeout" env:"STANDBY_TCP_USER_TIMEOUT"`
+	// When nil, the instance manager will use a default value of 5000ms.
+	// Set to 0 explicitly to use the system's default.
+	StandbyTCPUserTimeout *int `json:"standbyTcpUserTimeout" env:"STANDBY_TCP_USER_TIMEOUT"`
 
 	// KubernetesClusterDomain defines the domain suffix for service FQDNs
 	// within the Kubernetes cluster. If left unset, it defaults to `cluster.local`.
@@ -189,7 +191,7 @@ func newDefaultConfig() *Data {
 		CreateAnyService:        false,
 		CertificateDuration:     CertificateDuration,
 		ExpiringCheckThreshold:  ExpiringCheckThreshold,
-		StandbyTCPUserTimeout:   0,
+		StandbyTCPUserTimeout:   nil,
 		KubernetesClusterDomain: DefaultKubernetesClusterDomain,
 		DrainTaints:             DefaultDrainTaints,
 	}
diff --git a/pkg/configparser/configparser.go b/pkg/configparser/configparser.go
index af87d657..4182fa6c 100644
--- a/pkg/configparser/configparser.go
+++ b/pkg/configparser/configparser.go
@@ -64,7 +64,7 @@ var configparserLog = log.WithName("configparser")
 
 // ReadConfigMap reads the configuration from the environment and the passed in data map.
 // Config and defaults are supposed to be pointers to structs of the same type
-func ReadConfigMap(target interface{}, defaults interface{}, data map[string]string) {
+func ReadConfigMap(target interface{}, defaults interface{}, data map[string]string) { //nolint: gocognit
 	ensurePointerToCompatibleStruct("target", target, "default", defaults)
 
 	count := reflect.TypeOf(defaults).Elem().NumField()
@@ -89,6 +89,21 @@ func ReadConfigMap(target interface{}, defaults interface{}, data map[string]str
 		case reflect.Int:
 			value = fmt.Sprintf("%v", valueField.Int())
 
+		case reflect.Ptr:
+			// Handle pointer types - if not nil, get the underlying value
+			if !valueField.IsNil() {
+				switch valueField.Elem().Kind() {
+				case reflect.Int:
+					value = fmt.Sprintf("%v", valueField.Elem().Int())
+				default:
+					configparserLog.Info(
+						"Skipping unsupported pointer type while parsing default configuration",
+						"field", field.Name, "kind", valueField.Elem().Kind())
+					continue
+				}
+			}
+			// If nil, value stays empty, which means we'll only set it if env/data provides a value
+
 		case reflect.Slice:
 			if valueField.Type().Elem().Kind() != reflect.String {
 				configparserLog.Info(
@@ -129,6 +144,29 @@ func ReadConfigMap(target interface{}, defaults interface{}, data map[string]str
 				continue
 			}
 			reflect.ValueOf(target).Elem().FieldByName(field.Name).SetInt(intValue)
+		case reflect.Ptr:
+			// Handle pointer types
+			if value == "" {
+				// If no value is provided, leave the pointer as nil
+				continue
+			}
+			switch t.Elem().Kind() {
+			case reflect.Int:
+				intValue, err := strconv.ParseInt(value, 10, 0)
+				if err != nil {
+					configparserLog.Info(
+						"Skipping invalid integer pointer value parsing configuration",
+						"field", field.Name, "value", value)
+					continue
+				}
+				intVal := int(intValue)
+				reflect.ValueOf(target).Elem().FieldByName(field.Name).Set(reflect.ValueOf(&intVal))
+			default:
+				configparserLog.Info(
+					"Skipping unsupported pointer type while parsing configuration",
+					"field", field.Name, "kind", t.Elem().Kind())
+				continue
+			}
 		case reflect.String:
 			reflect.ValueOf(target).Elem().FieldByName(field.Name).SetString(value)
 		case reflect.Slice:
diff --git a/pkg/management/postgres/instance.go b/pkg/management/postgres/instance.go
index ff72d029..e082ae9e 100644
--- a/pkg/management/postgres/instance.go
+++ b/pkg/management/postgres/instance.go
@@ -1400,11 +1400,14 @@ func (instance *Instance) GetPrimaryConnInfo() string {
 	result := buildPrimaryConnInfo(instance.GetClusterName()+"-rw", instance.GetPodName()) + " dbname=postgres"
 
 	standbyTCPUserTimeout := os.Getenv("CNPG_STANDBY_TCP_USER_TIMEOUT")
-	if len(standbyTCPUserTimeout) > 0 {
-		result = fmt.Sprintf("%s tcp_user_timeout='%s'", result,
-			strings.ReplaceAll(strings.ReplaceAll(standbyTCPUserTimeout, `\`, `\\`), `'`, `\'`))
+	if len(standbyTCPUserTimeout) == 0 {
+		// Default to 5000ms (5 seconds) if not explicitly set
+		standbyTCPUserTimeout = "5000"
 	}
 
+	result = fmt.Sprintf("%s tcp_user_timeout='%s'", result,
+		strings.ReplaceAll(strings.ReplaceAll(standbyTCPUserTimeout, `\`, `\\`), `'`, `\'`))
+
 	return result
 }
 
diff --git a/pkg/management/postgres/restore.go b/pkg/management/postgres/restore.go
index affdeb61..bc7a7236 100644
--- a/pkg/management/postgres/restore.go
+++ b/pkg/management/postgres/restore.go
@@ -964,7 +964,18 @@ func (info InitInfo) ConfigureInstanceAfterRestore(ctx context.Context, cluster
 
 // GetPrimaryConnInfo returns the DSN to reach the primary
 func (info InitInfo) GetPrimaryConnInfo() string {
-	return buildPrimaryConnInfo(info.ClusterName+"-rw", info.PodName)
+	result := buildPrimaryConnInfo(info.ClusterName+"-rw", info.PodName) + " dbname=postgres"
+
+	standbyTCPUserTimeout := os.Getenv("CNPG_STANDBY_TCP_USER_TIMEOUT")
+	if len(standbyTCPUserTimeout) == 0 {
+		// Default to 5000ms (5 seconds) if not explicitly set
+		standbyTCPUserTimeout = "5000"
+	}
+
+	result = fmt.Sprintf("%s tcp_user_timeout='%s'", result,
+		strings.ReplaceAll(strings.ReplaceAll(standbyTCPUserTimeout, `\`, `\\`), `'`, `\'`))
+
+	return result
 }
 
 func (info *InitInfo) checkBackupDestination(
diff --git a/pkg/specs/pods.go b/pkg/specs/pods.go
index 57e5cdd1..3f82bbad 100644
--- a/pkg/specs/pods.go
+++ b/pkg/specs/pods.go
@@ -162,12 +162,12 @@ func CreatePodEnvConfig(cluster apiv1.Cluster, podName string) EnvConfig {
 	}
 	config.EnvVars = append(config.EnvVars, cluster.Spec.Env...)
 
-	if configuration.Current.StandbyTCPUserTimeout != 0 {
+	if configuration.Current.StandbyTCPUserTimeout != nil {
 		config.EnvVars = append(
 			config.EnvVars,
 			corev1.EnvVar{
 				Name:  "CNPG_STANDBY_TCP_USER_TIMEOUT",
-				Value: strconv.Itoa(configuration.Current.StandbyTCPUserTimeout),
+				Value: strconv.Itoa(*configuration.Current.StandbyTCPUserTimeout),
 			},
 		)
 	}



Test patch:
diff --git a/pkg/configparser/configparser_test.go b/pkg/configparser/configparser_test.go
index 709d7b3b..821f1c1c 100644
--- a/pkg/configparser/configparser_test.go
+++ b/pkg/configparser/configparser_test.go
@@ -44,6 +44,9 @@ type FakeData struct {
 
 	//  Threshold to consider a certificate as expiring
 	ExpiringCheckThreshold int `json:"expiringCheckThreshold" env:"EXPIRING_CHECK_THRESHOLD"`
+
+	// OptionalTimeout is an optional pointer to int for testing
+	OptionalTimeout *int `json:"optionalTimeout" env:"OPTIONAL_TIMEOUT"`
 }
 
 var defaultInheritedAnnotations = []string{
@@ -117,4 +120,55 @@ var _ = Describe("Data test suite", func() {
 		Expect(config.InheritedAnnotations).To(Equal(defaultInheritedAnnotations))
 		Expect(config.InheritedLabels).To(BeNil())
 	})
+
+	Context("pointer types", func() {
+		It("leaves pointer nil when no value is provided", func() {
+			GinkgoT().Setenv("OPTIONAL_TIMEOUT", "")
+			config := &FakeData{}
+			ReadConfigMap(config, &FakeData{}, nil)
+			Expect(config.OptionalTimeout).To(BeNil())
+		})
+
+		It("sets pointer from environment value", func() {
+			GinkgoT().Setenv("OPTIONAL_TIMEOUT", "5000")
+			config := &FakeData{}
+			ReadConfigMap(config, &FakeData{}, nil)
+			Expect(config.OptionalTimeout).ToNot(BeNil())
+			Expect(*config.OptionalTimeout).To(Equal(5000))
+		})
+
+		It("sets pointer from map value", func() {
+			GinkgoT().Setenv("OPTIONAL_TIMEOUT", "")
+			config := &FakeData{}
+			ReadConfigMap(config, &FakeData{}, map[string]string{
+				"OPTIONAL_TIMEOUT": "3000",
+			})
+			Expect(config.OptionalTimeout).ToNot(BeNil())
+			Expect(*config.OptionalTimeout).To(Equal(3000))
+		})
+
+		It("allows setting pointer to zero value explicitly", func() {
+			GinkgoT().Setenv("OPTIONAL_TIMEOUT", "0")
+			config := &FakeData{}
+			ReadConfigMap(config, &FakeData{}, nil)
+			Expect(config.OptionalTimeout).ToNot(BeNil())
+			Expect(*config.OptionalTimeout).To(Equal(0))
+		})
+
+		It("uses default pointer value when set", func() {
+			GinkgoT().Setenv("OPTIONAL_TIMEOUT", "")
+			config := &FakeData{}
+			defaultValue := 1000
+			ReadConfigMap(config, &FakeData{OptionalTimeout: &defaultValue}, nil)
+			Expect(config.OptionalTimeout).ToNot(BeNil())
+			Expect(*config.OptionalTimeout).To(Equal(1000))
+		})
+
+		It("skips invalid pointer value", func() {
+			GinkgoT().Setenv("OPTIONAL_TIMEOUT", "invalid")
+			config := &FakeData{}
+			ReadConfigMap(config, &FakeData{}, nil)
+			Expect(config.OptionalTimeout).To(BeNil())
+		})
+	})
 })
diff --git a/pkg/management/postgres/instance_test.go b/pkg/management/postgres/instance_test.go
index 5801cc3c..6ab80712 100644
--- a/pkg/management/postgres/instance_test.go
+++ b/pkg/management/postgres/instance_test.go
@@ -361,3 +361,58 @@ func getLibraryPathFromEnv(envs []string) string {
 
 	return ldLibraryPath
 }
+
+var _ = Describe("GetPrimaryConnInfo", func() {
+	var instance *Instance
+
+	BeforeEach(func() {
+		instance = &Instance{
+			Cluster: &apiv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+			},
+		}
+		instance.WithPodName("test-cluster-1").WithClusterName("test-cluster")
+	})
+
+	AfterEach(func() {
+		err := os.Unsetenv("CNPG_STANDBY_TCP_USER_TIMEOUT")
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("should use default 5000ms tcp_user_timeout when env var is not set", func() {
+		err := os.Unsetenv("CNPG_STANDBY_TCP_USER_TIMEOUT")
+		Expect(err).ToNot(HaveOccurred())
+		connInfo := instance.GetPrimaryConnInfo()
+		Expect(connInfo).To(ContainSubstring("tcp_user_timeout='5000'"))
+	})
+
+	It("should use custom tcp_user_timeout when env var is set", func() {
+		err := os.Setenv("CNPG_STANDBY_TCP_USER_TIMEOUT", "10000")
+		Expect(err).ToNot(HaveOccurred())
+		connInfo := instance.GetPrimaryConnInfo()
+		Expect(connInfo).To(ContainSubstring("tcp_user_timeout='10000'"))
+	})
+
+	It("should allow setting tcp_user_timeout to 0 explicitly", func() {
+		err := os.Setenv("CNPG_STANDBY_TCP_USER_TIMEOUT", "0")
+		Expect(err).ToNot(HaveOccurred())
+		connInfo := instance.GetPrimaryConnInfo()
+		Expect(connInfo).To(ContainSubstring("tcp_user_timeout='0'"))
+	})
+
+	It("should escape single quotes in tcp_user_timeout value", func() {
+		err := os.Setenv("CNPG_STANDBY_TCP_USER_TIMEOUT", "5000'injection")
+		Expect(err).ToNot(HaveOccurred())
+		connInfo := instance.GetPrimaryConnInfo()
+		Expect(connInfo).To(ContainSubstring("tcp_user_timeout='5000\\'injection'"))
+	})
+
+	It("should escape backslashes in tcp_user_timeout value", func() {
+		err := os.Setenv("CNPG_STANDBY_TCP_USER_TIMEOUT", "5000\\test")
+		Expect(err).ToNot(HaveOccurred())
+		connInfo := instance.GetPrimaryConnInfo()
+		Expect(connInfo).To(ContainSubstring("tcp_user_timeout='5000\\\\test'"))
+	})
+})