Re: Maybe don't process multi xmax in FreezeMultiXactId() if it is already marked as invalid?

Поиск
Список
Период
Сортировка
От Yura Sokolov
Тема Re: Maybe don't process multi xmax in FreezeMultiXactId() if it is already marked as invalid?
Дата
Msg-id 0e3466eb-ea42-47a6-8662-ca94b95f5a66@postgrespro.ru
обсуждение исходный текст
Ответ на Re: Maybe don't process multi xmax in FreezeMultiXactId() if it is already marked as invalid?  (Peter Geoghegan <pg@bowt.ie>)
Ответы Re: Maybe don't process multi xmax in FreezeMultiXactId() if it is already marked as invalid?
Список pgsql-hackers
18.06.2024 18:47, Peter Geoghegan пишет:
> On Tue, Jun 18, 2024 at 10:29 AM Maxim Orlov <orlovmg@gmail.com> 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_INVALIDflag.
 
>> My guess is nobody noticed, that MultiXactIdIsValid call does not check the mentioned flag in the "first" condition,
butit'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
unfortunateoversight.
 
> 
> 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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alvaro Herrera
Дата:
Сообщение: Re: BitmapHeapScan streaming read user and prelim refactoring
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: generic plans and "initial" pruning