Oom leaks rollout fallback by jkremser · Pull Request #7 · kedify/recommender

jkremser · 2026-09-24T17:37:41Z

No description provided.

Use one hour of measured history and distinct observation timestamps, with coverage guards that tolerate collection gaps. Expose shared sample normalization and versioned decision traces. Fall back to qualifying usage from up to three earlier rollouts while retaining current identity, allocation, and inventory guards. Initialize known-unset settings without material-change thresholds. Update the output schema, fixtures, regression tests, and caller guidance.

Normalize and deduplicate release-scoped OOM events, and use failed limits or explicit fallback allocations to establish a memory request floor. Allow justified growth without usage history while retaining identity, freshness, bounds, and allocation consistency guards. Report OOM evidence and sizing adjustments in results and decision traces. Cover interactions with rollout fallback and unset settings, update the policy fixture, and document event normalization for adapters.

Detect sustained growth in bucketed memory baselines using bounded robust trend analysis, with warmup, coverage, freshness, and recent-growth guards. Keep replicas and container lifetimes separate and correlate OOM events with individual episodes. Expose normalized policy and advisory findings without changing resource recommendations. Add regression coverage for growth, plateaus, recovery, insufficient evidence, identity boundaries, and deterministic output. Document configuration, evidence, and interpretation limits.

Copilot

Copilot review overview

🟡 Changes recommended

Moderate correctness issues remain in trace reasons, leak coverage accounting, and OOM validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 6

Open (7)

Use current entry reasons in request decision traces · New
Add required license header to Go source file · New
Add required license header to Go test file · New
Add required license header to Go source file · New
Add required license header to Go source file · New
Add required copyright/SPDX header to Go test file · New
Add required copyright/SPDX header to Go test file · New

What changed in this PR

Adds OOM-aware sizing, rollout fallback, unset-setting handling, decision traces, sample normalization, and optional memory-leak diagnostics.

Changes:

Adds OOM evidence and guarded memory adjustments.
Implements rollout fallback, unset handling, and leak detection.
Updates schemas, documentation, normalization, and tests.

File	Summary
README.md	Documents new analysis capabilities and contracts.
analysis/unset_test.go	Tests unset allocation handling. Nit: add the standard header.
analysis/types.go	Adds input, policy, evidence, and output types.
analysis/trace.go	Defines decision-trace structures. Nit: add the standard header.
analysis/trace_test.go	Tests decision traces and normalization. Nit: add the standard header.
analysis/testdata/default-output.json	Updates expected output schema and trace data.
analysis/samples.go	Normalizes raw samples. Nit: add the standard header.
analysis/oom.go	Implements OOM selection and adjustments. Moderate: validate and deduplicate all observations before time-window filtering.
analysis/oom_test.go	Tests OOM behavior and validation.
analysis/memory_leak.go	Implements leak detection. Moderate: account for empty or stale series so missing coverage does not produce a false no-leak result.
analysis/memory_leak_types.go	Defines leak policies and result types.
analysis/memory_leak_test.go	Tests leak detection behavior.
analysis/fallback.go	Implements previous-rollout usage fallback. Nit: add the standard header.
analysis/fallback_test.go	Tests rollout fallback behavior. Nit: add the standard header.
analysis/analyze.go	Integrates fallback, OOM, unset, and trace logic. Moderate: use per-entry missing/stale reasons in the request trace.
analysis/analyze_test.go	Tests sizing, normalization, and analysis behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical fallback and public API compatibility issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Treat zero release boundary as absent when capping fallback time · New
Preserve compatibility for removed MaximumGapSeconds field · New

Resolved since last review (7)

Use current entry reasons in request decision traces
Add required copyright/SPDX header to Go test file
Add required copyright/SPDX header to Go test file
Add required license header to Go source file
Add required license header to Go source file
Add required license header to Go test file
Add required license header to Go source file

Previously missed (1)

In code that hasn't changed since last review

Preserve current validation failures in fallback quality

analysis/fallback.go:64

