Re: Fix for consume_xids advancing XIDs incorrectly - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: Fix for consume_xids advancing XIDs incorrectly
Date
Msg-id a7667256-b98d-49a1-95a0-cca594151d07@oss.nttdata.com
Whole thread Raw
In response to Re: Fix for consume_xids advancing XIDs incorrectly  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: Fix for consume_xids advancing XIDs incorrectly
List pgsql-hackers

On 2024/10/18 14:57, Yushi Ogiwara wrote:
> In conclusion, I agree that:
> 
>   * Update lastxid with GetTopTransactionId().
>   * consume_xids with 0 should raise an error.
> 
> I have attached a new patch that incorporates your suggestions.

One concern in this discussion is that the return value of this function isn't
entirely clear. To address this, how about adding a comment at the beginning of
consume_xids() like: "Returns the last XID assigned by consume_xids()"?


       * the cache overflows, but beyond that, we don't keep track of the
       * consumed XIDs.
       */
-    (void) GetTopTransactionId();
+    if(!FullTransactionIdIsValid(GetTopFullTransactionIdIfAny()))
+    {
+        lastxid = GetTopTransactionId();
+        consumed++;
+    }

How about appending the following to the end of the first paragraph in
the above source comments?

     If no top-level XID is assigned, a new one is obtained,
     and the consumed XID counter is incremented.



    if (xids_left > 2000 &&
        consumed - last_reported_at < REPORT_INTERVAL &&
        MyProc->subxidStatus.overflowed)
    {
        int64        consumed_by_shortcut = consume_xids_shortcut();

        if (consumed_by_shortcut > 0)
        {
            consumed += consumed_by_shortcut;
            continue;
        }
    }

By the way, this isn't directly related to the proposed patch, but while reading
the xid_wraparound code, I noticed that consume_xids_common() could potentially
return an unexpected XID if consume_xids_shortcut() returns a value greater
than 2000. Based on the current logic, consume_xids_common() should always return
a value less than 2000, so the issue I'm concerned about shouldn't occur. Still,
would it be worth adding an assertion to ensure that consume_xids_common() never
returns a value greater than 2000?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION




pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Pgoutput not capturing the generated columns
Next
From: Peter Smith
Date:
Subject: Re: Pgoutput not capturing the generated columns