MIME-Version: 1.0
References: <CACG=ezaWg7_nt-8ey4aKv2w9LcuLthHknwCawmBgEeTnJrJTcw@mail.gmail.com>
 <CACG=ezYokoiumOFnqUfg_ffHD5s8T+6iHYfzKLfa=QQ-1pNrBg@mail.gmail.com>
 <CACG=ezY9xq73jcX_EjVqx5-f90nbQ9PyhFCTW2fwFCS2wmNiFw@mail.gmail.com>
 <CACG=eza+27CfLBobJJccRhXrA3He6c1irAnoyTtSC1-z9UXLrg@mail.gmail.com>
 <CAPpHfduczcop9s6gKUpLGgFUe2y4ERGMJx6SS6Kp+s-kQPwMjg@mail.gmail.com>
 <CACG=ezbye4g_ERNqE=gBcvQ0YypRaVENhNUu8xrs4PL12UdnUA@mail.gmail.com>
 <CACG=ezaMncd0-BcGHBgsSR2eqHfrz9WznHGLKX8biz6zu-azGw@mail.gmail.com>
 <5ecc35f5-1111-47fc-8a02-36d89490a50d@iki.fi> <CACG=ezb9XTvd3ZmS0y8gUunx_wBBdJO7ou+BfCOnnA5jE-11vg@mail.gmail.com>
 <CACG=ezYFNqGjsxF6Vb2CHF6JzKcjhAFauaFm9js0nu_3Ngcdkw@mail.gmail.com>
 <CAGjGUA+dcV7veaCV1H65vCNsbS++nT8=ho772gDvsXUW9H7eXQ@mail.gmail.com>
 <CACG=ezYThNkf8QsDA-aQfEFEkqn2L=_uUL83z0vJstPRasbZqg@mail.gmail.com>
 <24b3deb6-a732-4256-847a-560f4bf39d59@iki.fi> <CACG=ezYtCatcRODS-ZkwhcxuqBKCuhEsZGBruw=dGCLoepF+ZA@mail.gmail.com>
 <ca451a6a-5b0f-433c-8ab9-b28b7f480bd7@iki.fi>
In-Reply-To: <ca451a6a-5b0f-433c-8ab9-b28b7f480bd7@iki.fi>
From: Maxim Orlov <orlovmg@gmail.com>
Date: Fri, 15 Nov 2024 19:19:07 +0300
Message-ID: <CACG=ezb680eb=JXh1ns=t5eGH3h9y-uTfT4tf3Xc8t2UH2q6tQ@mail.gmail.com>
Subject: Re: POC: make mxidoff 64 bits
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: wenhui qiu <qiuwenhuifx@gmail.com>, Alexander Korotkov <aekorotkov@gmail.com>, 
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000f063850626f5edc1"
Archived-At: <https://www.postgresql.org/message-id/CACG%3Dezb680eb%3DJXh1ns%3Dt5eGH3h9y-uTfT4tf3Xc8t2UH2q6tQ%40mail.gmail.com>
Precedence: bulk

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

On Fri, 15 Nov 2024 at 14:06, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> Hmm, so if I understand correctly, this is related to how we determine
> the length of the members array, by looking at the next multixid's
> offset. This is explained in GetMultiXactIdMembers:
>
Correct.


> If we accept that, we don't need to worry about case 3 anymore. But if
> we upgrade wrapped-around members files by just renaming them, there
> could still be a members array where we had skipped offset 0, and
> reading that after the upgrade might get confused. We could continue to
> ignore a 0 XID in the members array like the comment says; I think that
> would be enough. But yeah, maybe it's better to bite the bullet in
> pg_upgrade and squeeze those out.
>
Correct. I couldn't explain this better. I'm more for the squeeze those
out. Overwise, we're ending up in adding another hack in multixact, but one
of the benefits from switching to 64-bits, it should make XID's logic more
straight forward. After all, mxact juggling in pg_upgrade is one time
inconvenience.


>
> Does your upgrade test suite include case 3, where the next multixact's
> offset is 1?
>
Not exactly.

