Re: GiST VACUUM

On 26/06/2019 06:07, Thomas Munro wrote:
> On Wed, Jun 26, 2019 at 2:33 PM Michael Paquier <michael@paquier.xyz> wrote:
>> On Tue, Jun 25, 2019 at 02:38:43PM +0500, Andrey Borodin wrote:
>>> I feel a little uncomfortable with number 0x7fffffff right in code.
>>
>> PG_INT32_MAX...
> 
> MaxTransactionId / 2?

Yeah, that makes sense.

Here's a new version of the patches. Changes:

* I changed the reuse-logging so that we always write a reuse WAL 
record, even if the deleteXid is very old. I tried to avoid that with 
the check for MaxTransactionId / 2 or 0x7fffffff, but it had some 
problems. In the previous patch version, it wasn't just an optimization. 
Without the check, we would write 32-bit XIDs to the log that had 
already wrapped around, and that caused the standby to unnecessarily 
wait for or kill backends. But checking for MaxTransaction / 2 isn't 
quite enough: there was a small chance that the next XID counter 
advanced just after checking for that, so that we still logged an XID 
that had just wrapped around. A more robust way to deal with this is to 
log a full 64-bit XID, and check for wraparound at redo in the standby. 
And if we do that, trying to optimize this in the master doesn't seem 
that important anymore. So in this patch version, we always log the 
64-bit XID, and check for the MaxTransaction / 2 when replaying the WAL 
record instead.

* I moved the logic to extend a 32-bit XID to 64-bits to a new helper 
function in varsup.c.

* Instead of storing just a naked FullTransactionId in the "page 
contents" of a deleted page, I created a new struct for that. The effect 
is the same, but I think the struct clarifies what's happening, and it's 
a good location to attach a comment explaining it.

* Fixed the mixup between < and >

I haven't done any testing on this yet. Andrey, would you happen to have 
an environment ready to test this?

- Heikki