Incorrect CHUNKHDRSZ in nodeAgg.c

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Incorrect CHUNKHDRSZ in nodeAgg.c
Date: 2025-01-01 22:55:32
Message-ID: CAApHDvpMpRQvsTqZo3FinXkgytwxwF8sCyZm83xDj-1s_hLe+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While reading nodeAgg.c, I noticed code that uses CHUNKHDRSZ to help
figure out how much memory a tuple uses so the code knows when to
spill to disk. CHUNKHDRSZ is currently set to 16 bytes, which was
fine when that code was added, but it's a few years out-of-date since
c6e0fe1f2 in 2022.

The attached adjusts the 16 to sizeof(MemoryChunk), which is normally
8, but 16 in assert builds.

The memory accounting should be more accurate with the patch, however,
that accounting doesn't account for blocks that the chunks are on. For
that reason, I'm thinking of not backpatching this as it will make
hash aggregates use a bit more memory than unpatched before spilling,
albeit, for most cases, closer to the hash_mem limit, which is when
the spilling should be happening.

Sound ok?

David

Attachment Content-Type Size
fix_nodeAgg_CHUNKHDRSZ.patch application/octet-stream 787 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-01-01 23:18:56 Re: Incorrect CHUNKHDRSZ in nodeAgg.c
Previous Message Peter Smith 2025-01-01 22:50:29 Re: Log a warning in pg_createsubscriber for max_slot_wal_keep_size