Skip to content

fix(spill): stop at max level instead of erroring, fix memory leaks#24850

Merged
mergify[bot] merged 1 commit into
matrixorigin:4.0-devfrom
aunjgr:fix/spill-max-level-and-leaks-4.0
Jun 5, 2026
Merged

fix(spill): stop at max level instead of erroring, fix memory leaks#24850
mergify[bot] merged 1 commit into
matrixorigin:4.0-devfrom
aunjgr:fix/spill-max-level-and-leaks-4.0

Conversation

@aunjgr
Copy link
Copy Markdown
Contributor

@aunjgr aunjgr commented Jun 4, 2026

What type of PR is this?

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

Which issue(s) this PR fixes:

Fixes #23353
Fixes #24836

What this PR does / why we need it:

Cherry-pick of #24848 to 4.0-dev. Four fixes in the spill code:

1. Spill max level: stop instead of erroring. When group-by spill hits the 3-level limit, now returns nil instead of erroring — data stays in memory.

2. spillAggList leak on error paths. Added freeSpillAggList() to deferred cleanup in loadSpilledData.

3. Group spill cached buffers never freed. Nil spillBuf, spillReader, spillHashCodes, spillChunkFlags, spillFlagFlat, spillNonEmptyBuckets, spillBucketRowIds in free().

4. Hashbuild/hashjoin spill executors and buffers never freed. Added freeSpillExprExecs/freeSpillBuildExprExecs and buffer nil to Free().

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

- group spill: return nil at spillMaxPass instead of erroring, keeping
  remaining data in memory rather than failing the query.
- group spill: fix spillAggList leak on error paths in loadSpilledData
  by freeing in deferred cleanup.
- group/hashbuild/hashjoin: nil cached spill buffers (bufio.Reader,
  bytes.Buffer, hashCodes, chunkFlags, flagFlat, nonEmptyBuckets,
  bucketRowIds, expression executors) in Free() to allow GC on
  operator reuse.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@aunjgr aunjgr requested a review from ouyuanning as a code owner June 4, 2026 09:50
@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 →

@mergify mergify Bot added the queued label Jun 4, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Jun 4, 2026

Merge Queue Status

  • Entered queue2026-06-04 13:47 UTC · Rule: release-4.0
  • Checks skipped · PR is already up-to-date
  • Merged2026-06-05 08:28 UTC · at 03d4975e88c08cca1f30e8861ec4b9e1dca9b370 · squash

This pull request spent 18 hours 41 minutes 12 seconds in the queue, including 5 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 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
  • 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

@mergify mergify Bot merged commit 373ec8e into matrixorigin:4.0-dev Jun 5, 2026
23 of 24 checks passed
@mergify mergify Bot removed the queued label Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working 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