Skip to content

add rss pressure cache controller - #24665

Merged
mergify[bot] merged 3 commits into
matrixorigin:mainfrom
LeftHandCold:fix/rss-pressure-controller-step3
May 28, 2026
Merged

add rss pressure cache controller#24665
mergify[bot] merged 3 commits into
matrixorigin:mainfrom
LeftHandCold:fix/rss-pressure-controller-step3

Conversation

@LeftHandCold

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

Fixes #24664

What this PR does / why we need it:

This PR adds an RSS pressure controller for memory cache pressure handling.

Before this change, RSS cache pressure used a fixed admission-target TTL as the main lifecycle mechanism. If RSS recovered quickly, the memory cache admission target could still remain active until the TTL expired, slowing data cache recovery.

This PR changes that behavior by:

  • adding explicit RSS pressure states: none, soft, and hard;
  • using hysteresis for state transitions:
    • none -> soft: RSS >= 85%;
    • soft -> hard: RSS >= 92%;
    • hard -> soft: RSS <= 88%;
    • soft/hard -> none: RSS <= 80%;
  • setting memory cache admission target by state:
    • soft: 80%;
    • hard: 50%;
  • clearing the memory cache admission target immediately when pressure exits;
  • keeping evict and debug.FreeOSMemory actions throttled by cooldown;
  • keeping the target TTL as a lease fallback that is renewed while pressure remains active;
  • splitting target set/clear from cache eviction so asynchronous evict actions cannot restore stale pressure targets;
  • guarding async evict with a pressure generation check.

The change keeps Step 1/2 behavior intact: RSS pressure still only controls data/memory cache and does not force objectio meta cache eviction.

Copilot AI review requested due to automatic review settings May 28, 2026 07:30
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an explicit RSS pressure controller for memory cache admission and eviction, replacing TTL-only lifecycle behavior with state-based pressure handling.

Changes:

  • Adds RSS pressure states (none, soft, hard) with hysteresis thresholds and target percentages.
  • Splits pressure target set/clear from cache eviction and adds generation checks for async eviction.
  • Exports memory cache pressure target clearing and wires the new pressure target callbacks in CN and workspace throttlers.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/common/rscthrottler/resource_throttler.go Implements RSS pressure state machine, target lifecycle callbacks, and async evict generation guard.
pkg/common/rscthrottler/resource_throttler_test.go Adds lifecycle coverage for state transitions and updates concurrent escalation test inputs.
pkg/fileservice/mem_cache.go Exports pressure target clearing and reuses it for invalid target inputs.
pkg/fileservice/mem_cache_test.go Adds coverage for clearing memory cache pressure targets.
pkg/cnservice/server.go Wires CN RSS cache pressure target setter/clearer separately from eviction.
pkg/vm/engine/disttae/engine.go Wires workspace RSS cache pressure target setter/clearer separately from eviction.

Comment thread pkg/common/rscthrottler/resource_throttler.go
@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label May 28, 2026
@LeftHandCold

Copy link
Copy Markdown
Contributor Author

Addressed the review findings:

  • Moved memory cache pressure target clear/set side effects into the rssScavengeMu critical section. The generation check is no longer used for synchronous target mutation, so there is no gap where a stale transition can clear a newer target after another refresh has changed the state.
  • Changed memory cache pressure target management to be owner-scoped:
    • CN uses owner cn-rss.
    • Workspace uses owner workspace-rss.
    • ClearMemoryCachePressureTargetByOwner only clears that owner.
    • The effective target is the minimum percent among active owners.
  • Kept the cache admission hot path on atomic reads. The owner map is locked only on set/clear or when the cached effective target expires and needs recomputation.
  • Added TestMemoryCachePressureTargetOwnerIsolation to cover cross-owner clear behavior.

Re-ran focused tests:

GOCACHE=/private/tmp/mo-gocache-step3 go test -mod=mod ./pkg/common/rscthrottler -run 'TestMemThrottlerRSS' -count=1
GOCACHE=/private/tmp/mo-gocache-step3 go test -mod=mod ./pkg/fileservice -run 'TestMemoryCachePressure|TestClearMemoryCachePressureTarget|TestMemoryCacheEvictToCapacityPercent' -count=1
GOCACHE=/private/tmp/mo-gocache-step3 go test -mod=mod ./pkg/cnservice -run 'TestMakeRSSCacheEvictorEvictsMemoryCacheOnly' -count=1
GOCACHE=/private/tmp/mo-gocache-step3 go test -mod=mod ./pkg/vm/engine/disttae -run 'TestNonExistent' -count=1

@LeftHandCold LeftHandCold changed the title fix: add rss pressure cache controller add rss pressure cache controller May 28, 2026

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-review skill result (full review mode): no must-fix / merge-blocking issues found.\n\nDestructive-test conclusion: PARTIAL. The PR adds focused lifecycle and owner-isolation tests, but does not include destructive stress tests for concurrent owner target churn or deadline/ownership races under continuous pressure transitions (e.g., interleaving SetByOwner and ClearByOwner under concurrent scavenge calls).\n\nNon-blocking findings:\n1) Consider adding a concurrency-focused destructive test around owner-target interleaving to validate effective target recomputation under high-frequency transitions.\n2) Consider adding a test for expired-owner cleanup during repeated pressure churn to confirm effective deadline handling remains correct under continuous updates.\n\nThese findings are non-blocking and do not prevent merge.

@mergify mergify Bot added the queued label May 28, 2026
@mergify

mergify Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-05-28 09:28 UTC · Rule: main
  • Checks skipped · PR is already up-to-date
  • Merged2026-05-28 09:29 UTC · at 7ed66db95afc82b092962acdb157f8c6b4e188d5 · squash

This pull request spent 1 minute 9 seconds in the queue, including 13 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage

@mergify
mergify Bot merged commit 2b52daa into matrixorigin:main May 28, 2026
23 of 24 checks passed
@mergify mergify Bot removed the queued label May 28, 2026
XuPeng-SH added a commit that referenced this pull request May 28, 2026
### What type of PR is this?

/kind improvement

### What this PR does / why we need it:

Cherry-pick #24665 to 4.0-dev.

This backports the RSS pressure controller state machine and cache
pressure target lifecycle improvements:
- use none / soft / hard RSS pressure states with hysteresis;
- keep memory cache pressure target as a lease while RSS pressure is
active, and clear it immediately after pressure exits;
- separate action throttling from pressure state transitions;
- use owner-scoped memory cache pressure targets so CN and Workspace
controllers do not clear each other;
- keep async cache eviction guarded by pressure generation to avoid
stale eviction actions from rewriting controller state.

### Which issue(s) this PR fixes:

Refs #24664
Backport of #24665

### Special notes for your reviewer:

The cherry-pick had one conflict in
pkg/common/rscthrottler/resource_throttler.go. The conflict was resolved
by keeping the main-branch generation guard for async cache eviction.

### Does this PR introduce a user-facing change?

NONE

### How to test:

- GOCACHE=/private/tmp/mo-gocache-pr24665-4dev go test
./pkg/common/rscthrottler ./pkg/fileservice ./pkg/cnservice -run
'TestMemThrottlerRSS|TestMemoryCachePressure|TestClearMemoryCachePressureTarget|TestMakeRSSCacheEvictor'
-count=1

Co-authored-by: XuPeng-SH <xupeng3112@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tech Request]: Add RSS pressure controller for memory cache target lifecycle

5 participants