public inbox for [email protected]
help / color / mirror / Atom feedFrom: Yura Sokolov <[email protected]>
To: Peter Geoghegan <[email protected]>
To: Maxim Orlov <[email protected]>
Cc: Anton A. Melnikov <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Maybe don't process multi xmax in FreezeMultiXactId() if it is already marked as invalid?
Date: Wed, 19 Jun 2024 20:00:28 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH2-WznwPKvmTv_Mo=yOfngcjX-1aus7VhjYWw9LTA5pF4o=yg@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CACG=ezbB2q2wYZseBA5mcNKXZwamPRb2dzzqrRtJ37E=K7do2w@mail.gmail.com>
<CAH2-WznwPKvmTv_Mo=yOfngcjX-1aus7VhjYWw9LTA5pF4o=yg@mail.gmail.com>
18.06.2024 18:47, Peter Geoghegan пишет:
> On Tue, Jun 18, 2024 at 10:29 AM Maxim Orlov <[email protected]> wrote:
>> Maybe, I'm too bold, but looks like a kinda bug to me. At least, I don't understand why we do not check the HEAP_XMAX_INVALID flag.
>> My guess is nobody noticed, that MultiXactIdIsValid call does not check the mentioned flag in the "first" condition, but it's all my speculation.
>
> A related code path was changed in commit 02d647bbf0. That change made
> the similar xmax handling that covers XIDs (not MXIDs) *stop* doing
> what you're now proposing to do in the Multi path.
I don't agree commit 02d647bbf0 is similar to suggested change.
Commit 02d647bbf0 fixed decision to set
freeze_xmax = false;
xmax_already_frozen = true;
Suggested change is for decision to set
*flags |= FRM_INVALIDATE_XMAX;
pagefrz->freeze_required = true;
Which leads to
freeze_xmax = true;
So it is quite different code paths, and one could not be used
to decline or justify other.
More over, certainly test on HEAP_XMAX_INVALID could be used
there in heap_prepare_freeze_tuple to set
freeze_xmax = true;
Why didn't you do it?
>
> Why do you think this is a bug?
It is not a bug per se.
But:
- it is missed opportunity for optimization,
- it is inconsistency in data handling.
Inconsistency leads to bugs when people attempt to modify code.
Yes, we changed completely different place mistakenly relying on
consistent reaction on this "hint", and that leads to bug in our
patch.
>> Does anyone know if there are reasons to deliberately ignore the HEAP_XMAX INVALID flag? Or this is just an unfortunate oversight.
>
> HEAP_XMAX_INVALID is just a hint.
>
WTF is "just a hint"?
I thought, hint is "yep, you can ignore it. But we already did some job
and stored its result as this hint. And if you don't ignore this hint,
then you can skip doing the job we did already".
So every time we ignore hint, we miss opportunity for optimization.
Why the hell we shouldn't optimize when we safely can?
If we couldn't rely on hint, then hint is completely meaningless.
--------
have a nice day
Yura Sokolov
view thread (7+ 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]
Subject: Re: Maybe don't process multi xmax in FreezeMultiXactId() if it is already marked as invalid?
In-Reply-To: <[email protected]>
* 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