Wrap multixact/members correctly during extension, take 2

In a50d97625497b7 I already changed this, but got it wrong for the case
where the number of members is larger than the number of entries that
fit in the last page of the last segment.

As reported by Serge Negodyuck in a followup to bug #8673.

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 8886ce4c672edaffce5dee9b590e014db76202f7..4f97c8ae9784503f99e2fcca48490de3ce35b6eb 100644 (file)
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -133,6 +133,19 @@
 #define MULTIXACT_MEMBERS_PER_PAGE \
    (MULTIXACT_MEMBERGROUPS_PER_PAGE * MULTIXACT_MEMBERS_PER_MEMBERGROUP)
 
+/*
+ * Because the number of items per page is not a divisor of the last item
+ * number (member 0xFFFFFFFF), the last segment does not use the maximum number
+ * of pages, and moreover the last used page therein does not use the same
+ * number of items as previous pages.  (Another way to say it is that the
+ * 0xFFFFFFFF member is somewhere in the middle of the last page, so the page
+ * has some empty space after that item.)
+ *
+ * This constant is the number of members in the last page of the last segment.
+ */
+#define MAX_MEMBERS_IN_LAST_MEMBERS_PAGE \
+       ((uint32) ((0xFFFFFFFF % MULTIXACT_MEMBERS_PER_PAGE) + 1))
+
 /* page in which a member is to be found */
 #define MXOffsetToMemberPage(xid) ((xid) / (TransactionId) MULTIXACT_MEMBERS_PER_PAGE)
 
@@ -2278,6 +2291,7 @@ ExtendMultiXactMember(MultiXactOffset offset, int nmembers)
    {
        int         flagsoff;
        int         flagsbit;
+       uint32      difference;
 
        /*
         * Only zero when at first entry of a page.
@@ -2299,24 +2313,29 @@ ExtendMultiXactMember(MultiXactOffset offset, int nmembers)
        }
 
        /*
-        * Advance to next page, taking care to properly handle the wraparound
-        * case.  OK if nmembers goes negative.
+        * Compute the number of items till end of current page.  Careful: if
+        * addition of unsigned ints wraps around, we're at the last page of
+        * the last segment; since that page holds a different number of items
+        * than other pages, we need to do it differently.
         */
-       if ((unsigned int) (offset + nmembers) < offset)
+       if (offset + MAX_MEMBERS_IN_LAST_MEMBERS_PAGE < offset)
        {
-           uint32      difference = offset + MULTIXACT_MEMBERS_PER_PAGE;
-
-           nmembers -= (unsigned int) (MULTIXACT_MEMBERS_PER_PAGE - difference);
-           offset = 0;
+           /*
+            * This is the last page of the last segment; we can compute the
+            * number of items left to allocate in it without modulo
+            * arithmetic.
+            */
+           difference = MaxMultiXactOffset - offset + 1;
        }
        else
-       {
-           int         difference;
-
            difference = MULTIXACT_MEMBERS_PER_PAGE - offset % MULTIXACT_MEMBERS_PER_PAGE;
-           nmembers -= difference;
-           offset += difference;
-       }
+
+       /*
+        * Advance to next page, taking care to properly handle the wraparound
+        * case.  OK if nmembers goes negative.
+        */
+       nmembers -= difference;
+       offset += difference;
    }
 }
 

diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
index b9681090c060418fadc2538bd29b93db8b924ad0..62a75ea212183e4ac9a95705aaaa4f41dc62d364 100644 (file)
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -25,6 +25,8 @@
 
 #define MultiXactIdIsValid(multi) ((multi) != InvalidMultiXactId)
 
+#define MaxMultiXactOffset ((MultiXactOffset) 0xFFFFFFFF)
+
 /* Number of SLRU buffers to use for multixact */
 #define NUM_MXACTOFFSET_BUFFERS        8
 #define NUM_MXACTMEMBER_BUFFERS        16