Fix incorrect logic for clearing BufferDirtiedByMe in ReleaseRelationBuffers

and DropBuffers.  Formerly we cleared the flag for each buffer currently
belonging to the target rel or database, but that's completely wrong!
Must look at BufferTagLastDirtied to see whether the BufferDirtiedByMe
flag is relevant to target rel or not; this is *independent* of the
current contents of the buffer.  Vadim spotted this problem, but his
fix was only partially correct...

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 9b7a1c144f8fd2c4198c632bd6fa9032b97821e3..dea01d252fb0c959dab540c50eda56b0a0442fdc 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1567,7 +1567,6 @@ recheck:
                        }
                        /* Now we can do what we came for */
                        bufHdr->flags &= ~(BM_DIRTY | BM_JUST_DIRTIED);
-                       BufferDirtiedByMe[i - 1] = false;
 
                        /*
                         * Release any refcount we may have.
@@ -1594,12 +1593,23 @@ recheck:
                else
                {
                        Assert(bufHdr->relId.relId != relid ||
-                       (bufHdr->relId.dbId != MyDatabaseId &&
-                        bufHdr->relId.dbId != InvalidOid));
-                       if (RelFileNodeEquals(rel->rd_node, 
-                                       BufferTagLastDirtied[i - 1].rnode))
-                               BufferDirtiedByMe[i - 1] = false;
+                                  (bufHdr->relId.dbId != MyDatabaseId &&
+                                       bufHdr->relId.dbId != InvalidOid));
                }
+
+               /*
+                * Also check to see if BufferDirtiedByMe info for this buffer
+                * refers to the target relation, and clear it if so.  This is
+                * independent of whether the current contents of the buffer
+                * belong to the target relation!
+                *
+                * NOTE: we have no way to clear BufferDirtiedByMe info in other
+                * backends, but hopefully there are none with that bit set for
+                * this rel, since we hold exclusive lock on this rel.
+                */
+               if (RelFileNodeEquals(rel->rd_node, 
+                                                         BufferTagLastDirtied[i - 1].rnode))
+                       BufferDirtiedByMe[i - 1] = false;
        }
 
        SpinRelease(BufMgrLock);
@@ -1652,7 +1662,6 @@ recheck:
                        }
                        /* Now we can do what we came for */
                        bufHdr->flags &= ~(BM_DIRTY | BM_JUST_DIRTIED);
-                       BufferDirtiedByMe[i - 1] = false;
 
                        /*
                         * The thing should be free, if caller has checked that no
@@ -1667,9 +1676,19 @@ recheck:
                else
                {
                        Assert(bufHdr->relId.dbId != dbid);
-                       if (BufferTagLastDirtied[i - 1].rnode.tblNode == dbid)
-                               BufferDirtiedByMe[i - 1] = false;
                }
+
+               /*
+                * Also check to see if BufferDirtiedByMe info for this buffer
+                * refers to the target database, and clear it if so.  This is
+                * independent of whether the current contents of the buffer
+                * belong to the target database!
+                *
+                * (Actually, this is probably unnecessary, since I shouldn't have
+                * ever dirtied pages of the target database, but...)
+                */
+               if (BufferTagLastDirtied[i - 1].rnode.tblNode == dbid)
+                       BufferDirtiedByMe[i - 1] = false;
        }
        SpinRelease(BufMgrLock);
 }