Re: FullTransactionIdAdvance question

Hi Andres:

> On 2024-09-20 17:38:40 +0800, Andy Fan wrote:
>> static inline void
>> FullTransactionIdAdvance(FullTransactionId *dest)
>> {
..
>> }
>>
>> I understand this functiona as: 'dest->value++' increases the epoch when
>> necessary and we don't want use the TransactionId which is smaller than
>> FirstNormalTransactionId. But what is the point of the below code:
>>
>> /* see FullTransactionIdAdvance() */
>> if (FullTransactionIdPrecedes(*dest, FirstNormalFullTransactionId))
>> return;
>>
>> It looks to me it will be never true(I added a 'Assert(false);' above
>> the return, make check-world pass).
>
> Hm. I think in the past we did have some code that could end up calling
> FullTransactionIdAdvance() on special xids for some reason, IIRC it was
> related to BootstrapTransactionId. Turning those into a normal xid doesn't
> seem quite right, I guess and could hide bugs.
>
> But I'm not sure it'd not better to simply assert out in those cases.

Per my current understanding, special XIDs are special and better be
used explicitly (vs advance special XID-1 to special XID-2)? Currently
if the input is BootstrapTransactionId, then we would get
FrozenTransactionId, which I'm still can't understand a reason of it. I
checkout the code to the commit where FullTransactionIdAdvance was
introduced, and then add the "Assert(false)". make clean .. make
check-world still passed.

>> IIUC, should we remove it to save a prediction on each GetNewTransactionId
>> call?
>
> I could see adding an unlikely() to make sure the compiler orders the code to
> make it statically predictable.

Thanks, Attached is the version to add the 'unlikely' while I'm still
searching the possibility to remove the code. I will defer to you after
my understanding is expressed. Actually I'm more fan on the knowledge
about the XIDs which I am not aware of. I think either the fix of
'prediction miss' or removing the code probably not make any noticeable
improvements in this case. So thanks for checking this!

--
Best Regards
Andy Fan