Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wXADG-0036DA-28 for pgsql-hackers@arkaria.postgresql.org; Wed, 10 Jun 2026 04:06:46 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wXADE-0093IQ-1A for pgsql-hackers@arkaria.postgresql.org; Wed, 10 Jun 2026 04:06:44 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wXADD-0093II-32 for pgsql-hackers@lists.postgresql.org; Wed, 10 Jun 2026 04:06:44 +0000 Received: from mail-vs1-xe2b.google.com ([2607:f8b0:4864:20::e2b]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wXADB-00000001wzi-3WUG for pgsql-hackers@lists.postgresql.org; Wed, 10 Jun 2026 04:06:42 +0000 Received: by mail-vs1-xe2b.google.com with SMTP id ada2fe7eead31-6c6f47198e3so4519053137.1 for ; Tue, 09 Jun 2026 21:06:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1781064400; cv=none; d=google.com; s=arc-20240605; b=Ex6dDSYa8XeaE0ZAM0+qpiGnThOv/O4ee+7cpVwxtDsaLeKpbyFmpPYU2t5NlfbMav V5F6GoFjHKfAGZLPbc/d0af678WW67x27UQiUq9aIEqDcT8l/8U3FZT5muO2TsnhPEDA rOF8aS51iHhd0M/FMvw/OgpBiV8wEb0sfzBh/n02xPXY114eL1kZuxBU80lsZCz5tUCU Crenc5pCk1oza8wVBfI/Jsne2OjFPyP6Ox7S502NILf0ZyJ/5VHZNVIzZReVFUCWkHBn v9u7p5cMFlFZBBRCkWJjysKae0bpOVH2WJWFki5XHm+xJxQ/iUU8H4IkpYtw1w17F0BZ /tOw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=1JOg0yW24WYkXq8bk2VxPp1aVxlaqN6H1y0+Wz/uuVc=; fh=Amp1xRGln6g0EWILETtuYxTXibrVJ+87XUo9hGefxkA=; b=cDsNM9vrRim+2lPE6vV8NzHl7iXI0UGAFmnrl/uHkMNt1BMvXD7boXk/1Gt3i7ocCt VUpE7QQV/hGOT++30/+zHYxMW2/WlCqaA5uaxGMjQD41TxYjPMnmtaKERrvsBMa4/XUA kQL8Nw//iajImbSYS/EpkanFCXBUyYqlINMVdhC5gAo4htvolXJVqHTN4SGXkoXFlFHt eL4VHXkLM7ZgDE1J3V3oqNpQW49nTtlfS+ns2p+hZ3VAqiGiMLEaIQhCDC+QNu+cSknU omrDAA0ltbSlfskkbTbbrTMfIxw08YrbgxaFdXL3MKHBmK+jK5JE+iQcCPfMxjvwlNLu Fayw==; darn=lists.postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1781064400; x=1781669200; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=1JOg0yW24WYkXq8bk2VxPp1aVxlaqN6H1y0+Wz/uuVc=; b=FhlgK/bRd48BR4/Vf5goRPCiq5AN2xVuODLEZP+D6AN7IoKJTGh2cyHBY98MFVP4zn /FHexGy2clXVtJa4WHdE9qMdqIWVFHJNR1g5O1AmMcYXn3sxmlgned8M3E+nRBbVkD3a TeTPo+BIl51Za21bqGOSGAkueVojipnuaqWPb3GNcqsAGWdxreI286142OqSnLM6rwrC qwSOug3vmh3ezzBb57S3EeWlQ/jm6doPe5jpKlgLpp8+MjdhSvCZVhkyNPMGN3ogz+HJ 2oNT5yQcRHyG78kWdZAN+u2hWT4Ig+wnVxvtPgCGzGnDliwYiU+z+oe9rd8El+RjiQew mBrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781064400; x=1781669200; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1JOg0yW24WYkXq8bk2VxPp1aVxlaqN6H1y0+Wz/uuVc=; b=Oa0qPeotBz1MiYK8K7DYVSx6+QoLSVaCdoR3dHMirIClYXoV40Tptkn6flDbciLdLC aRaHnZRRSGUZC1opb6sSsLrzwG6Q4TPVZXQPOptMexCsHFvA2rZ53N1KaaM3wbVpWmpQ uL/uDE/h0O+8iLCAXbR7w4+TT9ReMrPNp/L3GabgoeHQj4XuVGarWJ3k5fnrNEterUVT Y9DrcYxviMZMYhZZLXoVe66ItpamLaVArEMLmEtpPj1hO3tT+xkMcZDscriDZZVUPht9 umtHgnI166sh0BBgl1Qg+nUtLlPWbxE+dbk+LrJHbnFy46Uh4TPr7AQLoMxVvjL12uIL ux8w== X-Forwarded-Encrypted: i=1; AFNElJ/FIKePW09SBfCokPzStaZM+R9CJrtv7SkkTj1QMR6O7LMU/O2RIUh9uCZ+2IzJtGshoh8Lo5WAJ6baIA8l@lists.postgresql.org X-Gm-Message-State: AOJu0Yy0F8/nE+5MZbps1VyVcvfCUm3li5OZ1LCvdWumX+V3B569G4Gd bMiVlpWBHfHHupvNueMFW5VY/Jai6EyDylARtCGUM6avqFeG+KOrQ8C66jJE4AUtmpWfdBTqKni 2jeu2mda8znSK1TSy1TJczu1Z66ZAzVk= X-Gm-Gg: Acq92OGPNouIUVhDAqnZW9dCJEpr3Dc7ixpso9i48ZrBrM6lb4VurCodF/J0534gOFU JdVQBwG/e/US79x07wZBjhcKbsztKK7l70bBtpy6zQ9NGNmzxuKmpDKuNQzAnVP8bphCcQbJK/N nBWBTPKJFtZOAmb2eDPYu1V1mA6SYS0g/DK4x3wReNrlULIgSD+DxMCXWXHY3sNutcJyeyA5YRu lfZLKWmJQzlMlPDcS3uBYRy+zlR4kcdeoT08W6hTbAzi2T8woRcyG4GDZp4rIAywr0kmNVoNioH yb/39heliHqDt/MUyQ== X-Received: by 2002:a05:6102:419e:b0:6ce:d923:7642 with SMTP id ada2fe7eead31-7003bf5c587mr8382793137.14.1781064400474; Tue, 09 Jun 2026 21:06:40 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: SATYANARAYANA NARLAPURAM Date: Tue, 9 Jun 2026 21:06:29 -0700 X-Gm-Features: AVVi8CfWHetsCfAZATXnii37z4-Fdj-fHlxOfhxd5T0xtzIPT2pceKn9oS8vZcM Message-ID: Subject: Re: [PATCH] Release replication slot on error in SQL-callable slot functions To: shveta malik Cc: Fujii Masao , vignesh C , PostgreSQL Hackers , shveta malik Content-Type: multipart/alternative; boundary="000000000000e997080653de5e88" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000e997080653de5e88 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Thu, May 28, 2026 at 10:45=E2=80=AFPM shveta malik wrote: > On Wed, May 27, 2026 at 5:40=E2=80=AFPM Fujii Masao wrote: > > > > On Wed, May 27, 2026 at 8:00=E2=80=AFPM shveta malik > wrote: > > > pg_copy_physical_replication_slot() should not have it as the common > > > 'copy_replication_slot' is already fixed in the patch. > > > > copy_replication_slot() calls create_physical_replication_slot() before > > entering the PG_TRY/PG_CATCH block. So if > create_physical_replication_slot() > > throws an error, wouldn't the same issue still occur? > > > > > You are right. Using v5, if I force create_physical_replication_slot() > to fail while executing pg_copy_physical_replication_slot() (through > debugging), I can see that the next slot-related call hits an Assert. > > 1) > I also noticed that for pg_copy_logical_replication_slot(), we do not > hit the CATCH block of copy_replication_slot(), but instead the one in > create_logical_replication_slot(). That behavior seems fine. > > However, I noticed some inconsistencies in the implementation. > > For create_physical_replication_slot(), the caller > pg_create_physical_replication_slot() contains the TRY-CATCH block, > whereas create_logical_replication_slot() contains its own TRY-CATCH > block internally. > > I think it makes more sense to keep the TRY-CATCH handling inside the > internal functions, i.e. create_logical_replication_slot() and > create_physical_replication_slot(), since that would automatically > cover all callers. For example, create_logical_replication_slot() is > invoked from multiple places, so callers need not worry about cleanup > handling themselves. Similarly, for > create_physical_replication_slot(), we could move the TRY-CATCH block > inside the function instead of having it in > pg_create_physical_replication_slot(). Doing so would also resolve the > issue with pg_copy_physical_replication_slot(). > > > 2) > I also feel that ReplicationSlotCreate() should be moved inside the > TRY block in create_logical_replication_slot(). > > Currently, if in the future ReplicationSlotCreate() gains any > post-slot-creation implementation that could throw an error, we may > end up leaving the system in an unsafe state. Keeping it inside the > TRY block would make the code more robust against such future changes. > ~~ > > Reveiwign further. Need to review few more things on v5/v6. Got stuck with something. Will send a revised patch over the weekend, in the meantime if you want to take it forward please feel free to. Thanks, Satya > > > --000000000000e997080653de5e88 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Thu, May 28, 2026 at 10:4= 5=E2=80=AFPM shveta malik <shv= eta.malik@gmail.com> wrote:
On Wed, May 27, 2026 at 5:40=E2=80=AFPM Fuj= ii Masao <mas= ao.fujii@gmail.com> wrote:
>
> On Wed, May 27, 2026 at 8:00=E2=80=AFPM shveta malik <shveta.malik@gmail.com&g= t; wrote:
> > pg_copy_physical_replication_slot() should not have it as the com= mon
> > 'copy_replication_slot' is already fixed in the patch. >
> copy_replication_slot() calls create_physical_replication_slot() befor= e
> entering the PG_TRY/PG_CATCH block. So if create_physical_replication_= slot()
> throws an error, wouldn't the same issue still occur?
>


You are right. Using v5, if I force create_physical_replication_slot()
to fail while executing pg_copy_physical_replication_slot() (through
debugging), I can see that the next slot-related call hits an Assert.

1)
I also noticed that for pg_copy_logical_replication_slot(), we do not
hit the CATCH block of copy_replication_slot(), but instead the one in
create_logical_replication_slot(). That behavior seems fine.

