Fix calculation of members_size in pg_get_multixact_stats()

pg_get_multixact_stats() uses members_size to report the amount of
storage used by the currently retained multixact members.  However,
MultiXactOffsetStorageSize() divided the member count by the number of
members per storage group before multiplying by the group size, so it
was rounding down its result and incorrectly reported zero when there
were few retained members.  The calculation is changed to calculate the
same based on the member count.

While on it, this fixes a different issue in the isolation test
multixact-stats.  Three fields were defined for checks related to the
oldest offset values, but were not used.  The offsets existed in an
older version of the patch than what has been committed.  These are
replaced by checks for members_size, checking the new calculation
formula.

Thinkos introduced in 97b101776ce2.

Author: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/819AC1B2-1A71-4244-B081-3ADD85D1725D@gmail.com

diff --git a/src/include/access/multixact_internal.h b/src/include/access/multixact_internal.h
index 82349ea0d32b9e55f28018470c403e9011cce47c..ba73b3c2e148c97aa7bf7b0b3ade7a92b330759f 100644 (file)
--- a/src/include/access/multixact_internal.h
+++ b/src/include/access/multixact_internal.h
@@ -126,9 +126,11 @@ static inline uint64
 MultiXactOffsetStorageSize(MultiXactOffset new_offset,
                                                   MultiXactOffset old_offset)
 {
+       uint64          size_per_member;
+
        Assert(new_offset >= old_offset);
-       return (uint64) ((new_offset - old_offset) / MULTIXACT_MEMBERS_PER_MEMBERGROUP) *
-               MULTIXACT_MEMBERGROUP_SIZE;
+       size_per_member = MULTIXACT_MEMBERGROUP_SIZE / MULTIXACT_MEMBERS_PER_MEMBERGROUP;
+       return (new_offset - old_offset) * size_per_member;
 }
 
 #endif                                                 /* MULTIXACT_INTERNAL_H */

diff --git a/src/test/isolation/expected/multixact-stats.out b/src/test/isolation/expected/multixact-stats.out
index 27a6510c4ad57ca55509cf80c5098e86578b5308..4685bde6d4c9b0c8bdad62c3fcb102a0a29b440c 100644 (file)
--- a/src/test/isolation/expected/multixact-stats.out
+++ b/src/test/isolation/expected/multixact-stats.out
@@ -3,7 +3,7 @@ Parsed test spec with 2 sessions
 starting permutation: snap0 s1_begin s1_lock snap1 s2_begin s2_lock snap2 check_while_pinned s1_commit s2_commit
 step snap0: 
   CREATE TEMP TABLE snap0 AS
-  SELECT num_mxids, num_members, oldest_multixact
+  SELECT num_mxids, num_members, members_size, oldest_multixact
   FROM pg_get_multixact_stats();
 
 step s1_begin: BEGIN;
@@ -15,7 +15,7 @@ step s1_lock: SELECT 1 FROM mxq WHERE id=1 FOR KEY SHARE;
 
 step snap1: 
   CREATE TEMP TABLE snap1 AS
-  SELECT num_mxids, num_members, oldest_multixact
+  SELECT num_mxids, num_members, members_size, oldest_multixact
   FROM pg_get_multixact_stats();
 
 step s2_begin: BEGIN;
@@ -27,7 +27,7 @@ step s2_lock: SELECT 1 FROM mxq WHERE id=1 FOR KEY SHARE;
 
 step snap2: 
   CREATE TEMP TABLE snap2 AS
-  SELECT num_mxids, num_members, oldest_multixact
+  SELECT num_mxids, num_members, members_size, oldest_multixact
   FROM pg_get_multixact_stats();
 
 step check_while_pinned: 
@@ -39,21 +39,22 @@ step check_while_pinned:
     ARRAY[
       'is_init_mxids',
       'is_init_members',
+      'is_init_members_size',
       'is_init_oldest_mxid',
-      'is_init_oldest_off',
       'is_oldest_mxid_nondec_01',
       'is_oldest_mxid_nondec_12',
-      'is_oldest_off_nondec_01',
-      'is_oldest_off_nondec_12',
       'is_members_increased_ge1',
       'is_mxids_nondec_01',
       'is_mxids_nondec_12',
       'is_members_nondec_01',
-      'is_members_nondec_12'
+      'is_members_nondec_12',
+      'is_msize_nondec_01',
+      'is_msize_nondec_12'
     ],
     ARRAY[
       (s2.num_mxids        IS NOT NULL),
       (s2.num_members      IS NOT NULL),
+      (s2.members_size     IS NOT NULL),
       (s2.oldest_multixact IS NOT NULL),
 
       (s1.oldest_multixact::text::bigint >= COALESCE(s0.oldest_multixact::text::bigint, 0)),
@@ -64,7 +65,9 @@ step check_while_pinned:
       (s1.num_mxids   >= COALESCE(s0.num_mxids,   0)),
       (s2.num_mxids   >= COALESCE(s1.num_mxids,   0)),
       (s1.num_members >= COALESCE(s0.num_members, 0)),
-      (s2.num_members >= COALESCE(s1.num_members, 0))
+      (s2.num_members >= COALESCE(s1.num_members, 0)),
+      (s1.members_size >= COALESCE(s0.members_size, 0)),
+      (s2.members_size >= COALESCE(s1.members_size, 0))
     ]
   ) AS r(assertion, ok);
 
@@ -72,17 +75,17 @@ assertion               |ok
 ------------------------+--
 is_init_mxids           |t 
 is_init_members         |t 
+is_init_members_size    |t 
 is_init_oldest_mxid     |t 
-is_init_oldest_off      |t 
 is_oldest_mxid_nondec_01|t 
 is_oldest_mxid_nondec_12|t 
