Yukang-Lian opened a new pull request, #65963:
URL: https://github.com/apache/doris/pull/65963
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
#### Problem
Vertical compaction lazily opens one segment per input rowset for valid
NONOVERLAPPING rowsets. In the value-column merge path, however, a segment
context was retained after its last row source had already been consumed.
Its segment reader and decoded Blocks were not released until the whole value
group was destroyed.
As a result, the number of retained contexts grew with the total input
segment
count instead of staying near the merge way/input rowset count. This caused
vertical compaction peak memory to grow linearly with segment count even when
the rowsets were non-overlapping.
This was delayed resource release during compaction, not a permanent leak:
the contexts were eventually released when the value group ended.
#### Root cause
`RowSourcesBuffer` recorded which segment produced each output row, but did
not expose when a source had been consumed for the last time. The value paths
therefore had no lifecycle boundary at which they could safely release an
exhausted `VerticalMergeIteratorContext`.
#### Fix
- Record each source's last global row-source position. The position remains
correct when `RowSourcesBuffer` spills and is reset by `seek_to_begin()`
between value groups.
- After returning the last row or batch for a source, release that context's
segment iterator and context-owned Blocks. Returned `IteratorRowRef` and
`RowBatch` objects retain their own `shared_ptr<Block>`, so their data
remains
valid.
- Cover AGG `next_row()`, UNIQUE fallback `unique_key_next_row()`, and UNIQUE
sparse-batch `unique_key_next_batch()`.
- Add current and peak Bvars for active vertical-compaction segment contexts:
`vertical_compaction_active_segment_contexts` and
`vertical_compaction_active_segment_contexts_peak`.
- Keep the key-group heap merge, DUP batch path, compaction selection, batch
sizing, and MemTracker accounting unchanged.
#### Verification
The before/after memory test uses identical deterministic input: 10
NONOVERLAPPING input rowsets, 32,000 rows, an 8 KiB VARCHAR payload per row,
and compaction batch size 32.
| Input | Context peak before | Context peak after | Memory peak before |
Memory peak after |
|---|---:|---:|---:|---:|
| 100 segments | 100 | 10 | 51.89 MiB | 6.1145 MiB |
| 500 segments | 500 | 10 | 255.14 MiB | 6.1047 MiB |
Adding 400 segments increased the pre-fix peak by about 203 MiB. After the
fix, the two memory peaks differ by only 10,250 bytes. Repeated runs produced
the same context and memory peaks.
Additional lifecycle coverage verifies:
- UNIQUE fallback: 3 rowsets x 4 segments, context peak reduced from 12 to 3.
- UNIQUE sparse batch: context peak is 3, with sparse optimization confirmed
enabled.
- AGG row path: 2 rowsets x 2 segments, context peak is 2.
- Current context count returns to zero after every compaction.
- Output keys, 8 KiB payloads, delete signs, and row counters are unchanged.
- Spill and `seek_to_begin()` preserve global source-position tracking.
Focused ASAN BE unit tests passed:
```text
6/6:
TestRowSourcesBuffer
TestRowSourcesBufferSpillThreshold
TestUniqueKeyVerticalMerge
TestUniqueKeyNonOverlappingSegmentContextRetention
TestUniqueKeySegmentContextMemoryAmplification
TestUniqueKeyVerticalMergeWithNullableSparseColumn
3/3:
TestAggKeyVerticalMerge
RowBatchSharedPtrLifetime
RowBatchVector
Review follow-up 2/2:
TestUniqueKeyNonOverlappingSegmentContextRetention
TestAggKeyVerticalMerge
```
`build-support/check-format.sh` and `git diff --check` passed. The changed
production code also compiled with `-Wall -Wextra -Werror` in the ASAN unit
test build. Production-file clang-tidy is currently blocked by the existing
unmatched `NOLINTEND` at `be/src/core/types.h:576` on master, unrelated to
this change.
### Release note
Reduce vertical compaction peak memory by releasing exhausted segment readers
and decoded Blocks during value-column merging.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (detailed verification is included above)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
- Behavior changed:
- [ ] No.
- [x] Yes. Exhausted value-path segment contexts are released immediately
after their final row source is handed to the caller.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
