Subject: Re: out-of-order XID insertion in KnownAssignedXids
To: Michael Paquier <michael@paquier.xyz>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru>
 <20181005080422.GA7863@paquier.xyz>
From: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
Message-ID: <581a80f7-3992-f319-302d-7c9a384d64f7@postgrespro.ru>
Date: Mon, 8 Oct 2018 12:04:28 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <20181005080422.GA7863@paquier.xyz>
Content-Type: multipart/mixed;
 boundary="------------C3E06898757023A8CF0055B9"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------C3E06898757023A8CF0055B9
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 8bit


On 05.10.2018 11:04, Michael Paquier wrote:
> On Fri, Oct 05, 2018 at 10:06:45AM +0300, Konstantin Knizhnik wrote:
>> As you can notice, XID 2004495308 is encountered twice which cause error in
>> KnownAssignedXidsAdd:
>>
>>      if (head > tail &&
>>          TransactionIdFollowsOrEquals(KnownAssignedXids[head - 1], from_xid))
>>      {
>>          KnownAssignedXidsDisplay(LOG);
>>          elog(ERROR, "out-of-order XID insertion in KnownAssignedXids");
>>      }
>>
>> The probability of this error is very small but it can quite easily
>> reproduced: you should just set breakpoint in debugger after calling
>> MarkAsPrepared in twophase.c and then try to prepare any transaction.
>> MarkAsPrepared  will add GXACT to proc array and at this moment there will
>> be two entries in procarray with the same XID:
>>
>> [snip]
>>
>> Now generated RUNNING_XACTS record contains duplicated XIDs.
> So, I have been doing exactly that, and if you trigger a manual
> checkpoint then things happen quite correctly if you let the first
> session finish:
> rmgr: Standby     len (rec/tot):     58/    58, tx:          0, lsn:
> 0/016150F8, prev 0/01615088, desc: RUNNING_XACTS nextXid 608
> latestCompletedXid 605 oldestRunningXid 606; 2 xacts: 607 606
>
> If you still maintain the debugger after calling MarkAsPrepared, then
> the manual checkpoint would block.  Now if you actually keep the
> debugger, and wait for a checkpoint timeout to happen, then I can see
> the incorrect record.  It is impressive that your customer has been able
> to see that first, and then that you have been able to get into that
> state with simple steps.
>
>> I want to ask opinion of community about the best way of fixing this
>> problem.  Should we avoid storing duplicated XIDs in procarray (by
>> invalidating XID in original pgaxct) or eliminate/change check for
>> duplicate in KnownAssignedXidsAdd (for example just ignore
>> duplicates)?
> Hmmmmm...  Please let me think through that first.  It seems to me that
> the record should not be generated to begin with.  At least I am able to
> confirm what you see.

The simplest way to fix the problem is to ignore duplicates before 
adding them to KnownAssignedXids.
We in any case perform sort i this place...


-- 
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--------------C3E06898757023A8CF0055B9
Content-Type: text/x-patch;
 name="runnings_xids.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="runnings_xids.patch"

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index bd20497..a2ea233 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -807,9 +807,12 @@ ProcArrayApplyRecoveryInfo(RunningTransactions running)
 		/*
 		 * Add the sorted snapshot into KnownAssignedXids
 		 */
-		for (i = 0; i < nxids; i++)
-			KnownAssignedXidsAdd(xids[i], xids[i], true);
-
+		KnownAssignedXidsAdd(xids[0], xids[0], true);
+		for (i = 1; i < nxids; i++)
+		{
+			if (xids[i] != xids[i-1])
+				KnownAssignedXidsAdd(xids[i], xids[i], true);
+		}
 		KnownAssignedXidsDisplay(trace_recovery(DEBUG3));
 	}
 

--------------C3E06898757023A8CF0055B9--