On Fri, Apr 18, 2025 at 12:29 PM Amit Kapila wrote: > > On Thu, Apr 17, 2025 at 6:14 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> > > > > ----- > > Fix > > ----- > > > > I think we should keep the confirmed_flush even if the previous synced > > restart_lsn/catalog_xmin is newer. Attachments include a patch for the > same. > > > > This will fix the case we are facing but adds a new rule for slot > synchronization. > Can we think of a simpler way to fix this by avoiding updating other slot > fields > (like two_phase, two_phase_at) if restart_lsn or catalog_xmin of the local > slot > is ahead of the remote slot?
Since the fix for xmin advancement during fast-forward decoding has been pushed (commit aaf9e95), I'm attaching the V2 patch to address the assert failure by avoiding updating other slot fields (like two_phase, two_phase_at) if restart_lsn or catalog_xmin of the local slot is ahead. After commit aaf9e95, it's less likely for the synced slot's restart/catalog to advance ahead of the source slot (The tests in 040_standby_failover_slots_sync.pl would not trigger it anymore). However, this can still occur in certain scenarios. Here is an example scenario with the steps involved: 1. Create a failover replication slot named 'logicalslot' on primary and acquire it in the walsender. 2. Log two standby snapshots on primary: - `0/3000428` - `running_xacts` (no running transactions) - `0/3000460` - `running_xacts` (no running transactions) 3. The walsender sets `0/3000428` as the `candidate_restart_lsn`, skipping the second `running_xacts` because `candidate_restart_lsn` is already valid. 4. After receiving a reply from the apply worker, the walsender assigns `0/3000428` to `restart_lsn`. At this point, the replication slot 'logicalslot' has `restart_lsn` set to `0/3000428`. 5. On the standby, execute `pg_sync_replication_slots()` to synchronize 'logicalslot'. 6. During synchronization, with the initial `restart_lsn` at `0/3000428`, the sync slot reaches a consistent point at this position. As a result, it does not use this consistent point as `candidate_restart_lsn` (refer to `SnapBuildProcessRunningXacts()`). 7. The sync process identifies the second standby snapshot at `0/3000460` and uses its LSN as `candidate_restart_lsn`, eventually updating it to `restart_lsn`. 8. Now, the synced slot holds `restart_lsn` at `0/3000460`, which is ahead of the remote slot on the primary server. So, if a user prepares a txn and changes the two_phase setting to true after the steps mentioned above, the value for two_phase_at might get synced to the standby while skipping the sync of the latest confirmed_flush_lsn. So I think it's still necessary to fix the assert failure. BTW, this fix is only for HEAD as back-branches do not sync two_phase_at field. Best Regards, Hou zj
v2-0001-Fix-assertion-failure-when-decoding-synced-two-ph.patch
Description: v2-0001-Fix-assertion-failure-when-decoding-synced-two-ph.patch
