From fca9d828e8689eee9d10e130be928d4a24c7b550 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 25 Feb 2025 13:49:51 -0800 Subject: [PATCH] Limit max age of Praefect connections to 15 minutes By default, gRPC servers persist HTTP/2 connections indefinitely. However, load balancing doesn't work effectively if clients don't rotate connections between different Praefect servers. To prevent hotspots, limit the connection age to 15 minutes by default. This commit introduces two configuration parameters for Praefect: * max_connection_age (default: 15 minutes) * max_connection_age_grace (default: infinity) Relates to https://gitlab.com/gitlab-com/request-for-help/-/issues/2383 Changelog: fixed --- config.praefect.toml.example | 7 +++++++ internal/praefect/config/config.go | 15 +++++++++++++++ internal/praefect/config/config_test.go | 6 ++++++ internal/praefect/server.go | 4 +++- 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/config.praefect.toml.example b/config.praefect.toml.example index 99b83fdba1..5f29ad5fae 100644 --- a/config.praefect.toml.example +++ b/config.praefect.toml.example @@ -17,6 +17,13 @@ listen_addr = "127.0.0.1:2305" # # Optional: export metrics via Prometheus # prometheus_listen_addr = "127.0.01:10101" # # You can optionally configure Praefect to output JSON-formatted log messages to stdout +# # Optional: The maximum amount of time a server connection may exist before it will be shut down. +# # Defaults to "15m" +# max_connection_age = "15m" +# # Optional: The maximum time the connection will be kept alive for outstanding RPCs to complete. +# # Defaults to infinity +# max_connection_age_grace = "" + # [logging] # format = "json" # # Optional: Set log level to only log entries with that severity or above diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go index 3a4621d40b..2fb7789c36 100644 --- a/internal/praefect/config/config.go +++ b/internal/praefect/config/config.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "math" "os" "os/exec" "sort" @@ -46,6 +47,10 @@ const ( minimalSyncCheckInterval = time.Minute minimalSyncRunInterval = time.Minute + + infinity = time.Duration(math.MaxInt64) + defaultMaxConnectionAge = 15 * time.Minute + defaultMaxConnectionAgeGrace = infinity ) // Failover contains configuration for the mechanism that tracks healthiness of the cluster nodes. @@ -213,6 +218,8 @@ type Config struct { ListenAddr string `json:"listen_addr" toml:"listen_addr,omitempty"` TLSListenAddr string `json:"tls_listen_addr" toml:"tls_listen_addr,omitempty"` SocketPath string `json:"socket_path" toml:"socket_path,omitempty"` + MaxConnectionAge duration.Duration `json:"max_connection_age" toml:"max_connection_age,omitempty"` + MaxConnectionAgeGrace duration.Duration `json:"max_connection_age_grace" toml:"max_connection_age_grace,omitempty"` VirtualStorages []*VirtualStorage `json:"virtual_storage" toml:"virtual_storage,omitempty"` Logging log.Config `json:"logging" toml:"logging,omitempty"` Sentry sentry.Config `json:"sentry" toml:"sentry,omitempty"` @@ -470,6 +477,14 @@ func (c *Config) NeedsSQL() bool { } func (c *Config) setDefaults() { + if c.MaxConnectionAge.Duration() == 0 { + c.MaxConnectionAge = duration.Duration(defaultMaxConnectionAge) + } + + if c.MaxConnectionAgeGrace.Duration() == 0 { + c.MaxConnectionAgeGrace = duration.Duration(defaultMaxConnectionAgeGrace) + } + if c.GracefulStopTimeout.Duration() == 0 { c.GracefulStopTimeout = duration.Duration(time.Minute) } diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 6a16fb8fd8..88e11cbfc5 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -271,6 +271,8 @@ func TestConfigParsing(t *testing.T) { CertPath: "/home/git/cert.cert", KeyPath: "/home/git/key.pem", }, + MaxConnectionAge: duration.Duration(defaultMaxConnectionAge), + MaxConnectionAgeGrace: duration.Duration(infinity), Logging: log.Config{ Level: "info", Format: "json", @@ -381,6 +383,8 @@ func TestConfigParsing(t *testing.T) { BackgroundVerification: DefaultBackgroundVerificationConfig(), Yamux: DefaultYamuxConfig(), Logging: DefaultLoggingConfig(), + MaxConnectionAge: duration.Duration(defaultMaxConnectionAge), + MaxConnectionAgeGrace: duration.Duration(infinity), }, }, { @@ -405,6 +409,8 @@ func TestConfigParsing(t *testing.T) { BackgroundVerification: DefaultBackgroundVerificationConfig(), Yamux: DefaultYamuxConfig(), Logging: DefaultLoggingConfig(), + MaxConnectionAge: duration.Duration(defaultMaxConnectionAge), + MaxConnectionAgeGrace: duration.Duration(infinity), }, }, { diff --git a/internal/praefect/server.go b/internal/praefect/server.go index 766efebd70..9807db442b 100644 --- a/internal/praefect/server.go +++ b/internal/praefect/server.go @@ -166,7 +166,9 @@ func NewGRPCServer( PermitWithoutStream: true, }), grpc.KeepaliveParams(keepalive.ServerParameters{ - Time: 5 * time.Minute, + Time: 5 * time.Minute, + MaxConnectionAge: deps.Config.MaxConnectionAge.Duration(), + MaxConnectionAgeGrace: deps.Config.MaxConnectionAgeGrace.Duration(), }), grpc.WaitForHandlers(true), }...) -- GitLab