Re: In core use of RegisterXactCallback() and RegisterSubXactCallback()

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: In core use of RegisterXactCallback() and RegisterSubXactCallback()
Date: 2026-06-25 23:21:50
Message-ID: CA+HiwqFi-vbvi0rQhvx3ien05vJ1=pm4xcreJTHzHMhyBzzNaw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jun 26, 2026 at 12:05 AM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> On Thu, Jun 25, 2026 at 10:17:13PM +0900, Amit Langote wrote:
> > On Thu, Jun 25, 2026 at 2:25 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > >
> > > Note the RegisterSubXactCallback() call from b7b27eb41a5 was already
> > > removed by a later fix (4113873a) that confined fast-path batching to
> > > the top transaction level, so only the RegisterXactCallback() remains.
> > > You're right that the header comment points toward hardwiring for
> > > built-in code. I took the shortcut of using Register* because the RI
> > > fast-path callback has no ordering dependency on the other hard-wired
> > > work in Commit/AbortTransaction(), but I agree it should follow the
> > > convention. I'll work up a patch converting it to a hardwired
> > > AtEOXact_RI() called from CommitTransaction(), PrepareTransaction()
> > > and AbortTransaction(), and post it on this thread.
> >
> > Here is a patch to do so.
>
> Thanks for looking at it!
>
> > It's a mechanical conversion, except the new AtEOXact_RI() takes
> > isCommit and asserts that the fast-path cache was already torn down by
> > commit, with a WARNING in non-assert builds.
>
> That makes sense to me.

Thanks for checking.

> Just one nit:
>
> + Assert for development builds and, since
> + * the transaction is already committed by now and FK checks may have been
> + * skipped, also warn in production builds.
>
> s/development builds/assert-enabled builds/? That looks more common and "
> development builds" would be introduced by this patch.

I grepped, and while the word "development" isn't rare in the tree,
none of the uses are "development builds" as a build type.
"assert-enabled" is the established term for that, so I've switched to
it in v2.

--
Thanks, Amit Langote

Attachment Content-Type Size
v2-0001-Hardwire-RI-fast-path-end-of-xact-cleanup-into-xa.patch application/octet-stream 8.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message feng wu 2026-06-25 23:24:32 Re: [PATCH] Check dead heap items before marking them unused
Previous Message Chao Li 2026-06-25 23:15:37 Re: Fix doc about pg_get_multixact_stats()