Hi, On Tue, Apr 22, 2025 at 12:06 AM Zhijie Hou (Fujitsu) <houzj.f...@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