Put existing work for an experimental backpressure feature under a feature flag #1055
Conversation
|
I propose we use the featureflag package for backpressure since it is going to be static and doesn't need to change once the osm-controller pod is running. The backpressure flag will need to be extended to other packages (catalog, cds) and passing one flag around is not a great idea. Right now its okay because the only backpressure code lives in |
Jont828
force-pushed
the
backpressure-flag
branch
from
fb2aa5e to
2f5291f
Compare
Wraps the experimental backpressure code using a feature flag. This is in preparation for the merge from experimental -> main.
shashankram
force-pushed
the
backpressure-flag
branch
from
2b4db74 to
69ea8dc
Compare
pkg/smi/client.go
Outdated
| if c.backpressureEnabled { | ||
| log.Info().Msgf("Backpressure turned off!") | ||
| return backpressureList | ||
| } |
There was a problem hiding this comment.
@shashankram I think this is a good guard against c.caches.Backpressure being nil.
@Jont828 this should be reversed though - return the empty list of backpressures when the feature is not enabled (like we said - to avoid looking into uninitialized caches)
There was a problem hiding this comment.
And we could also swap c.backpressureEnabled with featureflags.IsBackpressureEnabled()
There was a problem hiding this comment.
Actually should we log an error instead if ListBackpressures is called while backpressure is disabled?
pkg/smi/client.go
Outdated
| if c.backpressureEnabled { | ||
| log.Info().Msgf("Backpressure turned off!") | ||
| return backpressureList | ||
| } |
There was a problem hiding this comment.
@shashankram I think this is a good guard against c.caches.Backpressure being nil.
@Jont828 this should be reversed though - return the empty list of backpressures when the feature is not enabled (like we said - to avoid looking into uninitialized caches)
pkg/smi/client.go
Outdated
| if c.backpressureEnabled { | ||
| log.Info().Msgf("Backpressure turned off!") | ||
| return backpressureList | ||
| } |
There was a problem hiding this comment.
And we could also swap c.backpressureEnabled with featureflags.IsBackpressureEnabled()
|
ref #1376 |
Put the experimental work for backpressure so far behind a feature flag so the experimental changes can be merged safely to main