simple
Latest checkpoint's NextMultiXactId:  119441
Latest checkpoint's NextMultiOffset:  5927049

offset-wrap
Latest checkpoint's NextMultiXactId:  119441
Latest checkpoint's NextMultiOffset:  5591183

multi-wrap
Latest checkpoint's NextMultiXactId:  82006
Latest checkpoint's NextMultiOffset:  7408811

offset-multi-wrap
Latest checkpoint's NextMultiXactId:  52146
Latest checkpoint's NextMultiOffset:  5591183

You want test case where NextMultiOffset will be 1?


> Can we remove MaybeExtendOffsetSlru() now? There are a bunch of other
> comments and checks that talk about binary-upgraded values too that we
> can hopefully clean up now.
>
Yes, technically we can. But this is kinda unrelated to the offsets and
will make the patch set significantly complicated, thus more complicated to
review and less likely to be committed. Again, I'm not opposing the idea,
I'm not sure if it is worth to do it right now.


>
> If we are to parse the member segments in detail in upgrade anyway, I'd
> be tempted to make some further changes / optimizations:
>
> -  You could leave out all locking XID members in upgrade, because
> they're not relevant after upgrade any more (all the XIDs will be
> committed or aborted and have released the locks; we require prepared
> transactions to be completed before upgrading too). It'd be enough to
> include actual UPDATE/DELETE XIDs.
>
> - The way we determine the length of the members array by looking at the
> next multixid's offset is a bit complicated. We could have one extra
> flag per XID in the members to indicate "this is the last member of this
> multixid". That could either to replace the current mechanism of looking
> at the next offset, or be just an additional cross-check.
>
> - Do we still like the "group" representation, with 4 bytes of flags
> followed by 4 XIDs? I wonder if it'd be better to just store 5 bytes per
> XID unaligned.
>
Not really. But I would leave it for next iteration - switching multi to 64
bit. I already have some drafts for this. In any case, we'll must do
adjustments in pg_upgrade again. My goal is to move towards 64 XIDs, but
with the small steps, and I plan changes in "group" representation in
combination with switching multi to 64 bit. This seems a bit more
appropriate in my view.

As for your optimization suggestions, I like them. I don=E2=80=99t against =
them,
but I=E2=80=99m afraid to disrupt the clarity of thought, especially since =
the
algorithm is not the simplest.

--=20
Best regards,
Maxim Orlov.

--000000000000f063850626f5edc1
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"></div><br><div class=3D"gmail_quote"><div=
 dir=3D"ltr" class=3D"gmail_attr">On Fri, 15 Nov 2024 at 14:06, Heikki Linn=
akangas &lt;<a href=3D"mailto:hlinnaka@iki.fi">hlinnaka@iki.fi</a>&gt; wrot=
e:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0=
.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hmm, so if I understand correctly, this is related to how we determine <br>
the length of the members array, by looking at the next multixid&#39;s <br>
offset. This is explained in GetMultiXactIdMembers:<br></blockquote><div>Co=
rrect.</div><div>=C2=A0<br></div><blockquote class=3D"gmail_quote" style=3D=
"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-le=
ft:1ex">
If we accept that, we don&#39;t need to worry about case 3 anymore. But if =
<br>
we upgrade wrapped-around members files by just renaming them, there <br>
could still be a members array where we had skipped offset 0, and <br>
reading that after the upgrade might get confused. We could continue to <br=
>
ignore a 0 XID in the members array like the comment says; I think that <br=
>
would be enough. But yeah, maybe it&#39;s better to bite the bullet in <br>
pg_upgrade and squeeze those out.<br></blockquote><div>Correct. I couldn=
9;t explain this better. I&#39;m more for the squeeze those out. Overwise, =
we&#39;re ending up in adding another hack in multixact, but one of the ben=
efits from switching to 64-bits, it should make XID&#39;s logic more straig=
ht forward. After all, mxact juggling in pg_upgrade is one time inconvenien=
ce.</div><div>=C2=A0<br></div><blockquote class=3D"gmail_quote" style=3D"ma=
rgin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:=
1ex">
<br>
Does your upgrade test suite include case 3, where the next multixact&#39;s=
 <br>
