Skip to content

MDEV-40669 HEAP MIN_ROWS pre-sizes blocks past max_heap_table_size - #5513

Open
arcivanov wants to merge 1 commit into
MariaDB:bb-blob-main-montyfrom
arcivanov:MDEV-40669
Open

MDEV-40669 HEAP MIN_ROWS pre-sizes blocks past max_heap_table_size#5513
arcivanov wants to merge 1 commit into
MariaDB:bb-blob-main-montyfrom
arcivanov:MDEV-40669

Conversation

@arcivanov

Copy link
Copy Markdown
Contributor

The bug

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:

ulong requested_min_records= min_records;        /* raw, unclamped */
...
min_records= MY_MIN(min_records, max_records);   /* the ceiling clamp */
...
cap_records= (heap_max_allocation_block - extra) / recbuffer;
if (records_in_block > cap_records)
  records_in_block= MY_MAX(requested_min_records, cap_records);

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.

max_heap_table_size MIN_ROWS before this PR main
64M 20000000 1.00 GiB 96.00 MiB 96.00 MiB
256M 20000000 1.00 GiB 384.00 MiB 384.00 MiB
256M 40000000 2.00 GiB 384.00 MiB 384.00 MiB
16M (shipped default) 4294967295 4.00 GiB 24.00 MiB -

The 4 GiB case needs no tuning at all and is bounded only by the INT_MAX32
clamp on memory_needed (2 GiB per block, one record block plus one hash key
block). A few such tables exhaust memory.

Sizing with this PR is identical to main in every case, down to the
96.02/384.02 HP_PTRS high-water values in the report.

The 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 - no
conditional needed.

Interaction with the MDEV-40447 cap

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; sizing therefore returns to exactly what it was before
MDEV-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_ROWS is an ask, honored only as
far 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-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, and a case showing that raising the ceiling lets
    the same MIN_ROWS pre-size further - the clamp is a ceiling clamp, not a
    MIN_ROWS ban.

Backing the fix out and rebuilding, unit tests 28/29/30 fail at 2147483616
while 25/26/27 pass unchanged; in MTR only the four unreachable-MIN_ROWS
rows 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,handler 49/49, and
main.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 touches
init_block(). git log -S requested_min_records -- storage/heap/hp_create.c
returns that one commit. It has not shipped in any release branch, so this is
a pre-release regression against bb-blob-main-monty.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant