MDEV-40669 HEAP MIN_ROWS pre-sizes blocks past max_heap_table_size - #5513
Open
arcivanov wants to merge 1 commit into
Open
MDEV-40669 HEAP MIN_ROWS pre-sizes blocks past max_heap_table_size#5513arcivanov wants to merge 1 commit into
arcivanov wants to merge 1 commit into
Conversation
`init_block()` captures `requested_min_records` from the caller's `min_records` before the `min_records= MY_MIN(min_records, max_records)` clamp, then restores that raw value when the block allocation cap added by MDEV-40447 fires. `max_records` is derived from `max_heap_table_size` / `tmp_memory_table_size` and is the most rows the table can ever hold, so that clamp is what has always bounded a HEAP table's allocations. Restoring the pre-clamp value undoes it, and an unreachable `MIN_ROWS` pre-sizes the record block and every hash key block past the ceiling. With `max_heap_table_size=64M` and `MIN_ROWS=20000000` a one-row table allocates 1GB where it used to allocate 96MB; `MIN_ROWS=4294967295` at the shipped default 16MB ceiling allocates 4GB, bounded only by the `INT_MAX32` clamp on `memory_needed`. A few such tables exhaust memory. Fix: clamp `requested_min_records` to `max_records` as well. `MY_MIN` keeps 0 at 0, so "no `min_records` requested" stays distinguishable from an explicit `MIN_ROWS`, and the cap keeps ignoring the defaulted 1000-row heuristic. The cap and the clamp together give four regimes, and only the last one changes: 1. No `MIN_ROWS`: capped, sizing comes from the ceiling alone. 2. `MIN_ROWS` below the cap: capped. 3. `MIN_ROWS` above the cap but within `max_records`: pre-sizes to `MIN_ROWS`, past the cap, as MDEV-40447 intends. 4. `MIN_ROWS` at or above `max_records`: unreachable, so it degrades to plain ceiling-derived sizing. In case 4 the cap branch becomes a no-op, because `records_in_block` already equals `max_records`, so sizing returns to exactly what it was before MDEV-40447. Tests: - `storage/heap/hp_test_block_size-t.c`: a four-case boundary walk asserting the exact `alloc_size` of each regime above at one ceiling-derived `max_records`, and a keyed case asserting that an unreachable `MIN_ROWS` clamps the hash key block as well as the record block (`sizeof(HASH_INFO)` gives that block its own `recbuffer` and its own cap). - `mysql-test/suite/heap/min_rows_alloc.test`: the same regimes end to end across three ceilings, plus `MIN_ROWS` at the .frm maximum under the shipped default ceiling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
init_block()capturesrequested_min_recordsfrom the caller'smin_recordsbefore the
min_records= MY_MIN(min_records, max_records)clamp, thenrestores that raw value when the block allocation cap added by MDEV-40447
fires:
max_recordsis derived frommax_heap_table_size/tmp_memory_table_sizeand is the most rows the table can ever hold, so that clamp is what has always
bounded a HEAP table's allocations. Restoring the pre-clamp value undoes it,
and an unreachable
MIN_ROWSpre-sizes the record block and every hash keyblock past the ceiling.
max_heap_table_sizeMIN_ROWSmainThe 4 GiB case needs no tuning at all and is bounded only by the
INT_MAX32clamp on
memory_needed(2 GiB per block, one record block plus one hash keyblock). A few such tables exhaust memory.
Sizing with this PR is identical to
mainin every case, down to the96.02/384.02HP_PTRShigh-water values in the report.The fix
Clamp
requested_min_recordstomax_recordsas well.MY_MINkeeps 0 at 0,so "no
min_recordsrequested" stays distinguishable from an explicitMIN_ROWS, and the cap keeps ignoring the defaulted 1000-row heuristic - noconditional needed.
Interaction with the MDEV-40447 cap
The cap and the clamp together give four regimes, and only the last one
changes:
MIN_ROWS- capped, sizing comes from the ceiling alone.MIN_ROWSbelow the cap - capped.MIN_ROWSabove the cap but withinmax_records- pre-sizes toMIN_ROWS, past the cap, as MDEV-40447 intends.MIN_ROWSat or abovemax_records- unreachable, so it degrades toplain ceiling-derived sizing.
In case 4 the cap branch becomes a no-op, because
records_in_blockalreadyequals
max_records; sizing therefore returns to exactly what it was beforeMDEV-40447 rather than being special-cased against it.
The unifying rule, now written into the code comments: the cap governs what
the caller did not ask for; an explicit
MIN_ROWSis an ask, honored only asfar as the ceiling reaches.
Tests
Existing coverage hit regimes 1 and 3 and small-table sizing. Regimes 2 and 4
had none, and regime 4 is the bug.
storage/heap/hp_test_block_size-t.c(plan(24)->plan(30)): a four-caseboundary walk asserting the exact
alloc_sizeof each regime above atone ceiling-derived
max_records, and a keyed case asserting that anunreachable
MIN_ROWSclamps the hash key block as well as the record block(
sizeof(HASH_INFO)gives that block its ownrecbufferand its own cap).mysql-test/suite/heap/min_rows_alloc.test: the same regimes end to endacross three ceilings, plus
MIN_ROWSat the.frmmaximum under theshipped default ceiling, and a case showing that raising the ceiling lets
the same
MIN_ROWSpre-size further - the clamp is a ceiling clamp, not aMIN_ROWSban.Backing the fix out and rebuilding, unit tests 28/29/30 fail at
2147483616while 25/26/27 pass unchanged; in MTR only the four unreachable-
MIN_ROWSrows differ and the regime 1/2/3 rows do not appear in the diff at all. The
cap's behaviour is byte-identical either way.
Green: 9 heap unit binaries,
--suite=heap,handler49/49, andmain.tmp_table_heap_alloc(MDEV-40447's own end-to-end test),main.memory_used,main.ps_4heap,main.ctype_utf8mb4_heap,main.strict_autoinc_3heap,main.sp-memory-leak,main.create,main.type_blob.Note on provenance
The Jira issue is filed as caused by MDEV-38975, which is the umbrella; the
defect is in MDEV-40447 (
47f0242d888) and MDEV-38975 never touchesinit_block().git log -S requested_min_records -- storage/heap/hp_create.creturns that one commit. It has not shipped in any release branch, so this is
a pre-release regression against
bb-blob-main-monty.