offset is 1?<br></blockquote><div>Not exactly.<br><br>simple<br>Latest chec=
kpoint&#39;s NextMultiXactId: =C2=A0119441<br>Latest checkpoint&#39;s NextM=
ultiOffset: =C2=A05927049<br><br>offset-wrap<br>Latest checkpoint&#39;s Nex=
tMultiXactId: =C2=A0119441<br>Latest checkpoint&#39;s NextMultiOffset: =C2=
=A05591183<br><br>multi-wrap<br>Latest checkpoint&#39;s NextMultiXactId: =
=C2=A082006<br>Latest checkpoint&#39;s NextMultiOffset: =C2=A07408811<br><b=
r>offset-multi-wrap<br>Latest checkpoint&#39;s NextMultiXactId: =C2=A052146=
<br>Latest checkpoint&#39;s NextMultiOffset: =C2=A05591183<br><br>You want =
test case where NextMultiOffset will be 1? <br></div><div>=C2=A0<br></div><=
blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l=
eft:1px solid rgb(204,204,204);padding-left:1ex">
Can we remove MaybeExtendOffsetSlru() now? There are a bunch of other <br>
comments and checks that talk about binary-upgraded values too that we <br>
can hopefully clean up now.<br></blockquote><div>Yes, technically we can. B=
ut this is kinda unrelated to the offsets and will make the patch set signi=
ficantly complicated, thus more complicated to review and less likely to be=
 committed. Again, I&#39;m not opposing the idea, I&#39;m not sure if it is=
 worth to do it right now.</div><div>=C2=A0<br></div><blockquote class=3D"g=
mail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204=
,204,204);padding-left:1ex">
<br>
If we are to parse the member segments in detail in upgrade anyway, I&#39;d=
 <br>
be tempted to make some further changes / optimizations:<br>
<br>
-=C2=A0 You could leave out all locking XID members in upgrade, because <br=
>
they&#39;re not relevant after upgrade any more (all the XIDs will be <br>
committed or aborted and have released the locks; we require prepared <br>
transactions to be completed before upgrading too). It&#39;d be enough to <=
br>
include actual UPDATE/DELETE XIDs.<br>
<br>
- The way we determine the length of the members array by looking at the <b=
r>
next multixid&#39;s offset is a bit complicated. We could have one extra <b=
r>
flag per XID in the members to indicate &quot;this is the last member of th=
is <br>
multixid&quot;. That could either to replace the current mechanism of looki=
ng <br>
at the next offset, or be just an additional cross-check.<br>
<br>
- Do we still like the &quot;group&quot; representation, with 4 bytes of fl=
ags <br>
followed by 4 XIDs? I wonder if it&#39;d be better to just store 5 bytes pe=
r <br>
XID unaligned.<br clear=3D"all"></blockquote><div>Not really. But I would l=
eave it for next iteration - switching multi to 64 bit. I already have some=
 drafts for this. In any case, we&#39;ll must do adjustments in pg_upgrade =
again. My goal is to move towards 64 XIDs, but with the small steps, and I =
plan changes in &quot;group&quot; representation in combination with switch=
ing multi to 64 bit. This seems a bit more appropriate in my view. <br></di=
v><div><br></div><div><span class=3D"gmail-HwtZe" lang=3D"en"><span class=
=3D"gmail-jCAhz gmail-ChMk0b"><span class=3D"gmail-ryNqvb">As for your opti=
mization suggestions, I like them. I don=E2=80=99t against them, but I=E2=
=80=99m afraid to disrupt the clarity of thought, especially since the algo=
rithm is not the simplest.</span></span></span></div></div><br><span class=
=3D"gmail_signature_prefix">-- </span><br><div dir=3D"ltr" class=3D"gmail_s=
ignature"><div dir=3D"ltr"><div>Best regards,<br></div><div>Maxim Orlov.</d=
iv></div></div></div>

--000000000000f063850626f5edc1--