Content-Type: multipart/alternative;
 boundary="------------iKeGHL6wV8FoO0450EGV77UK"
Message-ID: <591163e7-7e74-4c30-beb2-45bddcb9a90e@gmail.com>
Date: Tue, 8 Sep 2026 09:54:05 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: xact_rollback spikes when logical walsender exits
To: Nikolay Samokhvalov <nik@postgres.ai>, Fujii Masao <masao.fujii@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>,
 Rafael Thofehrn Castro <rafaelthca@gmail.com>
References: 
 <CAM527d_EbU5Li4a5FdKQjYsdF-4Lqr_i3jXmZOm7Wbb=Q2KzTw@mail.gmail.com>
 <CAHGQGwFDv6=Dcbf1YbGH5S7y-M4ar4-zC-c1GWco_Cn_SE0c7w@mail.gmail.com>
 <CAM527d8Evz3BCffJeL=1RJi-7jjPYbKz_ZoUxm2Go2i-_N+g6w@mail.gmail.com>
Content-Language: en-GB
From: Mircea Cadariu <cadariu.mircea@gmail.com>
In-Reply-To: 
 <CAM527d8Evz3BCffJeL=1RJi-7jjPYbKz_ZoUxm2Go2i-_N+g6w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/591163e7-7e74-4c30-beb2-45bddcb9a90e%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------iKeGHL6wV8FoO0450EGV77UK
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

On 08/07/2026 19:37, Nikolay Samokhvalov wrote:
> Concretely, I propose splitting:
>
> 1) 0001 (v3, attached): narrow fix for the walsender miscount,
> intended as a back-patchable bug fix -- this is what's paging people
> in production today. Rebased on current master; the TAP test now
> also verifies that xact_commit does not change as a function of
> decoded transactions (walsender shutdown has a small fixed
> bookkeeping delta, handled via a control run).
>
> 2) 0002 (attached, draft): master-only implementation of option 3 --
> count xact_commit/xact_rollback only for regular client backends --
> plus a doc change defining both columns explicitly. It also removes
> the now-redundant parallel argument from AtEOXact_PgStat() and
> AtEOXact_PgStat_Database(), since parallel workers are background
> workers and are excluded by the backend-type check. 0002 has no test
> yet; if the definition is agreed on, I'll extend the new TAP test to
> cover the autovacuum/walsender exclusion.

Thanks for submitting the v3 patch, I had a look.

The patch applies cleanly on master. As experiment I added the tests 
only from 0001 and as expected they failed, but with the patch applied 
they pass. So far so good.

While the approach in 0001 could be considered a bit "action at a 
distance" approach, a suitable structural solution is in 0002.

I agree with starting to count xact_commit/xact_rollback only for 
regular client backends. TBH I expected it works this way already. 
Indeed, 0002 needs tests.

I'll set it to Ready for Committer and am volunteering for adding the 
tests or any other aspect I can help with to progress this patch.

-- 
Thanks,
Mircea Cadariu

--------------iKeGHL6wV8FoO0450EGV77UK
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hi, </p>
    <div class="moz-cite-prefix">On 08/07/2026 19:37, Nikolay
      Samokhvalov wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAM527d8Evz3BCffJeL=1RJi-7jjPYbKz_ZoUxm2Go2i-_N+g6w@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div class="gmail_quote">Concretely, I propose splitting:<br>
            <br>
            1) 0001 (v3, attached): narrow fix for the walsender
            miscount,<br>
            intended as a back-patchable bug fix -- this is what's
            paging people<br>
            in production today. Rebased on current master; the TAP test
            now<br>
            also verifies that xact_commit does not change as a function
            of<br>
            decoded transactions (walsender shutdown has a small fixed<br>
            bookkeeping delta, handled via a control run).<br>
            <br>
            2) 0002 (attached, draft): master-only implementation of
            option 3 --<br>
            count xact_commit/xact_rollback only for regular client
            backends --<br>
            plus a doc change defining both columns explicitly. It also
            removes<br>
            the now-redundant parallel argument from AtEOXact_PgStat()
            and<br>
            AtEOXact_PgStat_Database(), since parallel workers are
            background<br>
            workers and are excluded by the backend-type check. 0002 has
            no test<br>
            yet; if the definition is agreed on, I'll extend the new TAP
            test to<br>
            cover the autovacuum/walsender exclusion.</div>
        </div>
      </div>
    </blockquote>
    <p>Thanks for submitting the v3 patch, I had a look. </p>
    <p>The patch applies cleanly on master. As experiment I added the
      tests only from 0001 and as expected they failed, but with the
      patch applied they pass. So far so good. </p>
    <p>While the approach in 0001 could be considered a bit "action at a
      distance" approach, a suitable structural solution is in 0002. </p>
    <p>I agree with starting to count xact_commit/xact_rollback only for
      regular client backends. TBH I expected it works this way already.
      Indeed, 0002 needs tests. </p>
    <p>I'll set it to Ready for Committer and am volunteering for adding
      the tests or any other aspect I can help with to progress this
      patch. </p>
    <pre class="moz-signature" cols="72">-- 
Thanks,
Mircea Cadariu</pre>
  </body>
</html>

--------------iKeGHL6wV8FoO0450EGV77UK--