cache: do not allocate chan for nothing (#133500)

* cache: do not allocate chan for nothing

The explicit purpose of this channel is to never be closed and nothing
to be sent down on it. Hence, there's no need to allocate a channel - a
nil channel has exactly the desired behaviour.

Additionally, this is more relevant now that testing/synctest gets
unhappy when goroutines are blocked on reading on channels which are
created outside of the synctest bubble. Since this is a package var, that's
hard to avoid when using this package. Synctest is fine with nil
channels though.

Reported-by: Jussi Maki <jussi@isovalent.com>
Signed-off-by: David Bimmler <david.bimmler@isovalent.com>

* handlers: do not allocate chan for nothing

Nil chan has the desired semantics already, and this breaks
testing/synctest because the channel is allocated outside of the bubble.

Signed-off-by: David Bimmler <david.bimmler@isovalent.com>

---------

Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
This commit is contained in:
David Bimmler
2025-08-28 03:30:16 +02:00
committed by GitHub
parent f1ceec70a6
commit 52b7d035f9
2 changed files with 7 additions and 13 deletions

View File

@@ -39,9 +39,6 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
)
// nothing will ever be sent down this channel
var neverExitWatch <-chan time.Time = make(chan time.Time)
// timeoutFactory abstracts watch timeout logic for testing
type TimeoutFactory interface {
TimeoutCh() (<-chan time.Time, func() bool)
@@ -56,7 +53,8 @@ type realTimeoutFactory struct {
// and a cleanup function to call when this happens.
func (w *realTimeoutFactory) TimeoutCh() (<-chan time.Time, func() bool) {
if w.timeout == 0 {
return neverExitWatch, func() bool { return false }
// nothing will ever be sent down this channel
return nil, func() bool { return false }
}
t := time.NewTimer(w.timeout)
return t.C, t.Stop

View File

@@ -49,11 +49,9 @@ import (
const defaultExpectedTypeName = "<unspecified>"
var (
// We try to spread the load on apiserver by setting timeouts for
// watch requests - it is random in [minWatchTimeout, 2*minWatchTimeout].
defaultMinWatchTimeout = 5 * time.Minute
)
// We try to spread the load on apiserver by setting timeouts for
// watch requests - it is random in [minWatchTimeout, 2*minWatchTimeout].
var defaultMinWatchTimeout = 5 * time.Minute
// ReflectorStore is the subset of cache.Store that the reflector uses
type ReflectorStore interface {
@@ -365,9 +363,6 @@ func (r *Reflector) RunWithContext(ctx context.Context) {
}
var (
// nothing will ever be sent down this channel
neverExitWatch <-chan time.Time = make(chan time.Time)
// Used to indicate that watching stopped because of a signal from the stop
// channel passed in from a client of the reflector.
errorStopRequested = errors.New("stop requested")
@@ -377,7 +372,8 @@ var (
// required, and a cleanup function.
func (r *Reflector) resyncChan() (<-chan time.Time, func() bool) {
if r.resyncPeriod == 0 {
return neverExitWatch, func() bool { return false }
// nothing will ever be sent down this channel
return nil, func() bool { return false }
}
// The cleanup function is required: imagine the scenario where watches
// always fail so we end up listing frequently. Then, if we don't