airborne12 commented on code in PR #70007:
URL: https://github.com/apache/doris/pull/70007#discussion_r4453538952
##########
be/src/storage/index/inverted/snii/writer/spimi_term_buffer.cpp:
##########
@@ -375,9 +376,17 @@ void SpimiTermBuffer::report_arena_delta() {
}
// Report reclaimable arena bytes to the process-wide limiter. It may flag
// this buffer for a spill, which the owner thread handles after this call.
- if (global_limiter_ != nullptr) {
Review Comment:
Thanks, this is partly right, and the part that is right is fixed in
57843edda5.
What already bounded the delay:
- Every new term takes a slice from the arena (5 payload bytes plus a 4-byte
pointer at level 0), so the arena grows at least once per about 3,600 new
terms, and sooner with positions. Vocabulary cannot keep growing for long
inside one block.
- `request_spills_locked()` ranks every registered entry, so any writer
whose arena grows re-decides for all of them, including an idle one with an
eligible arena.
- "May never occur" holds only if no SNII writer allocates again. Before
this PR a report also needed a resident change, so that case is the same as
before.
What you are right about: a capacity doubling moves the resident total by
MiBs in one token without touching the arena. For example, the term slots
doubling from 2^17 to 2^18 entries add 3.5 MiB. Before this PR that token
re-decided at once. With the arena gate alone it waited for the next block.
The limiter is now also told when the resident total has moved by
`kBuildBytesPublishStep` (1 MiB) since its last decision, even inside one
block. That is the step in which the decision's input, the registered build
bytes, moves anyway, so per-term growth still stays off the mutex.
`ResidentStepInsideOneArenaBlockReachesTheLimiter` finds a term whose resident
step lands inside one arena block and checks that it is decided at that term;
without the new clause it fails. It costs almost nothing: on 2M tokens with
600k new terms a buffer makes 321 decisions instead of 310 with the arena gate
alone, and 563 instead of 550 when all 2M terms are new. Before this PR, every
new term was a decision.
--
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]
