node: Derive cluster-size intervals from table

Project: cilium/cilium | Language: Go | Concepts: refactoring, data-structures | Difficulty: mid
Link: https://github.com/cilium/cilium/pull/48101

The problem

The codebase had several places where intervals (for syncs, polling, etc.) needed to scale with the cluster size (number of nodes). The calculation for these intervals was implemented as a method on the NodeManager, creating a tight coupling between unrelated components and the NodeManager. This made it hard to reuse the logic and complicated dependency injection, especially as the codebase moves away from the NodeManager.

Why it's non-trivial

A naive fix would be to just duplicate the interval calculation logic wherever needed, or to keep passing around the NodeManager just to access this one function. This would lead to code duplication, increased coupling, and make future refactoring harder. The challenge is to provide a reusable, decoupled way to compute cluster-size-dependent intervals, based on the actual number of nodes in the system.

The approach

The PR introduces a new function and type in the node package, decoupling interval calculation from NodeManager and basing it directly on the node table. Key code snippets:

// pkg/node/cluster_size.go
type ClusterSizeDependantIntervalFunc func(time.Duration) time.Duration

// NewClusterSizeDependantInterval returns a function that computes intervals
// from the current size of the node table.
func NewClusterSizeDependantInterval(
	db *statedb.DB,
	nodes statedb.Table[*Node],
) ClusterSizeDependantIntervalFunc {
	return func(baseInterval time.Duration) time.Duration {
		return backoff.ClusterSizeDependantInterval(
			baseInterval,
			nodes.NumObjects(db.ReadTxn()),
		)
	}
}

This defines a new function type and a constructor that returns a closure. The closure captures the database and node table, and computes the interval based on the current number of nodes.

-	ClusterSizeDependantInterval kvstore.ClusterSizeDependantIntervalFunc
+	ClusterSizeDependantInterval node.ClusterSizeDependantIntervalFunc

All usages are updated to use the new type from the node package, removing the redundant type from kvstore.

// pkg/node/cluster_size_test.go
func TestClusterSizeDependantInterval(t *testing.T) {
	db := statedb.New()
	nodes, err := NewNodeTable(db)
	require.NoError(t, err)
	interval := NewClusterSizeDependantInterval(db, nodes)
	base := time.Minute

	require.Equal(t, base, interval(base))

	txn := db.WriteTxn(nodes)
	for _, name := range []string{"node-1", "node-2", "node-3"} {
		_, _, err := nodes.Insert(txn, &Node{Node: nodeTypes.Node{Name: name}})
		require.NoError(t, err)
	}
	txn.Commit()

	require.Equal(
		t,
		backoff.ClusterSizeDependantInterval(base, 3),
		interval(base),
	)
}

A new unit test verifies that the interval function responds to the number of nodes in the table.

-	// ClusterSizeDependantInterval returns a time.Duration that is dependent on
-	// the cluster size, i.e. the number of nodes that have been discovered. This
-	// can be used to control sync intervals of shared or centralized resources to
-	// avoid overloading these resources as the cluster grows.
-	ClusterSizeDependantInterval(baseInterval time.Duration) time.Duration

The method is removed from the NodeManager interface, further decoupling interval logic from the manager.

From the review thread

MrFreezeex: This confused me a bit since it look like a function invocation but it's actually a cast 😅. I am wondering though, do we need to have two types from kvstore and node package or would it be possible to have just one?

joamaki: Oh yeah good point. We can just have one.

giorio94: Nit. The number of parameters of this function is getting insane, which is especially painful in tests. This may not be the best venue to address it though.

joamaki: Well I'm going to kill NodeManager pretty soon anyway...

Takeaways

• Prefer encapsulating logic as standalone functions or closures that depend only on minimal, relevant data (like a node table), rather than tying utility logic to large, unrelated objects.
• When multiple packages need the same behavior, define the function/type in a shared, appropriate package to avoid duplication and confusion.
• Decoupling utility functions from large manager objects makes future refactoring and testing easier.