Re: Fix premature xmin advancement during fast forward decoding - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Re: Fix premature xmin advancement during fast forward decoding
Date
Msg-id CAD21AoD1_-ypJg7e0WEqDyCGBgOhH8ka3M1nPj1Dk=ADG44+GQ@mail.gmail.com
Whole thread Raw
In response to Fix premature xmin advancement during fast forward decoding  ("Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>)
Responses RE: Fix premature xmin advancement during fast forward decoding
List pgsql-hackers
Hi,

On Tue, Apr 22, 2025 at 12:06 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> Hi,
>
> When analyzing some issues in another thread[1], I found a bug that fast forward
> decoding could lead to premature advancement of catalog_xmin, resulting in
> required catalog data being removed by vacuum.
>
> The issue arises because we do not build a base snapshot when decoding changes
> during fast forward mode, preventing reference to the minimum transaction ID
> that remains visible in the snapshot when determining the candidate for
> catalog_xmin. As a result, catalog_xmin was directly advanced to the oldest
> running transaction ID found in the latest running_xacts record.
>
> In code-level, SnapBuildProcessChange -> ReorderBufferSetBaseSnapshot could not
> be reached during fast forward decoding, resulting in
> rb->txns_by_base_snapshot_lsn being NULL. When advancing catalog_xmin, the
> system attempts to refer to rb->txns_by_base_snapshot_lsn via
> ReorderBufferGetOldestXmin(). However, since rb->txns_by_base_snapshot_lsn is
> NULL, it defaults to directly using running->oldestRunningXid as the candidate
> for catalog_xmin. For reference, see the details in
> SnapBuildProcessRunningXacts.

I agree with your analysis.

> To fix this, I think we can allow the base snapshot to be built during fast
> forward decoding, as implemented in the patch 0001 (We already built base
> snapshot in fast-forward mode for logical message in logicalmsg_decode()).
>
> I also explored the possibility of further optimizing the fix to reduce the
> overhead associated with building a snapshot in fast-forward mode. E.g., to
> maintain a single base_snapshot_xmin rather than generating a full snapshot for
> each transaction, and use this base_snapshot_xmin when determining the
> candidate catalog_xmin. However, I am not feeling confident about maintaining
> some fast-forward dedicated fields in common structures and
> perhaps employing different handling for catalog_xmin.
>
> Moreover, I conducted a basic test[2] to test the patch's impact, noting that
> advancing the slot incurs roughly a 4% increase in processing time after
> applying the patch, which appears to be acceptable.  Additionally, the cost
> associated with building the snapshot via SnapBuildBuildSnapshot() did not show
> up in the profile.

Where did the regression come from? I think that even with your patch
SnapBuildBuildSnapshot() would be called only a few times in the
workload. With the patch, I think that we need to create
ReorderBufferTXN entries for each transaction even during fast_forward
mode, which could lead to overheads. I think that 4% degradation is
something that we want to avoid.

Regards

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: Matthias van de Meent
Date:
Subject: Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?
Next
From: Jelte Fennema-Nio
Date:
Subject: Re: Making sslrootcert=system work on Windows psql