Обсуждение: POC: make mxidoff 64 bits
Hi!
I've been trying to introduce 64-bit transaction identifications to Postgres for quite a while [0]. All this implies,
of course, an enormous amount of change that will have to take place in various modules. Consider this, the
patch set become too big to be committed “at once”.
The obvious solutions was to try to split the patch set into smaller ones. But here it comes a new challenge,
not every one of these parts, make Postgres better at the moment. Actually, even switching to a
FullTransactionId in PGPROC will have disadvantage in increasing of WAL size [1].
In fact, I believe, we're dealing with the chicken and the egg problem here. Not able to commit full patch set
since it is too big to handle and not able to commit parts of it, since they make sense all together and do not
help improve Postgres at the moment.
But it's not that bad. Since the commit 4ed8f0913bfdb5f, added in [3], we are capable to use 64 bits to
indexing SLRUs.
PROPOSAL
Make multixact offsets 64 bit.
RATIONALE
It is not very difficult to overflow 32-bit mxidoff. Since, it is created for every unique combination of the
It is not very difficult to overflow 32-bit mxidoff. Since, it is created for every unique combination of the
transaction for each tuple, including XIDs and respective flags. And when a transaction is added to a
specific multixact, it is rewritten with a new offset. In other words, it is possible to overflow the offsets of
multixacts without overflowing the multixacts themselves and/or ordinary transactions. I believe, there
was something about in the hackers mail lists, but I could not find links now.
PFA, patch. Here is a WIP version. Upgrade machinery should be added later.
As always, any opinions on a subject a very welcome!
--
Best regards,
Maxim Orlov.
Вложения
On 23/04/2024 11:23, Maxim Orlov wrote: > PROPOSAL > Make multixact offsets 64 bit. +1, this is a good next step and useful regardless of 64-bit XIDs. > @@ -156,7 +148,7 @@ > ((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) > +#define MXOffsetToMemberPage(xid) ((xid) / (MultiXactOffset) MULTIXACT_MEMBERS_PER_PAGE) > #define MXOffsetToMemberSegment(xid) (MXOffsetToMemberPage(xid) / SLRU_PAGES_PER_SEGMENT) > > /* Location (byte offset within page) of flag word for a given member */ This is really a bug fix. It didn't matter when TransactionId and MultiXactOffset were both typedefs of uint32, but it was always wrong. The argument name 'xid' is also misleading. I think there are some more like that, MXOffsetToFlagsBitShift for example. -- Heikki Linnakangas Neon (https://neon.tech)
> On 23 Apr 2024, at 11:23, Maxim Orlov <orlovmg@gmail.com> wrote:
>
> Make multixact offsets 64 bit.
- ereport(ERROR,
- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("multixact \"members\" limit exceeded"),
Personally, I'd be happy with this! We had some incidents where the only mitigation was vacuum settings tweaking.
BTW as a side note... I see lot's of casts to (unsigned long long), can't we just cast to MultiXactOffset?
Best regards, Andrey Borodin.
Hi Maxim Orlov
Thank you so much for your tireless work on this. Increasing the WAL size by a few bytes should have very little impact with today's disk performance(Logical replication of this feature wal log is also increased a lot, logical replication is a milestone new feature, and the community has been improving the logical replication of functions),I believe removing troubled postgresql Transaction ID Wraparound was also a milestone new feature adding a few bytes is worth it!
Best regards
Thank you so much for your tireless work on this. Increasing the WAL size by a few bytes should have very little impact with today's disk performance(Logical replication of this feature wal log is also increased a lot, logical replication is a milestone new feature, and the community has been improving the logical replication of functions),I believe removing troubled postgresql Transaction ID Wraparound was also a milestone new feature adding a few bytes is worth it!
Best regards
On Tue, 23 Apr 2024 at 17:37, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 23/04/2024 11:23, Maxim Orlov wrote:
> PROPOSAL
> Make multixact offsets 64 bit.
+1, this is a good next step and useful regardless of 64-bit XIDs.
> @@ -156,7 +148,7 @@
> ((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)
> +#define MXOffsetToMemberPage(xid) ((xid) / (MultiXactOffset) MULTIXACT_MEMBERS_PER_PAGE)
> #define MXOffsetToMemberSegment(xid) (MXOffsetToMemberPage(xid) / SLRU_PAGES_PER_SEGMENT)
>
> /* Location (byte offset within page) of flag word for a given member */
This is really a bug fix. It didn't matter when TransactionId and
MultiXactOffset were both typedefs of uint32, but it was always wrong.
The argument name 'xid' is also misleading.
I think there are some more like that, MXOffsetToFlagsBitShift for example.
--
Heikki Linnakangas
Neon (https://neon.tech)
On Tue, 23 Apr 2024 at 12:37, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
This is really a bug fix. It didn't matter when TransactionId and
MultiXactOffset were both typedefs of uint32, but it was always wrong.
The argument name 'xid' is also misleading.
I think there are some more like that, MXOffsetToFlagsBitShift for example.
Yeah, I always thought so too. I believe, this is just a copy-paste. You mean, it is worth creating a separate CF
entry for these fixes?
On Tue, 23 Apr 2024 at 16:03, Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
BTW as a side note... I see lot's of casts to (unsigned long long), can't we just cast to MultiXactOffset?Actually, first versions of the 64xid patch set have such a cast to types TransactionID, MultiXact and so on. But,
after some discussions, we are switched to unsigned long long cast. Unfortunately, I could not find an exact link
for that discussion. On the other hand, such a casting is already used throughout the code. So, just for the
sake of the consistency, I would like to stay with these casts.
On Tue, 23 Apr 2024 at 16:03, wenhui qiu <qiuwenhuifx@gmail.com> wrote:
Hi Maxim Orlov
Thank you so much for your tireless work on this. Increasing the WAL size by a few bytes should have very little impact with today's disk performance(Logical replication of this feature wal log is also increased a lot, logical replication is a milestone new feature, and the community has been improving the logical replication of functions),I believe removing troubled postgresql Transaction ID Wraparound was also a milestone new feature adding a few bytes is worth it!
I'm 100% agree. Maybe, I should return to this approach and find some benefits for having FXIDs in WAL.