Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions config/core/configmaps/gc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ metadata:
labels:
serving.knative.dev/release: devel
annotations:
knative.dev/example-checksum: "83219cc3"
knative.dev/example-checksum: "c675b489"
data:
_example: |
################################
Expand All @@ -47,10 +47,40 @@ data:
# resync period (10 hours).
stale-revision-timeout: "15h"

# Minimum number of generations of revisions to keep before considering
# them for GC
# Minimum number of generations of non-active revisions to keep before
# considering them for GC.
stale-revision-minimum-generations: "20"

# To avoid constant updates, we allow an existing annotation to be stale by this
# amount before we update the timestamp.
stale-revision-lastpinned-debounce: "5h"


# ---------------------------------------
# V2 Garbage Collector Settings
# ---------------------------------------
#
# These settings are enabled via the 'responsive-revision-gc' feature flag.
# ALPHA NOTE: This feature is still experimental and under active development.

# Duration since revision creation before considering it for GC.
retain-since-create-time: "48h"

# Duration since a route has pointed at the revision
# before considering it for GC.
retain-since-last-active-time: "15h"

# Minimum number of non-active revisions to retain.
min-stale-revisions: "20"

# Maximum number of non-active revisions to retain regardless of
# creation or staleness time-bounds.
Comment thread
whaught marked this conversation as resolved.
#
# Instead of allowing revisions to become stale down to min-stale-revisions,
# this mode will keep around revisions up to this maximum limit. A value of 0
# will immediately delete any non-active revisions. The expected total number
# of revisions is the number of active plus this value.
#
# Set to -1 to disable this setting. Revisions will be eligible for gc as soon
# as they are considered stale as long as there are more than min-stale-revisions.
max-stale-revisions: "-1"
46 changes: 43 additions & 3 deletions pkg/gc/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,41 @@ type Config struct {
// Timeout since a revision lastPinned before it should be GC'd
// This must be longer than the controller resync period
StaleRevisionTimeout time.Duration
// Minimum number of generations of revisions to keep before considering for GC
// Minimum number of generations of revisions to keep before considering for GC.
StaleRevisionMinimumGenerations int64
// Minimum staleness duration before updating lastPinned
StaleRevisionLastpinnedDebounce time.Duration

// Duration from creation when a Revision should be considered active
// and exempt from GC. Note that GCMaxStaleRevision may override this if set.
RetainSinceCreateTime time.Duration
// Duration from last active when a Revision should be considered active
// and exempt from GC.Note that GCMaxStaleRevision may override this if set.
RetainSinceLastActiveTime time.Duration
// Minimum number of generations of revisions to keep before considering for GC.
// Set -1 to disable minimum and fill up to max.
// Either min or max must be set.
MinStaleRevisions int64
// Maximum number of stale revisions to keep before considering for GC.
// regardless of creation or staleness time-bounds
// Set -1 to disable this setting.
// Either min or max must be set.
MaxStaleRevisions int64
}

func defaultConfig() *Config {
return &Config{
// V1 GC Settings
StaleRevisionCreateDelay: 48 * time.Hour,
StaleRevisionTimeout: 15 * time.Hour,
StaleRevisionLastpinnedDebounce: 5 * time.Hour,
StaleRevisionMinimumGenerations: 20,

// V2 GC Settings
RetainSinceCreateTime: 48 * time.Hour,
RetainSinceLastActiveTime: 15 * time.Hour,
MinStaleRevisions: 20,
MaxStaleRevisions: -1,
}
}

Expand All @@ -62,18 +85,35 @@ func NewConfigFromConfigMapFunc(ctx context.Context) func(configMap *corev1.Conf
cm.AsDuration("stale-revision-create-delay", &c.StaleRevisionCreateDelay),
cm.AsDuration("stale-revision-timeout", &c.StaleRevisionTimeout),
cm.AsDuration("stale-revision-lastpinned-debounce", &c.StaleRevisionLastpinnedDebounce),

cm.AsInt64("stale-revision-minimum-generations", &c.StaleRevisionMinimumGenerations),

cm.AsDuration("retain-since-create-time", &c.RetainSinceCreateTime),
cm.AsDuration("retain-since-last-active-time", &c.RetainSinceLastActiveTime),
cm.AsInt64("min-stale-revisions", &c.MinStaleRevisions),
cm.AsInt64("max-stale-revisions", &c.MaxStaleRevisions),
); err != nil {
return nil, fmt.Errorf("failed to parse data: %w", err)
}

