| 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 |
| 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() |