Avoid killing btree items that are already dead

_bt_killitems marks btree items dead when a scan leaves the page where
they live, but it does so with only share lock (to improve concurrency).
This was historicall okay, since killing a dead item has no
consequences.  However, with the advent of data checksums and
wal_log_hints, this action incurs a WAL full-page-image record of the
page.  Multiple concurrent processes would write the same page several
times, leading to WAL bloat.  The probability of this happening can be
reduced by only killing items if they're not already dead, so change the
code to do that.

The problem could eliminated completely by having _bt_killitems upgrade
to exclusive lock upon seeing a killable item, but that would reduce
concurrency so it's considered a cure worse than the disease.

Backpatch all the way back to 9.5, since wal_log_hints was introduced in
9.4.

Author: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Discussion: https://postgr.es/m/CA+fd4k6PeRj2CkzapWNrERkja5G0-6D-YQiKfbukJV+qZGFZ_Q@mail.gmail.com

diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index d4f20ada37c1d508be136800b54051f79be43473..8de9e6789f3599f7723650efaa25a6d952b0d68c 100644 (file)
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -1799,9 +1799,19 @@ _bt_killitems(IndexScanDesc scan)
 
                        if (ItemPointerEquals(&ituple->t_tid, &kitem->heapTid))
                        {
-                               /* found the item */
-                               ItemIdMarkDead(iid);
-                               killedsomething = true;
+                               /*
+                                * Found the item.  Mark it as dead, if it isn't already.
+                                * Since this happens while holding a buffer lock possibly in
+                                * shared mode, it's possible that multiple processes attempt
+                                * to do this simultaneously, leading to multiple full-page
+                                * images being sent to WAL (if wal_log_hints or data checksums
+                                * are enabled), which is undesirable.
+                                */
+                               if (!ItemIdIsDead(iid))
+                               {
+                                       ItemIdMarkDead(iid);
+                                       killedsomething = true;
+                               }
                                break;                  /* out of inner search loop */
                        }
                        offnum = OffsetNumberNext(offnum);