Skip to content

fix: zero out event._args before returning events slice to sync.Pool - #24471

Merged
mergify[bot] merged 2 commits into
matrixorigin:mainfrom
Torrent2002:jhx/issue-24400-eventlogger-leak
May 20, 2026
Merged

fix: zero out event._args before returning events slice to sync.Pool#24471
mergify[bot] merged 2 commits into
matrixorigin:mainfrom
Torrent2002:jhx/issue-24400-eventlogger-leak

Conversation

@Torrent2002

@Torrent2002 Torrent2002 commented May 19, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

LogSlowEvent resets the events slice with [:0] before returning it to eventsPool, but this only clears the length — the backing array retains stale interface{} pointers in event._args[16]any slots. These retained references prevent GC from collecting pointed-to objects (*IOVector, []byte, etc.), contributing to heap growth under high concurrency.

  • Zero out event.args and event._args for every event before returning the slice to the pool
  • Add TestEventLoggerPoolCleanup to verify pooled slices are clean on reuse

This fix addresses the retention side (stale pointers in pooled slices), not the allocation rate. Whether this alone resolves the OOM needs stability test verification.

Which issue(s) this PR fixes:

#24400

Test plan

  • go test -count=1 ./pkg/fileservice/... — all pass
  • go test -race -run TestEventLogger — clean
  • Stability test on a cluster with this patch

PR Type

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

@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 →

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


jhx seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label May 19, 2026
@Torrent2002
Torrent2002 force-pushed the jhx/issue-24400-eventlogger-leak branch from 28adffc to 3efb099 Compare May 20, 2026 03:22
jhx and others added 2 commits May 20, 2026 13:58
The `[:0]` reset only clears the slice length, leaving stale interface{}
pointers in the backing array's event._args slots. These retained
references prevent the GC from collecting pointed-to objects (IOVector,
byte slices, etc.), contributing to heap growth under high concurrency.

Zero both args and _args before returning the slice to eventsPool.

Ref: matrixorigin#24400

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@mergify

mergify Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-05-20 06:59 UTC · Rule: main
  • Checks skipped · PR is already up-to-date
  • Merged2026-05-20 07:00 UTC · at e30c30880fba4345164c62268a4e08963baea5d2 · squash

This pull request spent 31 seconds in the queue, including 3 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 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 d05991e into matrixorigin:main May 20, 2026
23 of 24 checks passed
@mergify mergify Bot removed the queued label May 20, 2026
ULookup pushed a commit to ULookup/matrixone that referenced this pull request May 26, 2026
…atrixorigin#24471)

`LogSlowEvent` resets the events slice with `[:0]` before returning it to `eventsPool`, but this only clears the length — the backing array retains stale `interface{}` pointers in `event._args[16]any` slots. These retained references prevent GC from collecting pointed-to objects (`*IOVector`, `[]byte`, etc.), contributing to heap growth under high concurrency.

- Zero out `event.args` and `event._args` for every event before returning the slice to the pool
- Add `TestEventLoggerPoolCleanup` to verify pooled slices are clean on reuse

This fix addresses the retention side (stale pointers in pooled slices), not the allocation rate. Whether this alone resolves the OOM needs stability test verification.

Approved by: @fengttt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement size/S Denotes a PR that changes [10,99] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants