Skip to content

Commit 27f3504

Browse files
fix: prevent memory leak by reusing gRPC conn to kubelet pod-resources API
- Add persistent grpcConn to PodMapper, reused across scrapes instead of creating a new *grpc.ClientConn on every /metrics request (issue #702) - Reset cached conn on RPC error to handle kubelet restarts - Move resolver.SetDefaultScheme to one-time init in NewPodMapper - Clone labels map per entity in expCollector and gpuHealthStatusCollector to prevent cross-GPU label pollution and extra GC pressure - Add tests: connection reuse, reconnect after shutdown, Stop() cleanup
1 parent 181290c commit 27f3504

4 files changed

Lines changed: 151 additions & 5 deletions

File tree

internal/pkg/collector/expcollector.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,9 @@ func (c *expCollector) getMetrics() (MetricsByCounter, error) {
102102
entityValues, exists := mapEntityIDToValues[mi.Entity]
103103
if exists {
104104
for entityValue, val := range entityValues {
105-
106105
metricValueLabels := maps.Clone(labels)
107106
c.labelFiller(metricValueLabels, entityValue)
108-
109107
m := c.createMetric(metricValueLabels, mi, uuid, val)
110-
111108
metrics[c.counter] = append(metrics[c.counter], m)
112109
}
113110
} else {

internal/pkg/transformation/kubernetes.go

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"google.golang.org/grpc/status"
3636

3737
"google.golang.org/grpc"
38+
"google.golang.org/grpc/connectivity"
3839
"google.golang.org/grpc/credentials/insecure"
3940

4041
"github.com/NVIDIA/go-dcgm/pkg/dcgm"
@@ -354,6 +355,37 @@ func (p *PodMapper) Run() {
354355

355356
func (p *PodMapper) Stop() {
356357
close(p.stopChan)
358+
p.grpcConnMu.Lock()
359+
defer p.grpcConnMu.Unlock()
360+
if p.grpcConn != nil {
361+
p.grpcConn.Close()
362+
p.grpcConn = nil
363+
}
364+
}
365+
366+
// getGRPCConn returns the cached gRPC connection to the kubelet pod-resources socket,
367+
// creating a new one if the cached connection is absent or has shut down.
368+
// Reusing the connection across scrapes is the primary fix for the RSS growth in issue #702:
369+
// each grpc.NewClient call allocates HTTP/2 frame buffers and spawns goroutines whose
370+
// cleanup is asynchronous, so creating one per scrape causes steady heap growth.
371+
func (p *PodMapper) getGRPCConn(socketPath string) (*grpc.ClientConn, error) {
372+
p.grpcConnMu.Lock()
373+
defer p.grpcConnMu.Unlock()
374+
375+
if p.grpcConn != nil {
376+
if state := p.grpcConn.GetState(); state != connectivity.Shutdown {
377+
return p.grpcConn, nil
378+
}
379+
p.grpcConn.Close()
380+
p.grpcConn = nil
381+
}
382+
383+
conn, _, err := connectToServer(socketPath)
384+
if err != nil {
385+
return nil, err
386+
}
387+
p.grpcConn = conn
388+
return conn, nil
357389
}
358390

359391
func (p *PodMapper) getMappings(deviceInfo deviceinfo.Provider) (map[string][]PodInfo, map[string]PodInfo, map[string][]PodInfo, error) {
@@ -365,14 +397,21 @@ func (p *PodMapper) getMappings(deviceInfo deviceinfo.Provider) (map[string][]Po
365397
return nil, nil, nil, err
366398
}
367399

368-
c, cleanup, err := connectToServer(socketPath)
400+
c, err := p.getGRPCConn(socketPath)
369401
if err != nil {
370402
return nil, nil, nil, err
371403
}
372-
defer cleanup()
373404

374405
pods, err := p.listPods(c)
375406
if err != nil {
407+
// Reset the cached connection on RPC failure so the next scrape
408+
// establishes a fresh one (e.g., after kubelet restart).
409+
p.grpcConnMu.Lock()
410+
if p.grpcConn != nil {
411+
p.grpcConn.Close()
412+
p.grpcConn = nil
413+
}
414+
p.grpcConnMu.Unlock()
376415
return nil, nil, nil, err
377416
}
378417

internal/pkg/transformation/kubernetes_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"go.uber.org/mock/gomock"
3838
"google.golang.org/grpc"
3939
"google.golang.org/grpc/codes"
40+
"google.golang.org/grpc/connectivity"
4041
"google.golang.org/grpc/status"
4142
v1 "k8s.io/api/core/v1"
4243
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -2678,3 +2679,106 @@ func TestProcessKeepsGenericWarningOnNonResourceExhaustedPodResourcesError(t *te
26782679
require.Contains(t, gotLog, "Failed to get pod mappings")
26792680
require.NotContains(t, gotLog, "Kubelet pod-resources response exceeded gRPC receive limit")
26802681
}
2682+
2683+
// TestGetGRPCConn_ReusesConnection verifies that getGRPCConn returns the same
2684+
// *grpc.ClientConn on successive calls, avoiding the per-scrape allocation
2685+
// overhead that caused the RSS growth in issue #702.
2686+
func TestGetGRPCConn_ReusesConnection(t *testing.T) {
2687+
testutils.RequireLinux(t)
2688+
2689+
tmpDir, cleanupDir := testutils.CreateTmpDir(t)
2690+
defer cleanupDir()
2691+
2692+
socketPath := filepath.Join(tmpDir, "kubelet.sock")
2693+
server := grpc.NewServer()
2694+
podresourcesapi.RegisterPodResourcesListerServer(server,
2695+
testutils.NewMockPodResourcesServer(appconfig.NvidiaResourceName, []string{"gpu-0"}))
2696+
stopServer := testutils.StartMockServer(t, server, socketPath)
2697+
defer stopServer()
2698+
2699+
pm := &PodMapper{
2700+
Config: &appconfig.Config{PodResourcesKubeletSocket: socketPath},
2701+
stopChan: make(chan struct{}),
2702+
}
2703+
2704+
conn1, err := pm.getGRPCConn(socketPath)
2705+
require.NoError(t, err)
2706+
require.NotNil(t, conn1)
2707+
2708+
conn2, err := pm.getGRPCConn(socketPath)
2709+
require.NoError(t, err)
2710+
require.NotNil(t, conn2)
2711+
2712+
assert.Same(t, conn1, conn2, "getGRPCConn must return the cached connection on repeated calls")
2713+
}
2714+
2715+
// TestGetGRPCConn_ReconnectsAfterShutdown verifies that getGRPCConn creates a
2716+
// fresh connection when the cached one has been shut down (e.g., after a kubelet
2717+
// restart that invalidates the Unix socket).
2718+
func TestGetGRPCConn_ReconnectsAfterShutdown(t *testing.T) {
2719+
testutils.RequireLinux(t)
2720+
2721+
tmpDir, cleanupDir := testutils.CreateTmpDir(t)
2722+
defer cleanupDir()
2723+
2724+
socketPath := filepath.Join(tmpDir, "kubelet.sock")
2725+
server := grpc.NewServer()
2726+
podresourcesapi.RegisterPodResourcesListerServer(server,
2727+
testutils.NewMockPodResourcesServer(appconfig.NvidiaResourceName, []string{"gpu-0"}))
2728+
stopServer := testutils.StartMockServer(t, server, socketPath)
2729+
defer stopServer()
2730+
2731+
pm := &PodMapper{
2732+
Config: &appconfig.Config{PodResourcesKubeletSocket: socketPath},
2733+
stopChan: make(chan struct{}),
2734+
}
2735+
2736+
conn1, err := pm.getGRPCConn(socketPath)
2737+
require.NoError(t, err)
2738+
require.NotNil(t, conn1)
2739+
2740+
// Forcibly shut down the cached connection to simulate a broken link.
2741+
conn1.Close()
2742+
require.Eventually(t, func() bool {
2743+
return conn1.GetState() == connectivity.Shutdown
2744+
}, 2*time.Second, 10*time.Millisecond, "connection should reach Shutdown state after Close()")
2745+
2746+
conn2, err := pm.getGRPCConn(socketPath)
2747+
require.NoError(t, err)
2748+
require.NotNil(t, conn2)
2749+
2750+
assert.NotSame(t, conn1, conn2, "getGRPCConn should allocate a new connection after the cached one shuts down")
2751+
}
2752+
2753+
// TestPodMapper_Stop_ClosesGRPCConn verifies that Stop() closes the persistent
2754+
// gRPC connection and clears the cached pointer.
2755+
func TestPodMapper_Stop_ClosesGRPCConn(t *testing.T) {
2756+
testutils.RequireLinux(t)
2757+
2758+
tmpDir, cleanupDir := testutils.CreateTmpDir(t)
2759+
defer cleanupDir()
2760+
2761+
socketPath := filepath.Join(tmpDir, "kubelet.sock")
2762+
server := grpc.NewServer()
2763+
podresourcesapi.RegisterPodResourcesListerServer(server,
2764+
testutils.NewMockPodResourcesServer(appconfig.NvidiaResourceName, []string{"gpu-0"}))
2765+
stopServer := testutils.StartMockServer(t, server, socketPath)
2766+
defer stopServer()
2767+
2768+
pm := &PodMapper{
2769+
Config: &appconfig.Config{PodResourcesKubeletSocket: socketPath},
2770+
stopChan: make(chan struct{}),
2771+
}
2772+
2773+
conn, err := pm.getGRPCConn(socketPath)
2774+
require.NoError(t, err)
2775+
require.NotNil(t, conn)
2776+
2777+
pm.Stop()
2778+
2779+
assert.Eventually(t, func() bool {
2780+
return conn.GetState() == connectivity.Shutdown
2781+
}, 2*time.Second, 10*time.Millisecond, "gRPC connection should be shut down after Stop()")
2782+
2783+
assert.Nil(t, pm.grpcConn, "grpcConn field should be nil after Stop()")
2784+
}

internal/pkg/transformation/types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"regexp"
2323
"sync"
2424

25+
"google.golang.org/grpc"
2526
"k8s.io/client-go/informers"
2627
"k8s.io/client-go/kubernetes"
2728
corev1listers "k8s.io/client-go/listers/core/v1"
@@ -48,6 +49,11 @@ type PodMapper struct {
4849
podLister corev1listers.PodLister
4950
podInformerSynced cache.InformerSynced
5051
stopChan chan struct{}
52+
// grpcConn is a persistent connection to the kubelet pod-resources endpoint,
53+
// reused across scrapes to avoid the per-scrape allocation/goroutine overhead
54+
// of grpc.NewClient that causes the slow RSS growth reported in issue #702.
55+
grpcConn *grpc.ClientConn
56+
grpcConnMu sync.Mutex
5157
}
5258

5359
// LabelFilterCache provides efficient caching for label filtering decisions

0 commit comments

Comments
 (0)