Update comments related to the crash-safety of the visibility map.

In hio.c, document how we avoid deadlock with respect to visibility map
buffer locks.  In visibilitymap.c, update the LOCKING section of the
file header comment.

Both oversights noted by Heikki Linnakangas.

diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index b4ea84a6141daeb699af39554f84949a06023af0..26db1e3d8d0012d619e16383a16030fe5f77829d 100644 (file)
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -178,6 +178,10 @@ GetVisibilityMapPins(Relation relation, Buffer buffer1, Buffer buffer2,
  * happen if space is freed in that page after heap_update finds there's not
  * enough there).  In that case, the page will be pinned and locked only once.
  *
+ * For the vmbuffer and vmbuffer_other arguments, we avoid deadlock by
+ * locking them only after locking the corresponding heap page, and taking
+ * no further lwlocks while they are locked.
+ *
  * We normally use FSM to help us find free space.  However,
  * if HEAP_INSERT_SKIP_FSM is specified, we just append a new empty page to
  * the end of the relation if the tuple won't fit on the current target page.

diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 8bed6edc76e498a351883cce2b9e76a330c5660d..5a0511f1988cf89cf734fbe2d71bab15f2eaa4b4 100644 (file)
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -45,9 +45,19 @@
  *
  * In heapam.c, whenever a page is modified so that not all tuples on the
  * page are visible to everyone anymore, the corresponding bit in the
- * visibility map is cleared. The bit in the visibility map is cleared
- * after releasing the lock on the heap page, to avoid holding the lock
- * over possible I/O to read in the visibility map page.
+ * visibility map is cleared. In order to be crash-safe, we need to do this
+ * while still holding a lock on the heap page and in the same critical
+ * section that logs the page modification. However, we don't want to hold
+ * the buffer lock over any I/O that may be required to read in the visibility
+ * map page.  To avoid this, we examine the heap page before locking it;
+ * if the page-level PD_ALL_VISIBLE bit is set, we pin the visibility map
+ * bit.  Then, we lock the buffer.  But this creates a race condition: there
+ * is a possibility that in the time it takes to lock the buffer, the
+ * PD_ALL_VISIBLE bit gets set.  If that happens, we have to unlock the
+ * buffer, pin the visibility map page, and relock the buffer.  This shouldn't
+ * happen often, because only VACUUM currently sets visibility map bits,
+ * and the race will only occur if VACUUM processes a given page at almost
+ * exactly the same time that someone tries to further modify it.
  *
  * To set a bit, you need to hold a lock on the heap page. That prevents
  * the race condition where VACUUM sees that all tuples on the page are