Fix one-off bug causing missing commit timestamps for subtransactions

michaelpq · michaelpq · commit 1c0452c48ca4 · 2022-01-21T14:54:55.000+09:00
The logic in charge of writing commit timestamps (enabled with track_commit_timestamp) for subtransactions had a one-bug bug, where it would be possible that commit timestamps go missing for the last subtransaction committed. While on it, simplify a bit the iteration logic in the loop writing the commit timestamps, as per suggestions from Kyotaro Horiguchi and Tom Lane, so as some variable initializations are not part of the loop itself. Issue introduced in 73c986a. Analyzed-by: Alex Kingsborough Author: Alex Kingsborough, Kyotaro Horiguchi Discussion: https://postgr.es/m/73A66172-4050-4F2A-B7F1-13508EDA2144@amazon.com Backpatch-through: 10
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
@@ -184,7 +184,9 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
 	 * subxid not on the previous page as head.  This way, we only have to
 	 * lock/modify each SLRU page once.
 	 */
-	for (i = 0, headxid = xid;;)
+	headxid = xid;
+	i = 0;
+	for (;;)
 	{
 		int			pageno = TransactionIdToCTsPage(headxid);
 		int			j;
@@ -200,15 +202,15 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
 							 pageno);
 
 		/* if we wrote out all subxids, we're done. */
-		if (j + 1 >= nsubxids)
+		if (j >= nsubxids)
 			break;
 
 		/*
 		 * Set the new head and skip over it, as well as over the subxids we
 		 * just wrote.
 		 */
 		headxid = subxids[j];
-		i += j - i + 1;
+		i = j + 1;
 	}
 
 	/* update the cached value in shared memory */
