Disk-based aggregation spills and reloads in a loop: RowAggStorage::dump() keeps ~9 GiB free since MCOL-5950

Summary: since MCOL-5950, RowAggStorage::dump() tries to keep far more memory free than the next batch can use. With disk-based aggregation on, it spills rowgroups to disk after every batch and reads them straight back, so a large GROUP BY that spills runs several times slower. Results stay correct.

Where: utils/rowgroup/rowstorage.cpp, RowAggStorage::dump(). Commit bd0f59910 (MCOL-5950, PR #3525) changed the free-memory target from

const int64_t leaveFree = fNumOfInputRGPerThread * fRowGroupOut->getRowSize() * getBucketSize();

to

const int64_t leaveFree = std::max(RGDataSizeType(freeMemLimit), fNumOfInputRGPerThread * fRowGroupOut->getSizeWithStrings(fMaxRows) * getBucketSize());

The new value is one full rowgroup (with whatever strings the current RGData holds) x 20 x 17 bytes, the hash bucket size. The x getBucketSize() factor has the wrong unit: it multiplies bytes by bytes. The old value had the same factor but only one row, so it was tiny and rarely mattered. The PR is about finalization; this line wasn't discussed in review.

Why it matters now: with disk aggregation on, RMMemManager isn't strict. Since MCOL-6441, acquires go through getMemoryForce(), which always succeeds and can drive the pool negative. The dump() loop is then the only thing that brings an aggregation back under TotalUmMemory: it spills until getFree() > leaveFree. With MCOL-6441's correct accounting, a target this large is rarely met, so most calls spill.

Measured (3-node test cluster, 23.02.24 plus a backport of MCOL-5950, TotalUmMemory=18G, AllowDiskBasedAggregation=Y, default RowAggrRowGroupsPerThread=20, a daily GROUP BY over about 2M groups per query, two queries per node). gdb on a thread stopped at the if (freeMem > leaveFree) line:

leaveFree    = 9882185000   (9.2 GiB; more than half of the 18 GiB pool)

freeMem      = 9377971406

fNumOfInputRGPerThread = 20, fMaxRows = 8192, disk aggregation on, generations allowed

With the 23.02.24 formula the same frame gives about 1.2 MB, so it wouldn't have spilled at all.

• Stack snapshots during the run: 9 to 27 PrimProc threads in RowAggStorage::dump() > RowGroupStorage::saveRG(), plus 3 to 7 in RowGroupStorage::loadRG() < getRow() <- RowAggStorage::getTargetRow(). The same workload on 23.02.24's formula: none in either, in every snapshot.
• Per query: about 650 GB written to spill files and about 649 GB read back (counted before compression), with 0 new generations. On the old formula: tens of MB of spill and up to 4 generations.
• Elapsed time: 3 to 5 times slower per partition. Group counts were identical to the old formula's on every completed query, so this is speed only.

Affects: stable-23.10 since 23.10.4 (bd0f59910), 25.10.7 and the version_25.10.8 branch (all with MCOL-6441), and our 23.02 backport (MCOL-6674). Not stock 23.02.24.

Proposed fix: keep room for exactly one more input batch. A thread aggregates fNumOfInputRGPerThread input rowgroups at a time, and in the worst case every row is a new group, needing an output row and a hash bucket:

const int64_t leaveFree = std::max<int64_t>(

    freeMemLimit, fNumOfInputRGPerThread * (fRowGroupOut->getDataSize(fMaxRows) + fMaxRows * getBucketSize()));

On the cluster above that's about 0.3 to 0.6 GB. That's a larger margin than 23.02.24's ~1.2 MB, so it overshoots the memory limit less, and about 20 times less than the current value, so it doesn't spill and reload in a loop. It also stops reading the string store of whatever RGData fRowGroupOut points at. leaveFree is only read in this loop; finalize and the new-generation checks are unchanged.

Status: fix on our 23.02 test branch, compile-checked. Rig run pending (exact group counts plus timing back near the old formula). Attached: the gdb session output.