On 1/22/22, 4:43 PM, "Tomas Vondra" <tomas.von...@enterprisedb.com> wrote: > There's a bug in ProcArrayApplyRecoveryInfo, introduced by 8431e296ea, > which may cause failures when starting a replica, making it unusable. > The commit message for 8431e296ea is not very clear about what exactly > is being done and why, but the root cause is that at while processing > RUNNING_XACTS, the XIDs are sorted like this: > > /* > * Sort the array so that we can add them safely into > * KnownAssignedXids. > */ > qsort(xids, nxids, sizeof(TransactionId), xidComparator); > > where "safely" likely means "not violating the ordering expected by > KnownAssignedXidsAdd". Unfortunately, xidComparator compares the values > as plain uint32 values, while KnownAssignedXidsAdd actually calls > TransactionIdFollowsOrEquals() and compares the logical XIDs :-(
Wow, nice find. > This likely explains why we never got any reports about this - most > systems probably don't leave transactions running for this long, so the > probability is much lower. And replica restarts are generally not that > common events either. I'm aware of one report with the same message [0], but I haven't read closely enough to determine whether it is the same issue. It looks like that particular report was attributed to backup_label being removed. > Attached patch is fixing this by just sorting the XIDs logically. The > xidComparator is meant for places that can't do logical ordering. But > these XIDs come from RUNNING_XACTS, so they actually come from the same > wraparound epoch (so sorting logically seems perfectly fine). The patch looks reasonable to me. Nathan [0] https://postgr.es/m/1476795473014.15979.2188%40webmail4
