On 02/02/2024 11:00, Alexander Lakhin wrote:
Please try the following script:
mkdir /tmp/50m
sudo mount -t tmpfs -o size=50M tmpfs /tmp/50m
export PGDATA=/tmp/50m/tmpdb
initdb
pg_ctl -l server.log start
cat << 'EOF' | psql
CREATE TEMP TABLE t (a name, b name, c name, d name);
INSERT INTO t SELECT 'a', 'b', 'c', 'd' FROM generate_series(1, 1000) g;
COPY t TO '/tmp/t.data';
SELECT 'COPY t FROM ''/tmp/t.data''' FROM generate_series(1, 100)
\gexec
EOF
which produces an unexpected error, a warning, and an assertion failure,
starting from b8bff07da:
Fixed, thanks for the report!
Comparing ExtendBufferedRelLocal() and ExtendBufferedRelShared(), it's
easy to see that ExtendBufferedRelLocal() was missing a
ResourceOwnerEnlarge() call in the loop. But it's actually a bit more
subtle: it was correct without the ResourceOwnerEnlarge() call until
commit b8bff07da, because ExtendBufferedRelLocal() unpins the old buffer
pinning the new one, while ExtendBufferedRelShared() does it the other
way 'round. The implicit assumption was that unpinning the old buffer
ensures that you can pin a new one. That no longer holds with commit
b8bff07da. Remembering a new resource expects there to be a free slot in
the fixed-size array, but if the forgotten resource was in the hash,
rather than the array, forgetting it doesn't make space in the array.
We also make that assumption here in BufferAlloc:
/*
* Got a collision. Someone has already done what we were about
to do.
* We'll just handle this as if it were found in the buffer
pool in
* the first place. First, give up the buffer we were planning
to
* use.
*
* We could do this after releasing the partition lock, but
then we'd
* have to call ResourceOwnerEnlarge() &
ReservePrivateRefCountEntry()
* before acquiring the lock, for the rare case of such a
collision.
*/
UnpinBuffer(victim_buf_hdr);
It turns out to be OK in that case, because it unpins the buffer that
was the last one pinned. That does ensure that you have one free slot in
the array, but forgetting anything other than the most recently
remembered resource does not.
I've added a note to that in ResourceOwnerForget. I read through the
other callers of ResourceOwnerRemember and PinBuffer, but didn't find
any other unsafe uses. I'm not too happy with this subtlety, but at
least it's documented now.
--
Heikki Linnakangas
Neon (https://neon.tech)