MIME-Version: 1.0
References: 
 <CACG=ezaWg7_nt-8ey4aKv2w9LcuLthHknwCawmBgEeTnJrJTcw@mail.gmail.com>
 <CACG=ezajc_Pcqmy6fcq-N8+LzCRMzOzJzez2_BgHEu-6RVJtKQ@mail.gmail.com>
 <4535f3aa-3220-4760-b1f5-2bc91f248e03@iki.fi>
 <CACG=ezbKwypBp=14q9+hMQApus3=1hKxJ9x1+KinUhtT48570Q@mail.gmail.com>
 <2bc58592-9d74-4af0-bdd1-1a88e8683f7c@iki.fi>
 <CACG=ezZwdvsijzuXE3hex3xHcoz75EQYBXRTsQJVwbo5J5sS3g@mail.gmail.com>
 <CACG=ezbs912S58=uR17b4w8uuWv1=OcCRaTW_OWdFm4+tXZA6w@mail.gmail.com>
 <CAGjGUA+BfcWyccNN4=tHsW_E-koRxbg8h8ut6hjvPsHMgmek6w@mail.gmail.com>
 <CACG=ezYbYO_KHWdeDedbDcY0tOS0JfaqBxG3=bG5+DdsDK4MpQ@mail.gmail.com>
 <CACG=ezYpZRPwoRCz_h3Qerd3XJNdpTHCpwGbZphNdy26tA4_qQ@mail.gmail.com>
 <36531c0e-292c-409d-bbc7-a252cf6e910a@iki.fi>
 <54aa8f65-f0e4-4464-b543-e0399c1cab1e@iki.fi>
 <CACG=ezYUJSvnuxntkURNWo_1vZ+AtmcQfqd_h6WgDzGaudfw+Q@mail.gmail.com>
 <4a9dda70-0af7-41a4-9636-b168f2fc48ef@iki.fi>
 <46cc45e9-fddd-44bc-bcb3-96889aafd921@iki.fi>
 <CAExHW5tUEkiQrvm9hgccjKUNkWBnJ5_HDUrAwiHBTxu+Vuj29Q@mail.gmail.com>
 <6c298bc4-7029-4c1d-bf16-3e094842ce32@iki.fi>
In-Reply-To: <6c298bc4-7029-4c1d-bf16-3e094842ce32@iki.fi>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Fri, 28 Nov 2025 18:35:44 +0530
Message-ID: 
 <CAExHW5t3kzJiVqmoqCLyGmfkTjD4Rwa27kXH-S_XvHWLkM2fzw@mail.gmail.com>
Subject: Re: POC: make mxidoff 64 bits
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Maxim Orlov <orlovmg@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Alexander Korotkov <aekorotkov@gmail.com>,
 wenhui qiu <qiuwenhuifx@gmail.com>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000593b5b0644a74a76"
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5t3kzJiVqmoqCLyGmfkTjD4Rwa27kXH-S_XvHWLkM2fzw%40mail.gmail.com>
Precedence: bulk

--000000000000593b5b0644a74a76
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Nov 21, 2025 at 7:26=E2=80=AFPM Heikki Linnakangas <hlinnaka@iki.fi=
> wrote:
>
>
> > I have reviewed patch 0002 and multxact.c changes in 0003. So far I
> > have only these comments. I will review the pg_upgrade.c changes next.

007_multixact_conversion.pl fires thousands of queries through
BackgroundPsql which prints debug output for each of the queries. When
running this file with oldinstall set,
2.2M  regress_log_007_multixact_conversion (size of file)
77874 regress_log_007_multixact_conversion (wc -l output)

Since this output is also copied in testlog.txt, the effect is two-fold.

Most, if not all, of this output is useless. It also makes it hard to
find the output we are looking for. PFA patch which reduces this
output. The patch adds a flag verbose to query_safe() and query() to
toggle this output. With the patch the sizes are
27K  regress_log_007_multixact_conversion
588 regress_log_007_multixact_conversion

And it makes the test faster by about a second or two on my laptop.
Something on those lines or other is required to reduce the output
from query_safe().

Some more comments
+++ b/src/bin/pg_upgrade/multixact_old.c

We may need to introduce new _new and then _old will become _older.
Should we rename the files to have pre19 and post19 or some similar
suffixes which make it clear what is meant by old and new?

+
+static inline int64
+MultiXactIdToOffsetPage(MultiXactId multi)

The prologue mentions that the definitions are copy-pasted from
multixact.c from version 18, but they share the names with functions
in the current version. I think that's going to be a good source of
confusion especially in a file which is a few hundred lines long. Can
we rename them to have "Old" prefix or something similar?

+
+# Dump contents of the 'mxofftest' table, created by mxact_workload
+sub get_dump_for_comparison

This local function shares its name with a local function in
002_pg_upgrade.pl. Better to use a separate name. Also it's not
"dumping" data using "pg_dump", so "dump" in the name can be
misleading.

+ $newnode->start;
+ my $new_dump =3D get_dump_for_comparison($newnode, "newnode_${tag}_dump")=
;
+ $newnode->stop;

There is no code which actually looks at the multixact offsets here to
make sure that the conversion happened correctly. I guess the test
relies on visibility checks for that. Anyway, we need a comment
explaining why just comparing the contents of the table is enough to
ensure correct conversion. Better if we can add an explicit test that
the offsets were converted correctly. I don't have any idea of how to
do that right now, though. Maybe use pg_get_multixact_members()
somehow in the query to extract data out of the table?

+
+ compare_files($old_dump, $new_dump,
+ 'dump outputs from original and restored regression databases match');

A shared test name too :); but there is not regression database here.

+
+ note ">>> case #${tag}\n"
+   . " oldnode mxoff from ${start_mxoff} to ${finish_mxoff}\n"
+   . " newnode mxoff ${new_next_mxoff}\n";

Should we check that some condition holds between finish_mxoff and
new_next_mxoff?

I will continue reviewing it further.


--
Best Wishes,
Ashutosh Bapat

--000000000000593b5b0644a74a76
Content-Type: application/octet-stream;
	name="reduce_testoutput.patch.nocibot"
Content-Disposition: attachment; filename="reduce_testoutput.patch.nocibot"
Content-Transfer-Encoding: base64
Content-ID: <f_miim3how0>
X-Attachment-Id: f_miim3how0

