rowexec: improve join reader memory usage when ordering is maintained #82693
Conversation
This commit improves the join reader behavior when used by lookup joins when ordering is maintained. In that case, we have a disk-backed row container that we can spill to disk in order to free up some memory. Previously, we would do so only in the case when the memory error occurs inside of the strategy but would make the query error out if the memory reservation is denied in the span generator or in the join reader itself. This commit makes it so that in all places where the memory accounting is performed, we try to ask the strategy to handle the memory reservations. This allows the join reader ordering strategy attempt spilling looked up rows to disk in all scenarios first, before erroring out the query. Additionally, this commit makes it so that the memory monitor used by that disk-backed row container doesn't hold on to any memory when the corresponding memory account is cleared. By default, our memory monitor can keep a margin of 100KiB in reserve, but this behavior doesn't make sense when we're forcing the disk-backed container to use disk. Release note: None
|
This change is |
|
This is a follow-up to the third commit in #79250. I initially thought that the scenario this PR is fixing is rather unlikely (in particular, we need to use up most of the memory limit by the looked up rows, without spilling to disk, only to hit the "budget exceeded" error in the join reader itself or in the span generator - which tend to use much less memory), but I ran into this problem in a test modified in #82671, so it seems worth fixing it. |
There was a problem hiding this comment.
Nice work!
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @mgartner)
|
TFTR! bors r+ |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 98821a7 to blathers/backport-release-22.1-82693: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
|
Build succeeded: |
This commit improves the join reader behavior when used by lookup joins
when ordering is maintained. In that case, we have a disk-backed row
container that we can spill to disk in order to free up some memory.
Previously, we would do so only in the case when the memory error occurs
inside of the strategy but would make the query error out if the memory
reservation is denied in the span generator or in the join reader
itself. This commit makes it so that in all places where the memory
accounting is performed, we try to ask the strategy to handle the memory
reservations. This allows the join reader ordering strategy attempt
spilling looked up rows to disk in all scenarios first, before erroring
out the query.
Additionally, this commit makes it so that the memory monitor used by
that disk-backed row container doesn't hold on to any memory when the
corresponding memory account is cleared. By default, our memory monitor
can keep a margin of 100KiB in reserve, but this behavior doesn't make
sense when we're forcing the disk-backed container to use disk.
Release note: None