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