Re: Confine vacuum skip logic to lazy_scan_skip

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Melanie Plageman <melanieplageman(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Noah Misch <noah(at)leadboat(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Date: 2025-02-14 21:50:40
Message-ID: CA+hUKG+g6aXpi2FEHqeLOzE+xYw=OV+-N5jhOEnnV+F0USM9xA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Feb 15, 2025 at 7:30 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
> Seems valgrind doesn't like this [1]. I'm looking into it.

Melanie was able to reproduce this on her local valgrind and
eventually we figured out that it's my fault. I put code into
read_stream.c that calls wipe_mem(), thinking that that was our
standard way of scribbling 0x7f on memory that you shouldn't access
again until it's reused. I didn't realise that wipe_mem() also tells
valgrind that the memory is now "no access". That makes sense for
palloc/pfree because when that range is allocated again it'll clear
that. The point is to help people discover that they have a dangling
reference to per-buffer data after they advance to the next buffer,
which wouldn't work because it's in a circular queue and could be
overwritten any time after that.

This fixes it, but is not yet my proposed change:

@@ -193,9 +193,12 @@ read_stream_get_block(ReadStream *stream, void
*per_buffer_data)
if (blocknum != InvalidBlockNumber)
stream->buffered_blocknum = InvalidBlockNumber;
else
+ {
+ VALGRIND_MAKE_MEM_UNDEFINED(per_buffer_data,
stream->per_buffer_data_size);
blocknum = stream->callback(stream,

stream->callback_private_data,

per_buffer_data);
+ }

Thinking about how to make it more symmetrical...

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melanie Plageman 2025-02-14 22:21:25 Re: Parallel heap vacuum
Previous Message Nathan Bossart 2025-02-14 21:40:58 Re: [PATCH] SVE popcount support