| 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 13:17:13 |
| Message-ID: | CA+HiwqGnh=DXt_MGQ6k4S-YVLo1BtfXicMFAsTTMALGvSBJD_Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jun 25, 2026 at 2:25 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Jun 25, 2026 at 1:06 PM Bertrand Drouvot
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> >
> > Hi hackers,
> >
> > while working on [1] I wanted to make use of RegisterSubXactCallback() and
> > realized that RegisterXactCallback() has this comment:
> >
> > "
> > * These functions are intended for use by dynamically loaded modules.
> > * For built-in modules we generally just hardwire the appropriate calls
> > * (mainly because it's easier to control the order that way, where needed).
> > "
> >
> > So I thought of hardwiring the call directly in Start/Commit/AbortSubTransaction()
> > instead.
> >
> > Then I realized that b7b27eb41a5 just made use of RegisterXactCallback() and
> > RegisterSubXactCallback(), so I'm wondering if it should hardwire the calls
> > instead?
> >
> > Note that the other RegisterSubXactCallback() and RegisterXactCallback() look
> > legitimate to me (as in loadable modules).
>
> Thanks for flagging this.
>
> 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.
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. A survivor would mean a
trigger batch went unflushed. On abort it's expected, so it just
resets.
--
Thanks, Amit Langote
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Hardwire-RI-fast-path-end-of-xact-cleanup-instead.patch | application/octet-stream | 8.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-06-25 13:21:13 | Re: Fix \crosstabview to honor \pset display_true/display_false |
| Previous Message | Emanuele | 2026-06-25 13:14:44 | Re: Out-of-Cycle release? (was Re: BUG #19490: Streaming standby on 16.14 stops applying WAL on MultiXactOffsetSLRU when primary is 16.8) |