|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Michael Paquier <michael(at)paquier(dot)xyz>|
|Cc:||pgsql-hackers(at)postgresql(dot)org, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>|
|Subject:||Re: Incorrect snapshots while promoting hot standby node when 2PC is used|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2021-05-26 16:57:31 +0900, Michael Paquier wrote:
> Yes, there should not be any as far as I recall. 2PC is kind of
> special with its fake ProcArray entries.
It's really quite an awful design :(
> > I think to fix the issue we'd have to move
> > ShutdownRecoveryTransactionEnvironment() to after XLogCtl->SharedRecoveryState
> > = RECOVERY_STATE_DONE.
> > The acquisition of ProcArrayLock() in
> > ShutdownRecoveryTransactionEnvironment()->ExpireAllKnownAssignedTransactionIds()
> > should prevent the data from being removed between the RecoveryInProgress()
> > and the KnownAssignedXidsGetAndSetXmin() calls in GetSnapshotData().
> > I haven't yet figured out whether there would be a problem with deferring the
> > other tasks in ShutdownRecoveryTransactionEnvironment() until after
> > RECOVERY_STATE_DONE.
> Hmm. This would mean releasing all the exclusive locks tracked by a
> standby, as of StandbyReleaseAllLocks(), after opening the instance
> for writes after a promotion. I don't think that's unsafe, but it
> would be intrusive.
Why would it be intrusive? We're talking a split second here, no? More
importantly, I don't think it's correct to release the locks at that
> Anyway, isn't the issue ExpireAllKnownAssignedTransactionIds() itself,
> where we should try to not wipe out the 2PC entries to make sure that
> all those snapshots still see the 2PC transactions as something to
> count on? I am attaching a crude patch to show the idea.
I don't think that's sufficient. We can't do most of the other stuff in
ShutdownRecoveryTransactionEnvironment() before changing
XLogCtl->SharedRecoveryState either. As long as the other backends think
we are in recovery, we shouldn't release e.g. the virtual transaction.
|Next Message||Bharath Rupireddy||2021-05-27 17:03:39||Re: CREATE COLLATION - check for duplicate options and error out if found one|
|Previous Message||Bharath Rupireddy||2021-05-27 16:58:23||Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation|