Skip to content

Commit ae20fed

Browse files
committed
Make keepalive parameters configurable via environment variables
Address review feedback: keepalive timings are now configurable through environment variables instead of being hardcoded. Environment variables: - GNMI_CLIENT_KEEPALIVE_INTERVAL: Client ping interval in seconds (default: 30) - GNMI_CLIENT_KEEPALIVE_TIMEOUT: Client ping ack timeout in seconds (default: 10) - GNMI_SERVER_KEEPALIVE_MIN_TIME: Server minimum allowed ping interval in seconds (default: 20) Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
1 parent 54942e6 commit ae20fed

2 files changed

Lines changed: 25 additions & 5 deletions

File tree

pkg/interceptors/dpuproxy/proxy.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"os"
8+
"strconv"
79
"sync"
810
"time"
911

@@ -77,6 +79,17 @@ var defaultForwardableMethods = []ForwardableMethod{
7779

7880
var defaultProxy *DPUProxy
7981

82+
// getEnvDuration reads an environment variable as a duration in seconds.
83+
// Returns the fallback if the variable is unset or invalid.
84+
func getEnvDuration(key string, fallback time.Duration) time.Duration {
85+
if v := os.Getenv(key); v != "" {
86+
if secs, err := strconv.Atoi(v); err == nil && secs > 0 {
87+
return time.Duration(secs) * time.Second
88+
}
89+
}
90+
return fallback
91+
}
92+
8093
// SetDefaultProxy registers the DPU proxy singleton for use by handlers
8194
// that need direct DPU connections (e.g., TransferToRemote file operations).
8295
func SetDefaultProxy(p *DPUProxy) { defaultProxy = p }
@@ -187,9 +200,9 @@ func (p *DPUProxy) getConnection(ctx context.Context, dpuIndex, ipAddress string
187200
target,
188201
grpc.WithTransportCredentials(insecure.NewCredentials()),
189202
grpc.WithKeepaliveParams(keepalive.ClientParameters{
190-
Time: 30 * time.Second, // Send keepalive ping every 30s
191-
Timeout: 10 * time.Second, // Wait 10s for ping ack before considering connection dead
192-
PermitWithoutStream: true, // Send pings even when no active RPCs
203+
Time: getEnvDuration("GNMI_CLIENT_KEEPALIVE_INTERVAL", 30*time.Second),
204+
Timeout: getEnvDuration("GNMI_CLIENT_KEEPALIVE_TIMEOUT", 10*time.Second),
205+
PermitWithoutStream: true,
193206
}),
194207
)
195208

telemetry/telemetry.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"os"
1313
"os/signal"
1414
"path/filepath"
15+
"strconv"
1516
"strings"
1617
"sync"
1718
"sync/atomic"
@@ -532,9 +533,15 @@ func startGNMIServer(telemetryCfg *TelemetryConfig, cfg *gnmi.Config, serverCont
532533
// causing "too_many_pings" GOAWAY for clients that ping more
533534
// frequently during long-running operations like SetPackage.
534535
// See: https://github.com/sonic-net/sonic-gnmi/issues/619
536+
serverMinPingTime := 20 * time.Second
537+
if v := os.Getenv("GNMI_SERVER_KEEPALIVE_MIN_TIME"); v != "" {
538+
if secs, err := strconv.Atoi(v); err == nil && secs > 0 {
539+
serverMinPingTime = time.Duration(secs) * time.Second
540+
}
541+
}
535542
keep_alive_policy := keepalive.EnforcementPolicy{
536-
MinTime: 20 * time.Second, // Allow pings as frequent as every 20s
537-
PermitWithoutStream: true, // Allow pings when there are no active streams
543+
MinTime: serverMinPingTime,
544+
PermitWithoutStream: true,
538545
}
539546
commonOpts = append(commonOpts, grpc.KeepaliveEnforcementPolicy(keep_alive_policy))
540547

0 commit comments

Comments
 (0)