current is passed into this function before analyzeResource validates current request/limit freshness and inventory (those checks occur later in analyze.go). Consequently, a selected fallback's rolloutFallback.currentDataQuality omits those current-rollout failures even though the public contract says it preserves the failed current checks. Capture the completed current quality, or otherwise merge the later guard reasons, before populating this field.

Copilot

Copilot review overview

🟡 Changes recommended

No-usage safety retention is incorrectly reported as bounds enforcement, and sample-count documentation conflicts with implementation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 · 1

Open (3)

Compute request bounding before applying the no-usage safety floor · New
Avoid reporting limit bounding when retaining the current limit · New
Clarify sampleCount excludes duplicates and the initial raw point · New

Resolved since last review (2)

Preserve compatibility for removed MaximumGapSeconds field
Treat zero release boundary as absent when capping fallback time

+	if usageBlocked {
+		suggestedRequest = math.Max(suggestedRequest, obs.CurrentRequest.Value)
+	}
+	requestTrace.step("request bounds", map[string]float64{"before": rawSuggestedRequest, "minimum": bounds.Minimum, "maximum": bounds.Maximum, "candidate": suggestedRequest})
 	requestBounded := suggestedRequest != rawSuggestedRequest
-	boundsSuppressedAction := requestBounded && isMaterial(obs.CurrentRequest.Value, rawSuggestedRequest, bounds) && !isMaterial(obs.CurrentRequest.Value, suggestedRequest, bounds)
+	boundsSuppressedAction := requestBounded && isMaterial(obs.CurrentRequest, rawSuggestedRequest, bounds) && !isMaterial(obs.CurrentRequest, suggestedRequest, bounds)


+		if usageBlocked && limitOK {
+			suggestedLimit = math.Max(suggestedLimit, obs.CurrentLimit.Value)
+		}
+		limitTrace.step("request × limit ratio", map[string]float64{"request": suggestedRequest, "ratio": ratio, "candidate": rawSuggestedLimit})
+		limitTrace.step("limit bounds", map[string]float64{"before": rawSuggestedLimit, "minimum": suggestedRequest, "maximum": bounds.Maximum, "candidate": suggestedLimit})
 		limitBounded := suggestedLimit != rawSuggestedLimit
-		boundsSuppressedAction = boundsSuppressedAction || limitOK && limitBounded && isMaterial(obs.CurrentLimit.Value, rawSuggestedLimit, bounds) && !isMaterial(obs.CurrentLimit.Value, suggestedLimit, bounds)
+		boundsSuppressedAction = boundsSuppressedAction || limitOK && limitBounded && isMaterial(obs.CurrentLimit, rawSuggestedLimit, bounds) && !isMaterial(obs.CurrentLimit, suggestedLimit, bounds)


+`sampleCount` reports all distinct per-series samples; `observationCount` reports
+distinct timestamps across series after normalization (including CPU counter
+conversion) and is the count checked against `minimumSamples`. Confidence uses unique release


jkremser added 3 commits September 24, 2026 18:37

Copilot AI lite review requested due to automatic review settings September 24, 2026 17:37

Copilot started reviewing on behalf of jkremser September 24, 2026 17:38 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread analysis/analyze.go Outdated

Comment thread analysis/fallback.go

Comment thread analysis/fallback_test.go

Comment thread analysis/samples.go

Comment thread analysis/trace.go

Comment thread analysis/trace_test.go

Comment thread analysis/unset_test.go

Address review comments

1f6aae6

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

Copilot AI review requested due to automatic review settings September 25, 2026 09:59

Copilot started reviewing on behalf of jkremser September 25, 2026 10:00 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread analysis/fallback.go

Comment thread analysis/types.go

zroubalik approved these changes Sep 25, 2026

View reviewed changes

zroubalik requested a balanced review from Copilot September 25, 2026 13:07

Copilot started reviewing on behalf of zroubalik September 25, 2026 13:07 View session

jkremser merged commit 48832c5 into main Sep 25, 2026
4 checks passed

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Navigation Menu

Oom leaks rollout fallback - #7

Oom leaks rollout fallback#7
jkremser merged 4 commits into
mainfrom
oom-leaks-rollout-fallback

jkremser commented Sep 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jkremser commented Sep 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants