Repository navigation
feat(soak): treat an OOM refusal of a full noeviction master as an expected state - #324
Conversation
The operator keeps maxmemory under a policy that is not allkeys-*. A master under noeviction then refuses writes with OOM when its data reaches maxmemory, and RedisSoakNotWritable fired on op-noevict, although no write was lost. - The observer reads used_memory, mem_not_counted_for_evict, maxmemory and the policy in its INFO of each pod. The master is full under noeviction when the counted memory is at maxmemory, less 1 percent and at least 1Mi. - A write probe that fails with OOM while the master is full sets redis_soak_expected_not_writable and counts its interval in redis_soak_expected_not_writable_seconds_total. RedisSoakNotWritable ignores such a client. Another error is still a fault. - The state is expected for observer.noevictionFullLimit, 30 minutes by default. Then it is the finding noeviction_full, and the alert fires again, because the fill and the ledger stop. - The finding maxmemory_below_used counts a maxmemory that decreased below the used memory of a pod under a policy that is not allkeys-*. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e9091f6af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A round without a single master, for example after an INFO timeout, cleared the start of the full state. Short gaps then started the limit again, so noeviction_full never counted. Only a master that is not full ends the state now. A round without a master keeps the time and gives no time beyond the limit. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
|
@codex review Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6611cdd20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tance The series of noeviction_full and maxmemory_below_used started at 0 only for an instance with maxMemoryPolicy. A change by hand can give another instance maxMemory, and both judges read the INFO of each instance. The first finding then started its series at 1, and the increase of the alert did not show it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
|
@codex review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Merge the soak noeviction rule (#324) and the root module cleanup (#327). The one conflict is the tracker line of observer.New: keep newTracker without the dwell parameter and the fullLimit field of #324. Observer.apply calls observeMaster, observeOOMKills, judgeMaxmemory and judgeNoeviction in the order of main, and New creates the series of noeviction_full and maxmemory_below_used for each instance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
Changes proposed on the PR:
740f2c9eraisedRedisSoakNotWritableonop-noevict(operator mode, policynoeviction) for about 85 s without any lost write. The operator behaves as documented (README: with a non-allkeys-*policy the operator never setsmaxmemorybelow the used memory it reads, and on a raised limit it sets the requested percent). Withnoeviction, Redis refuses writes ("OOM command not allowed") as soon as the used memory reachesmaxmemory, and nothing frees memory until the user deletes data or changes the policy, percent or limit.noevictionmaster as an expected state. "Full" means policynoevictionandused_memory - mem_not_counted_for_evict >= maxmemory - margin(1 percent ofmaxmemory, at least 1Mi), read from the INFO call that the observer already makes. A write probe counts as expected only when its ownSETfailed with OOM and the single master is full:redis_soak_expected_not_writable{path,client,reason="noeviction_full"}andredis_soak_expected_not_writable_seconds_total. Any other error stays "not writable".redis_soak_writablestays 0.RedisSoakNotWritablegetsunless on (rf, namespace, path, client) (redis_soak_expected_not_writable == 1). The threshold andfor: 1mare unchanged.e2e/rules-test.yamlhas a new case.observer.noevictionFullLimit(default 30m, a negative value is rejected) it counts the findingnoeviction_full, and the alert fires again.maxmemory_below_used: a pod'smaxmemorydecreased between two rounds below that pod's counted memory plus the margin, under a policy that is notallkeys-*. A pod without INFO keeps its last value; a new pod has no earlier value.Tests:
TestFull(10 cases),TestNoevictionFullBound,TestNoevictionFullDefault,TestMaxmemoryBelowUsed(9 cases),TestMaxmemoryBelowUsedWithoutInfo,TestExpectedNotWritable,TestRunnerSetFull; four mutation checks each fail a test.TestRunnerFollowsPaths(prober) times out under-race -count=3at high machine load, on main as well (pre-existing). Not run locally: the promtool check of the rules (make check-monitoringneeds Docker or promtool).🤖 Generated with Claude Code
https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
Generated by Claude Code