Re: out-of-order XID insertion in KnownAssignedXids

On Tue, Oct 09, 2018 at 05:26:49PM +0300, Konstantin Knizhnik wrote:
> But GetRunningTransactionData may be used in some other cases... For
> example I have used it in my snapfs Postgres extensions (fast
> file-level snapshots) to check if there are no more active
> transactions in the system. In my case presence of duplicates is not
> important and ordering of XIDs is not needed. But performance of
> GetRunningTransactionData is also not critical. But I can suspect that
> there can be cases when it is critical and doing unneeded qsort is not
> desired. There may be thousands of active transactions and sorting
> their XIDs is notinstantaneous.

I am not sure if the performance argument is actually this much
sensible, it could be as you say, but another thing that we could argue
about is that the presence of duplicate entries in
GetRunningTransactionData() can be used as a point to understand that
2PC transactions are running, and that among the two, one of them is a
dummy entry while the other is pending for being cleared.

> So I completely understand the argument that it is better to eliminate
> source of the problem (presence of duplicates in RUNNING_XACTS record)
> but not sure that in this particular case it is really the best
> solution. If presence of duplicates is considered to be acceptable for
> procarray, why we can not accept it for RUNNING_XACTS record?

This won't solve the case where records have already been generated and
won't be processed correctly, but based on the chances this can happen
we can rather safely say that fixing only the source would be fine.

> I am not insisting that skipping duplicates in ProcArrayApplyRecoveryInfo is
> the right place (but at least is seems to be the most efficient
> alternative). But I also do not fill that moving sort to
> GetRunningTransactionData and elimination of duplicates here (requires one
> more scan and copying of the whole array) is principally better...

Making recovery a couple of instructions shorter is always a good thing
in my opinion. Users don't care much if backups take a long time, ans
sweat less if restores take a shorter time.

Hence what about doing roughly the following:

1) Document that GetRunningTransactionData() fetches information also
about 2PC entries, like that:
* GetRunningTransactionData -- returns information about running transactions.
*
* Similar to GetSnapshotData but returns more information. We include
- * all PGXACTs with an assigned TransactionId, even VACUUM processes.
+ * all PGXACTs with an assigned TransactionId, even VACUUM processes and
+ * dummy two-phase related entries created when preparing the transaction.

2) Update LogStandbySnapshot so as the list of XIDs fetched is sorted
and ordered. This can be used as a sanity check for recovery via
ProcArrayApplyRecoveryInfo().
--
Michael