However, I noticed some inconsistencies in the implementation.

For create_physical_replication_slot(), the caller
pg_create_physical_replication_slot() contains the TRY-CATCH block,
whereas create_logical_replication_slot() contains its own TRY-CATCH
block internally.

I think it makes more sense to keep the TRY-CATCH handling inside the
internal functions, i.e. create_logical_replication_slot() and
create_physical_replication_slot(), since that would automatically
cover all callers. For example, create_logical_replication_slot() is
invoked from multiple places, so callers need not worry about cleanup
handling themselves. Similarly, for
create_physical_replication_slot(), we could move the TRY-CATCH block
inside the function instead of having it in
pg_create_physical_replication_slot(). Doing so would also resolve the
issue with pg_copy_physical_replication_slot().


2)
I also feel that ReplicationSlotCreate() should be moved inside the
TRY block in create_logical_replication_slot().

Currently, if in the future ReplicationSlotCreate() gains any
post-slot-creation implementation that could throw an error, we may
end up leaving the system in an unsafe state. Keeping it inside the
TRY block would make the code more robust against such future changes.
~~

Reveiwign further. Need to review few more things on v5/v6.

Got stuck with something. Will s= end a revised patch over the weekend, in the meantime if you want to take i= t forward please feel free to.

Thanks,
Satya


--000000000000e997080653de5e88--