public inbox for [email protected]  
help / color / mirror / Atom feed
From: Ashutosh Bapat <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Maxim Orlov <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: wenhui qiu <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: POC: make mxidoff 64 bits
Date: Mon, 8 Dec 2025 21:13:00 +0530
Message-ID: <CAExHW5s_uNeD_xYjdbSR8khMXwWJQOY9Qg=j4T+5KOfGz5-RsQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CACG=ezaWg7_nt-8ey4aKv2w9LcuLthHknwCawmBgEeTnJrJTcw@mail.gmail.com>
	<[email protected]>
	<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>
	<[email protected]>
	<[email protected]>
	<CACG=ezYUJSvnuxntkURNWo_1vZ+AtmcQfqd_h6WgDzGaudfw+Q@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAExHW5tUEkiQrvm9hgccjKUNkWBnJ5_HDUrAwiHBTxu+Vuj29Q@mail.gmail.com>
	<[email protected]>
	<CAExHW5t3kzJiVqmoqCLyGmfkTjD4Rwa27kXH-S_XvHWLkM2fzw@mail.gmail.com>
	<CAExHW5ucnoyjd6p7UVVhQTeV7hc8-vX81ti8f7sU0COqfUWzQg@mail.gmail.com>
	<[email protected]>

On Sat, Dec 6, 2025 at 5:06 AM Heikki Linnakangas <[email protected]> wrote:
> On 05/12/2025 15:42, Ashutosh Bapat wrote:

>
> > 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().
>
> Nice! That log bloat was the reason I bundled together the "COMMIT;
> BEGIN; SELECT ...;" steps into one statement in the loop. Your solution
> addresses it more directly.

Now we can call query_safe() separately on each of those. That will be
more readable and marginally less code.

>
> I turned 'verbose' into a keyword parameter, for future extensibility of
> those functions, so you now call it like "$node->query_safe("SELECT 1",
> verbose => 0);". I also set "log_statements=none" in those connections,
> to reduce the noise in the server log too.

keyword parameter is better. also +1 for log_statements.

>
> > 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?
>
> +1. I renamed multixact_old.c to multixact_pre_v19.c. And
> multixact_new.c to multixact_rewrite.c. I also moved the
> "convert_multixact" function that drives the conversion to
> multixact_rewrite.c. The idea is that if in the future we change the
> format again, we will have:
>
> multixact_pre_v19.c  # for reading -v19 files
> multixact_pre_v24.c  # for reading v19-v23 files
> multixact_rewrite.c  # for writing new files
>
> Hard to predict what a possible future format might look like and how
> we'd want to organize the code then, though. This can be changed then if
> needed, but it makes sense now.

+1.

>
> > +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?
>
> Fair. On the other hand, having the same names makes it easier to see
> what the real differences with the server functions are. Not sure what's
> best here..
>
> As long as we use the same names, it's important that
> multixact_pre_v19.c doesn't #include the new definitions. I added some
> comments on that, and also this safeguard:
>
> #define MultiXactOffset should_not_be_used
>
> That actually caught one (harmless) instance in the file where we had
> not renamed MultiXactOffset to OldMultiXactOffset.
>

That looks useful, and has proved to be useful already.

> I'm not entirely happy with the "Old" prefix here, because as you
> pointed out, we might end up needing "older" or "oldold" in the future.
> I couldn't come up with anything better though. "PreV19MultiXactOffset"
> is quite a mouthful.

How about MultiXactOffset32?

Thanks for addressing rest of the comments.

>
> > +
> > + 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?
>
> Got something in mind that we could check?
>

I have always seen that finish_mxoff is very high compared to newnode
mxoff - given that we write only one member per mxid, is newnode mxoff
going to be always something like 4K or so? Then we can check that
value. But I will experiment more to see if I can come up with
something, if possible.

-- 
Best Wishes,
Ashutosh Bapat





view thread (79+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: POC: make mxidoff 64 bits
  In-Reply-To: <CAExHW5s_uNeD_xYjdbSR8khMXwWJQOY9Qg=j4T+5KOfGz5-RsQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox