Fix the buffer release order for parallel index scans.

Amit Kapila · Amit Kapila · commit 8ce29bb4f0df · 2018-07-27T10:53:00.000+05:30
During parallel index scans, if the current page to be read is deleted, we skip it and try to get the next page for a scan without releasing the buffer lock on the current page. To get the next page, sometimes it needs to wait for another process to complete its scan and advance it to the next page. Now, it is quite possible that the master backend has errored out before advancing the scan and issued a termination signal for all workers. The workers failed to notice the termination request during wait because the interrupts are held due to buffer lock on the previous page. This lead to all workers being stuck. The fix is to release the buffer lock on current page before trying to get the next page. We are already doing same in backward scans, but missed it for forward scans. Reported-by: Victor Yegorov Bug: 15290 Diagnosed-by: Thomas Munro and Amit Kapila Author: Amit Kapila Reviewed-by: Thomas Munro Tested-By: Thomas Munro and Victor Yegorov Backpatch-through: 10 where parallel index scans were introduced Discussion: https://postgr.es/m/153228422922.1395.1746424054206154747@wrigleys.postgresql.org
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
@@ -1497,17 +1497,19 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber blkno, ScanDirection dir)
 			/* nope, keep going */
 			if (scan->parallel_scan != NULL)
 			{
+				_bt_relbuf(rel, so->currPos.buf);
 				status = _bt_parallel_seize(scan, &blkno);
 				if (!status)
 				{
-					_bt_relbuf(rel, so->currPos.buf);
 					BTScanPosInvalidate(so->currPos);
 					return false;
 				}
 			}
 			else
+			{
 				blkno = opaque->btpo_next;
-			_bt_relbuf(rel, so->currPos.buf);
+				_bt_relbuf(rel, so->currPos.buf);
+			}
 		}
 	}
 	else

Original file line number	Diff line number	Diff line change
`@@ -1497,17 +1497,19 @@ _bt_readnextpage(IndexScanDesc scan, BlockNumber blkno, ScanDirection dir)`
`1497`	`1497`	`/* nope, keep going */`
`1498`	`1498`	`if (scan->parallel_scan != NULL)`
`1499`	`1499`	`{`
	`1500`	`+ _bt_relbuf(rel, so->currPos.buf);`
`1500`	`1501`	`status = _bt_parallel_seize(scan, &blkno);`
`1501`	`1502`	`if (!status)`
`1502`	`1503`	`{`
`1503`		`- _bt_relbuf(rel, so->currPos.buf);`
`1504`	`1504`	`BTScanPosInvalidate(so->currPos);`
`1505`	`1505`	`return false;`
`1506`	`1506`	`}`
`1507`	`1507`	`}`
`1508`	`1508`	`else`
	`1509`	`+ {`
`1509`	`1510`	`blkno = opaque->btpo_next;`
`1510`		`- _bt_relbuf(rel, so->currPos.buf);`
	`1511`	`+ _bt_relbuf(rel, so->currPos.buf);`
	`1512`	`+ }`
`1511`	`1513`	`}`
`1512`	`1514`	`}`
`1513`	`1515`	`else`