-is_oldest_off_nondec_01 |t 
-is_oldest_off_nondec_12 |t 
 is_members_increased_ge1|t 
 is_mxids_nondec_01      |t 
-is_mxids_nondec_12      |  
-is_members_nondec_01    |  
-is_members_nondec_12    |  
+is_mxids_nondec_12      |t 
+is_members_nondec_01    |t 
+is_members_nondec_12    |t 
+is_msize_nondec_01      |t 
+is_msize_nondec_12      |t 
 (13 rows)
 
 step s1_commit: COMMIT;

diff --git a/src/test/isolation/specs/multixact-stats.spec b/src/test/isolation/specs/multixact-stats.spec
index 07d4b11be6dcc6bd4c2d1a74b78fee75aa59d0b1..b77c40885e64e3487022b37ef85ec3eaf73ac7c4 100644 (file)
--- a/src/test/isolation/specs/multixact-stats.spec
+++ b/src/test/isolation/specs/multixact-stats.spec
@@ -4,8 +4,10 @@
 # is pinned by two open transactions, we check some patterns that VACUUM and
 # FREEZE cannot violate:
 # 1) "members" increased by at least 1 when the second session locked the row.
-# 2) (num_mxids / num_members) not decreased compared to earlier snapshots.
-# 3) "oldest_*" fields never decreased.
+# 2) "members_size" reflects the storage used by the member entries.
+# 3) (num_mxids / num_members / members_size) not decreased compared to
+#    earlier snapshots.
+# 4) "oldest_*" fields never decreased.
 #
 # This test does not run checks after releasing locks, as freezing and/or
 # truncation may shrink the multixact ranges calculated.
@@ -39,14 +41,14 @@ step s2_commit { COMMIT; }
 # multixacts have not initialized yet.
 step snap0 {
   CREATE TEMP TABLE snap0 AS
-  SELECT num_mxids, num_members, oldest_multixact
+  SELECT num_mxids, num_members, members_size, oldest_multixact
   FROM pg_get_multixact_stats();
 }
 
 # Save multixact state after s1 has locked the row.
 step snap1 {
   CREATE TEMP TABLE snap1 AS
-  SELECT num_mxids, num_members, oldest_multixact
+  SELECT num_mxids, num_members, members_size, oldest_multixact
   FROM pg_get_multixact_stats();
 }
 
@@ -54,21 +56,24 @@ step snap1 {
 # a multixact with at least 2 members.
 step snap2 {
   CREATE TEMP TABLE snap2 AS
-  SELECT num_mxids, num_members, oldest_multixact
+  SELECT num_mxids, num_members, members_size, oldest_multixact
   FROM pg_get_multixact_stats();
 }
 
 # Pretty, deterministic key/value outputs based of boolean checks:
 #   is_init_mxids            : num_mxids not NULL
 #   is_init_members          : num_members not NULL
+#   is_init_members_size     : members_size not NULL
 #   is_init_oldest_mxid      : oldest_multixact not NULL
 #   is_oldest_mxid_nondec_01 : oldest_multixact not decreased (snap0->snap1)
-#   is_oldest_mxid_nondec_12 : oldest_multixact did not decreased (snap1->snap2)
+#   is_oldest_mxid_nondec_12 : oldest_multixact not decreased (snap1->snap2)
 #   is_members_increased_ge1 : members increased by at least 1 when s2 joined
 #   is_mxids_nondec_01       : num_mxids not decreased (snap0->snap1)
 #   is_mxids_nondec_12       : num_mxids not decreased (snap1->snap2)
 #   is_members_nondec_01     : num_members not decreased (snap0->snap1)
 #   is_members_nondec_12     : num_members not decreased (snap1->snap2)
+#   is_msize_nondec_01       : members_size not decreased (snap0->snap1)
+#   is_msize_nondec_12       : members_size not decreased (snap1->snap2)
 step check_while_pinned {
   SELECT r.assertion, r.ok
   FROM snap0 s0
@@ -78,21 +83,22 @@ step check_while_pinned {
     ARRAY[
       'is_init_mxids',
       'is_init_members',
+      'is_init_members_size',
       'is_init_oldest_mxid',
-      'is_init_oldest_off',
       'is_oldest_mxid_nondec_01',
       'is_oldest_mxid_nondec_12',
-      'is_oldest_off_nondec_01',
-      'is_oldest_off_nondec_12',
       'is_members_increased_ge1',
       'is_mxids_nondec_01',
       'is_mxids_nondec_12',
       'is_members_nondec_01',
-      'is_members_nondec_12'
+      'is_members_nondec_12',
+      'is_msize_nondec_01',
+      'is_msize_nondec_12'
     ],
     ARRAY[
       (s2.num_mxids        IS NOT NULL),
       (s2.num_members      IS NOT NULL),
+      (s2.members_size     IS NOT NULL),
       (s2.oldest_multixact IS NOT NULL),
 
       (s1.oldest_multixact::text::bigint >= COALESCE(s0.oldest_multixact::text::bigint, 0)),
@@ -103,7 +109,9 @@ step check_while_pinned {
       (s1.num_mxids   >= COALESCE(s0.num_mxids,   0)),
       (s2.num_mxids   >= COALESCE(s1.num_mxids,   0)),
       (s1.num_members >= COALESCE(s0.num_members, 0)),
-      (s2.num_members >= COALESCE(s1.num_members, 0))
+      (s2.num_members >= COALESCE(s1.num_members, 0)),
+      (s1.members_size >= COALESCE(s0.members_size, 0)),
+      (s2.members_size >= COALESCE(s1.members_size, 0))
     ]
   ) AS r(assertion, ok);
 }