ZGlmZiAtLWdpdCBhL3NyYy9iaW4vcGdfdXBncmFkZS90LzAwN19tdWx0aXhhY3RfY29udmVyc2lv
bi5wbCBiL3NyYy9iaW4vcGdfdXBncmFkZS90LzAwN19tdWx0aXhhY3RfY29udmVyc2lvbi5wbApp
bmRleCBmZThkYTlhZGVkMi4uMTQxMzNlYjNjN2EgMTAwNjQ0Ci0tLSBhL3NyYy9iaW4vcGdfdXBn
cmFkZS90LzAwN19tdWx0aXhhY3RfY29udmVyc2lvbi5wbAorKysgYi9zcmMvYmluL3BnX3VwZ3Jh
ZGUvdC8wMDdfbXVsdGl4YWN0X2NvbnZlcnNpb24ucGwKQEAgLTY4LDggKzY4LDggQEAgc3ViIG14
YWN0X3dvcmtsb2FkCiAJCSMgdmVyc2lvbnMuCiAJCW15ICRjb25uID0gJGJpbm5vZGUtPmJhY2tn
cm91bmRfcHNxbCgnJywKIAkJCWNvbm5zdHIgPT4gJG5vZGUtPmNvbm5zdHIoJ3Bvc3RncmVzJykp
OwotCQkkY29ubi0+cXVlcnlfc2FmZSgiU0VUIGVuYWJsZV9zZXFzY2FuPW9mZiIpOwotCQkkY29u
bi0+cXVlcnlfc2FmZSgiQkVHSU4iKTsKKwkJJGNvbm4tPnF1ZXJ5X3NhZmUoIlNFVCBlbmFibGVf
c2Vxc2Nhbj1vZmYiLCAwKTsKKwkJJGNvbm4tPnF1ZXJ5X3NhZmUoIkJFR0lOIiwgMCk7CiAKIAkJ
cHVzaChAY29ubmVjdGlvbnMsICRjb25uKTsKIAl9CkBAIC0xMTAsNyArMTEwLDcgQEAgc3ViIG14
YWN0X3dvcmtsb2FkCiAJCQkgICkgYXMgeAogCQkJXTsKIAkJfQotCQkkY29ubi0+cXVlcnlfc2Fm
ZSgkc3FsKTsKKwkJJGNvbm4tPnF1ZXJ5X3NhZmUoJHNxbCwgMCk7CiAJfQogCiAJZm9yIG15ICRj
b25uIChAY29ubmVjdGlvbnMpCmRpZmYgLS1naXQgYS9zcmMvdGVzdC9wZXJsL1Bvc3RncmVTUUwv
VGVzdC9CYWNrZ3JvdW5kUHNxbC5wbSBiL3NyYy90ZXN0L3BlcmwvUG9zdGdyZVNRTC9UZXN0L0Jh
Y2tncm91bmRQc3FsLnBtCmluZGV4IDYwYmJkNWRkNDQ1Li45ZDYyYzdhMDBjMCAxMDA2NDQKLS0t
IGEvc3JjL3Rlc3QvcGVybC9Qb3N0Z3JlU1FML1Rlc3QvQmFja2dyb3VuZFBzcWwucG0KKysrIGIv
c3JjL3Rlc3QvcGVybC9Qb3N0Z3JlU1FML1Rlc3QvQmFja2dyb3VuZFBzcWwucG0KQEAgLTIyOCwy
MCArMjI4LDI0IEBAIHN1YiByZWNvbm5lY3RfYW5kX2NsZWFyCiAKIEV4ZWN1dGVzIGEgcXVlcnkg
aW4gdGhlIGN1cnJlbnQgc2Vzc2lvbiBhbmQgcmV0dXJucyB0aGUgb3V0cHV0IGluIHNjYWxhcgog
Y29udGV4dCBhbmQgKG91dHB1dCwgZXJyb3IpIGluIGxpc3QgY29udGV4dCB3aGVyZSBlcnJvciBp
cyAxIGluIGNhc2UgdGhlcmUKLXdhcyBvdXRwdXQgZ2VuZXJhdGVkIG9uIHN0ZGVyciB3aGVuIGV4
ZWN1dGluZyB0aGUgcXVlcnkuCit3YXMgb3V0cHV0IGdlbmVyYXRlZCBvbiBzdGRlcnIgd2hlbiBl
eGVjdXRpbmcgdGhlIHF1ZXJ5LiBJZiBDPHZlcmJvc2U+IGlzCit0cnVlIChkZWZhdWx0KSB0aGUg
cXVlcnkgYW5kIGl0cyByZXN1bHRzIGFyZSBwcmludGVkIHRvIHRoZSB0ZXN0IG91dHB1dC4KIAog
PWN1dAogCiBzdWIgcXVlcnkKIHsKLQlteSAoJHNlbGYsICRxdWVyeSkgPSBAXzsKKwlteSAoJHNl
bGYsICRxdWVyeSwgJHZlcmJvc2UpID0gQF87CiAJbXkgJHJldDsKIAlteSAkb3V0cHV0OwogCW15
ICRxdWVyeV9jbnQgPSAkc2VsZi0+e3F1ZXJ5X2NudH0rKzsKIAorCSMgU2V0ICR2ZXJib3NlIHRv
IHRydWUgaWYgbm90IHBhc3NlZAorCSR2ZXJib3NlID0gMSB1bmxlc3MgZGVmaW5lZCgkdmVyYm9z
ZSk7CisKIAlsb2NhbCAkVGVzdDo6QnVpbGRlcjo6TGV2ZWwgPSAkVGVzdDo6QnVpbGRlcjo6TGV2
ZWwgKyAxOwogCi0Jbm90ZSAiaXNzdWluZyBxdWVyeSAkcXVlcnlfY250IHZpYSBiYWNrZ3JvdW5k
IHBzcWw6ICRxdWVyeSI7CisJbm90ZSAiaXNzdWluZyBxdWVyeSAkcXVlcnlfY250IHZpYSBiYWNr
Z3JvdW5kIHBzcWw6ICRxdWVyeSIgdW5sZXNzICEkdmVyYm9zZTsKIAogCSRzZWxmLT57dGltZW91
dH0tPnN0YXJ0KCkgaWYgKGRlZmluZWQoJHNlbGYtPntxdWVyeV90aW1lcl9yZXN0YXJ0fSkpOwog
CkBAIC0yODAsNyArMjg0LDcgQEAgc3ViIHF1ZXJ5CiAJICBleHBsYWluIHsKIAkJc3Rkb3V0ID0+
ICRzZWxmLT57c3Rkb3V0fSwKIAkJc3RkZXJyID0+ICRzZWxmLT57c3RkZXJyfSwKLQkgIH07CisJ
ICB9IHVubGVzcyAhJHZlcmJvc2U7CiAKIAkjIFJlbW92ZSBiYW5uZXIgZnJvbSBzdGRvdXQgYW5k
IHN0ZGVyciwgb3VyIGNhbGxlciBkb2Vzbid0IGNhcmUuICBUaGUKIAkjIGZpcnN0IG5ld2xpbmUg
aXMgb3B0aW9uYWwsIGFzIHRoZXJlIHdvdWxkIG5vdCBiZSBvbmUgaWYgY29uc3VtaW5nIGFuCkBA
IC0zMDgsOSArMzEyLDkgQEAgUXVlcnkgZmFpbHVyZSBpcyBkZXRlcm1pbmVkIGJ5IGl0IHByb2R1
Y2luZyBvdXRwdXQgb24gc3RkZXJyLgogCiBzdWIgcXVlcnlfc2FmZQogewotCW15ICgkc2VsZiwg
JHF1ZXJ5KSA9IEBfOworCW15ICgkc2VsZiwgJHF1ZXJ5LCAkdmVyYm9zZSkgPSBAXzsKIAotCW15
ICRyZXQgPSAkc2VsZi0+cXVlcnkoJHF1ZXJ5KTsKKwlteSAkcmV0ID0gJHNlbGYtPnF1ZXJ5KCRx
dWVyeSwgJHZlcmJvc2UpOwogCiAJaWYgKCRzZWxmLT57c3RkZXJyfSBuZSAiIikKIAl7Cg==
--000000000000593b5b0644a74a76--