if c.MaxStaleRevisions >= 0 && c.MinStaleRevisions > c.MaxStaleRevisions {
return nil, fmt.Errorf(
"min-stale-revisions(%d) must be <= max-stale-revisions(%d)",
c.MinStaleRevisions, c.MaxStaleRevisions)
}
if c.MinStaleRevisions < 0 {
return nil, fmt.Errorf("min-stale-revisions must be non-negative, was: %d", c.MinStaleRevisions)
}
if c.MaxStaleRevisions < -1 {
return nil, fmt.Errorf("max-stale-revisions must be >= -1, was: %d", c.MaxStaleRevisions)
}

if c.StaleRevisionMinimumGenerations < 0 {
return nil, fmt.Errorf("stale-revision-minimum-generations must be non-negative, was: %d", c.StaleRevisionMinimumGenerations)
}

if c.StaleRevisionTimeout-c.StaleRevisionLastpinnedDebounce < minRevisionTimeout {
logger.Warnf("Got revision timeout of %v, minimum supported value is %v", c.StaleRevisionTimeout, minRevisionTimeout+c.StaleRevisionLastpinnedDebounce)
logger.Warnf("Got revision timeout of %v, minimum supported value is %v",
c.StaleRevisionTimeout, minRevisionTimeout+c.StaleRevisionLastpinnedDebounce)
c.StaleRevisionTimeout = minRevisionTimeout + c.StaleRevisionLastpinnedDebounce
}
return c, nil
Expand Down
41 changes: 37 additions & 4 deletions pkg/gc/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,20 @@ func TestOurConfig(t *testing.T) {
StaleRevisionTimeout: 14 * time.Hour,
StaleRevisionMinimumGenerations: 10,
StaleRevisionLastpinnedDebounce: 2*time.Hour + 30*time.Minute + 44*time.Second,
RetainSinceCreateTime: 17 * time.Hour,
RetainSinceLastActiveTime: 16 * time.Hour,
MinStaleRevisions: 5,
MaxStaleRevisions: 500,
},
data: map[string]string{
"stale-revision-create-delay": "15h",
"stale-revision-timeout": "14h",
"stale-revision-minimum-generations": "10",
"stale-revision-lastpinned-debounce": "2h30m44s",
"retain-since-create-time": "17h",
"retain-since-last-active-time": "16h",
"min-stale-revisions": "5",
"max-stale-revisions": "500",
},
}, {
name: "Invalid duration",
Expand All @@ -72,6 +80,28 @@ func TestOurConfig(t *testing.T) {
data: map[string]string{
"stale-revision-minimum-generations": "-1",
},
}, {
name: "Invalid negative minimum generation v2",
fail: true,
want: nil,
data: map[string]string{
"min-stale-revisions": "-1",
},
}, {
name: "Invalid negative maximum generation",
fail: true,
want: nil,
data: map[string]string{
"max-stale-revisions": "-2",
},
}, {
name: "Invalid max less than min",
fail: true,
want: nil,
data: map[string]string{
"min-stale-revisions": "20",
"max-stale-revisions": "10",
},
}, {
name: "Invalid minimum generation",
fail: true,
Expand All @@ -85,13 +115,16 @@ func TestOurConfig(t *testing.T) {
want: &Config{
StaleRevisionCreateDelay: 15 * time.Hour,
StaleRevisionTimeout: 15 * time.Hour,
StaleRevisionMinimumGenerations: 10,
StaleRevisionMinimumGenerations: 20,
StaleRevisionLastpinnedDebounce: 5 * time.Hour,
RetainSinceCreateTime: 48 * time.Hour,
RetainSinceLastActiveTime: 15 * time.Hour,
MinStaleRevisions: 20,
MaxStaleRevisions: -1,
},
data: map[string]string{
"stale-revision-create-delay": "15h",
"stale-revision-minimum-generations": "10",
"stale-revision-timeout": "1h",
"stale-revision-create-delay": "15h",
"stale-revision-timeout": "1h",
},
}} {
t.Run(tt.name, func(t *testing.T) {
Expand Down