External Scaler: pooled gRPC connections are never released, leaking a connection and a goroutine per scaler address

Report

pkg/scalers/external_scaler.go keeps a package-level connectionPool sync.Map of grpc.ClientConn, keyed by a hash of the scaler address and TLS settings, so ScaledObjects that share connection properties share one connection.

Nothing ever releases an entry. Once every scaler using a pooled connection has been closed, the connection stays open, the pool entry stays, and a watcher goroutine stays blocked for the lifetime of the process.

The cleanup path cannot fire. getClientForConnectionPool starts this after storing a new connection:

go func() {
    // once gRPC client is shutdown, remove the connection from the pool and Close() grpc.ClientConn
    <-waitForState(context.TODO(), connGroup.grpcConnection, connectivity.Shutdown)
    connectionPoolMutex.Lock()
    defer connectionPoolMutex.Unlock()
    connectionPool.Delete(key)
    connGroup.grpcConnection.Close()
}()

It waits for the connection to reach connectivity.Shutdown and then closes it. A grpc.ClientConn reaches Shutdown only when Close() is called on it, and that line inside this goroutine is the only Close() of the pooled connection in the package. The trigger for the cleanup is the thing the cleanup performs, so it never runs.

externalScaler.Close(context.Context) error returns nil and releases nothing, so scaler teardown does not help either.

Expected Behavior

When the last scaler using a pooled connection is closed, the grpc.ClientConn is closed, the pool entry is removed, and the watcher goroutine exits.

Actual Behavior

The pool entry, the connection and the goroutine all survive. Measured on bf289bfad with a test that acquires a pooled connection for two scalers and then closes both:

pool=0 goroutines=4  (start)
pool=1 goroutines=8  state=IDLE  (two scalers hold the connection)
pool=1 goroutines=9  state=IDLE  (both scalers closed)

The pool still holds the entry, the connection is still IDLE where SHUTDOWN is expected, and the goroutine count does not come back down.

Per distinct scaler address and TLS combination ever configured, the operator retains one grpc.ClientConn, one sync.Map entry and at least one goroutine blocked in WaitForStateChange. ScaledObject churn, address changes and TLS changes each add an entry that is never reclaimed.

Steps to Reproduce the Problem

grpc.NewClient connects lazily, so this needs no external scaler running. Add the following to pkg/scalers and run it:

func TestPoolRelease(t *testing.T) {
	md := externalScalerMetadata{ScalerAddress: "probe.default.svc.cluster.local:9090"}
	if _, err := getClientForConnectionPool(md); err != nil {
		t.Fatal(err)
	}

	var cg *connectionGroup
	connectionPool.Range(func(_, v any) bool { cg, _ = v.(*connectionGroup); return false })

	s := &externalScaler{metadata: md}
	if err := s.Close(context.Background()); err != nil {
		t.Fatal(err)
	}
	time.Sleep(200 * time.Millisecond)

	n := 0
	connectionPool.Range(func(_, _ any) bool { n++; return true })
	t.Logf("pool entries after close: %d, connection state: %v", n, cg.grpcConnection.GetState())
}

It reports pool entries after close: 1, connection state: IDLE.

How this got here

f5508ebe5, "reduce gRPC connection create" (#3184, 20 Jun 2022), removed the reference counting that used to release these connections:

Worth saying that #3184 was fixing something real. The old count was taken per RPC call, so it dropped to zero at the end of every poll cycle, which closed the connection and forced a rebuild on the next one. Simply restoring that would bring the churn back. The count belongs at the scaler's lifetime instead: taken when the scaler is created, released when it is closed.

The doc comment on getClientForConnectionPool still describes the removed contract:

// getClientForConnectionPool returns a grpcClient and a done() Func. The done() function must be called once the client is no longer
// in use to clean up the shared grpc.ClientConn

The signature returns (pb.ExternalScalerClient, error). There is no done().

KEDA Version

2.18.0 (reproduced on main at bf289bfad)

Scaler Details

External Scaler / External Push Scaler (gRPC)

Anything else?

I have a fix ready that takes the reference at scaler construction and releases it in Close(), keeping the connection reuse #3184 introduced, together with tests that fail without it. Happy to open it, or to leave it if a maintainer would rather take this.