public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
Date: Wed, 10 Jun 2026 15:14:00 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHg+QDfZV_U2SQDrGy7DyBFtb45rQ_4Ln2NiQBk3p74bFTj40Q@mail.gmail.com>
References: <CAHGQGwEbt2UdAsA4i8aE0snysmEtG0YeRtpr-NO=uTtJa4AB-Q@mail.gmail.com>
<CAJpy0uD-_VurCWEYe3mczoZFDpXqjaZmC8gygaNZ5a_cuObOdQ@mail.gmail.com>
<CAHg+QDfZV_U2SQDrGy7DyBFtb45rQ_4Ln2NiQBk3p74bFTj40Q@mail.gmail.com>
Sorry for the late response.
I may be misunderstanding the patch, but it looks to me as if it is
trying to handle several different classes of problems uniformly by
wrapping them in exception handling.
ReplicationSlotAcquire() can exit with an ERROR after setting
MyReplicationSlot. I think it should restore that state itself. Or
rather, perhaps it should not set MyReplicationSlot until it is
certain that the slot can be successfully acquired. I think the same
kind of argument applies to ReplicationSlotCreate().
There is also a different issue around pgstat_create_replslot(). If
ReplicationSlotCreate() calls it and then an error is thrown later in
the caller, the caller needs to clean up that pgstat entry. That
seems like a separate kind of cleanup from restoring
MyReplicationSlot.
In pg_logical_slot_get_changes_guts(), the patch moves the call to
ReplicationSlotAcquire() inside the PG_TRY block. In practice the
code checks whether MyReplicationSlot is NULL before releasing it, so
this may work. However, semantically, it looks as if the caller is
also responsible for releasing the slot even when
ReplicationSlotAcquire() itself fails.
More generally, I am not sure that all of these cleanup actions belong
at the same level.
Restoring MyReplicationSlot, cleaning up pgstat entry, and releasing
an acquired slot seem to be different kinds of
responsibilities. Handling them all through the same PG_TRY/PG_CATCH
blocks causes those cleanup responsibilities to cross component
boundaries. That, in turn, makes it harder to tell whether all
resources affected by an error are actually being cleaned up.
Regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
view thread (23+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox