Create subscription with `create_slot=false` and incorrect slot name
Hi
Maybe this question was already raised before, but I couldn't find a
discussion. When I'm creating a subscription with `create_slot=false` looks
like it's possible to pass a slot name with invalid characters. In this
particular case both publisher and subscriber were on the same machine:
=# CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=db host=host
port=port user=user' PUBLICATION test_pub WITH (create_slot=false,
slot_name='test slot');
NOTICE: 00000: synchronized table states
LOCATION: CreateSubscription, subscriptioncmds.c:443
CREATE SUBSCRIPTION
Time: 5.887 ms
Of course `test slot` doesn't exist, because I can't create a slot with
incorrect name. And consequently I can't drop this subscription:
=# DROP SUBSCRIPTION test_sub;
ERROR: XX000: could not drop the replication slot "test slot" on
publisher
DETAIL: The error was: ERROR: replication slot name "test slot"
contains invalid character
HINT: Replication slot names may only contain lower case letters,
numbers, and the underscore character.
LOCATION: DropSubscription, subscriptioncmds.c:947
Time: 2.615 ms
=# CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=db host=host
port=port user=user' PUBLICATION test_pub WITH (create_slot=false,
slot_name='test slot');
ERROR: 42710: subscription "test_sub" already exists
LOCATION: CreateSubscription, subscriptioncmds.c:344
Time: 0.492 ms
Is it an issue or I'm doing something wrong?
2017-05-22 17:52 GMT-03:00 Dmitry Dolgov <9erthalion6@gmail.com>:
Maybe this question was already raised before, but I couldn't find a
discussion. When I'm creating a subscription with `create_slot=false` looks
like it's possible to pass a slot name with invalid characters. In this
particular case both publisher and subscriber were on the same machine:=# CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=db host=host
port=port user=user' PUBLICATION test_pub WITH (create_slot=false,
slot_name='test slot');
NOTICE: 00000: synchronized table states
LOCATION: CreateSubscription, subscriptioncmds.c:443
CREATE SUBSCRIPTION
Time: 5.887 msThe command succeed even if slot_name is invalid. That's because slot_name
isn't validated. ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error. IMHO we should prevent
a future error (use of invalid slot name).
Of course `test slot` doesn't exist, because I can't create a slot with
incorrect name. And consequently I can't drop this subscription:=# DROP SUBSCRIPTION test_sub;
ERROR: XX000: could not drop the replication slot "test slot" on
publisher
DETAIL: The error was: ERROR: replication slot name "test slot"
contains invalid character
HINT: Replication slot names may only contain lower case letters,
numbers, and the underscore character.
LOCATION: DropSubscription, subscriptioncmds.c:947
Time: 2.615 msIndeed you can drop the subscription. There are two details: (i)
subscription should be disabled and (ii) slot name can't be set.
bar=# drop subscription sub1;
ERROR: could not drop the replication slot "does_not_exist" on publisher
DETAIL: The error was: ERROR: replication slot "does_not_exist" does not
exist
bar=# alter subscription sub1 set (slot_name = NONE);
ERROR: cannot set slot_name = NONE for enabled subscription
bar=# alter subscription sub1 disable;
ALTER SUBSCRIPTION
bar=# drop subscription sub1;
ERROR: could not drop the replication slot "does_not_exist" on publisher
DETAIL: The error was: ERROR: replication slot "does_not_exist" does not
exist
bar=# alter subscription sub1 set (slot_name = NONE);
ALTER SUBSCRIPTION
bar=# drop subscription sub1;
DROP SUBSCRIPTION
Should we add a hint for 'could not drop the replication slot' message?
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
On Tue, May 23, 2017 at 10:56 AM, Euler Taveira <euler@timbira.com.br> wrote:
2017-05-22 17:52 GMT-03:00 Dmitry Dolgov <9erthalion6@gmail.com>:
Maybe this question was already raised before, but I couldn't find a
discussion. When I'm creating a subscription with `create_slot=false`
looks
like it's possible to pass a slot name with invalid characters. In this
particular case both publisher and subscriber were on the same machine:=# CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=db host=host
port=port user=user' PUBLICATION test_pub WITH (create_slot=false,
slot_name='test slot');
NOTICE: 00000: synchronized table states
LOCATION: CreateSubscription, subscriptioncmds.c:443
CREATE SUBSCRIPTION
Time: 5.887 msThe command succeed even if slot_name is invalid. That's because slot_name
isn't validated. ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error. IMHO we should prevent
a future error (use of invalid slot name).
+1. Since, slot_name can be provided even when create_slot is set
false, it should be validated as well while creating the subscription.
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 23 May 2017 at 07:26, Euler Taveira <euler@timbira.com.br> wrote:
ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error.
IMHO we should prevent a future error (use of invalid slot name).
Yes, I see now. I assume this little patch should be enough for that.
Attachments:
validate-slot-name.patchtext/x-patch; charset=US-ASCII; name=validate-slot-name.patchDownload
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 86eb31d..625b5e9 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -39,6 +39,7 @@
#include "replication/logicallauncher.h"
#include "replication/origin.h"
+#include "replication/slot.h"
#include "replication/walreceiver.h"
#include "replication/walsender.h"
#include "replication/worker_internal.h"
@@ -138,6 +139,9 @@ parse_subscription_options(List *options, bool *connect, bool *enabled_given,
*slot_name_given = true;
*slot_name = defGetString(defel);
+ /* Validate slot_name even if create_slot = false */
+ ReplicationSlotValidateName(*slot_name, ERROR);
+
/* Setting slot_name = NONE is treated as no slot name. */
if (strcmp(*slot_name, "none") == 0)
*slot_name = NULL;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 91ba8ab..6334a18 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -62,6 +62,9 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
ERROR: subscription with slot_name = NONE must also set create_slot = false
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
ERROR: subscription with slot_name = NONE must also set enabled = false
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = "invalid name", create_slot = false);
+ERROR: replication slot name "invalid name" contains invalid character
+HINT: Replication slot names may only contain lower case letters, numbers, and the underscore character.
-- ok - with slot_name = NONE
CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 4b694a3..5b635bc 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -47,6 +47,7 @@ CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE);
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false);
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = "invalid name", create_slot = false);
-- ok - with slot_name = NONE
CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
On 5/23/17 02:33, Kuntal Ghosh wrote:
The command succeed even if slot_name is invalid. That's because slot_name
isn't validated. ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error. IMHO we should prevent
a future error (use of invalid slot name).+1. Since, slot_name can be provided even when create_slot is set
false, it should be validated as well while creating the subscription.
This came up in a previous thread. It is up to the publishing end what
slot names it accepts. So running the validation locally is incorrect.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 24, 2017 at 7:31 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 5/23/17 02:33, Kuntal Ghosh wrote:
The command succeed even if slot_name is invalid. That's because slot_name
isn't validated. ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error. IMHO we should prevent
a future error (use of invalid slot name).+1. Since, slot_name can be provided even when create_slot is set
false, it should be validated as well while creating the subscription.This came up in a previous thread. It is up to the publishing end what
slot names it accepts. So running the validation locally is incorrect.
That argument seems pretty tenuous; surely both ends are PostgreSQL,
and the rules for valid slot names aren't likely to change very often.
But even if we accept it as true, I still don't like the idea that a
DROP can just fail, especially with no real guidance as to how to fix
things so it doesn't fail. Ideas:
1. If we didn't create the slot (and have never connected to it?),
don't try to drop it.
2. Emit some kind of a HINT telling people about ALTER SUBSCRIPTION ..
SET (slot_name = NONE).
3. ???
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 24, 2017 at 9:41 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, May 24, 2017 at 7:31 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 5/23/17 02:33, Kuntal Ghosh wrote:
The command succeed even if slot_name is invalid. That's because slot_name
isn't validated. ReplicationSlotValidateName() should be called in
parse_subscription_options() to avoid a pilot error. IMHO we should prevent
a future error (use of invalid slot name).+1. Since, slot_name can be provided even when create_slot is set
false, it should be validated as well while creating the subscription.This came up in a previous thread. It is up to the publishing end what
slot names it accepts. So running the validation locally is incorrect.That argument seems pretty tenuous; surely both ends are PostgreSQL,
and the rules for valid slot names aren't likely to change very often.But even if we accept it as true, I still don't like the idea that a
DROP can just fail, especially with no real guidance as to how to fix
things so it doesn't fail. Ideas:1. If we didn't create the slot (and have never connected to it?),
don't try to drop it.2. Emit some kind of a HINT telling people about ALTER SUBSCRIPTION ..
SET (slot_name = NONE).3. ???
+1 to #2 idea. We already emit such errhint when connection to the
publisher failed. I think we can do the same thing in this case.
subscriptioncmds.c:L928
wrconn = walrcv_connect(conninfo, true, subname, &err);
if (wrconn == NULL)
ereport(ERROR,
(errmsg("could not connect to publisher when attempting to "
"drop the replication slot \"%s\"", slotname),
errdetail("The error was: %s", err),
errhint("Use ALTER SUBSCRIPTION ... SET (slot_name = NONE) "
"to disassociate the subscription from the
slot.")));
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/24/17 21:41, Robert Haas wrote:
This came up in a previous thread. It is up to the publishing end what
slot names it accepts. So running the validation locally is incorrect.That argument seems pretty tenuous; surely both ends are PostgreSQL,
and the rules for valid slot names aren't likely to change very often.
Remember that this could be used for upgrades and side-grades, so the
local rules could change or be more restricted in the future or
depending on compilation options.
But even if we accept it as true, I still don't like the idea that a
DROP can just fail, especially with no real guidance as to how to fix
things so it doesn't fail. Ideas:1. If we didn't create the slot (and have never connected to it?),
don't try to drop it.
That would conceptually be nice, but it would probably create a bunch of
problems of its own. For one, we would need an interlock so that the
first $anything that connects to the slot registers it in the local
catalog as "it's mine now".
2. Emit some kind of a HINT telling people about ALTER SUBSCRIPTION ..
SET (slot_name = NONE).
The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.). We don't want to give the hint that is effectively
"just forget about the slot then" for all of them. So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).
Another way to fix this particular issue is to not verify the
replication slot name before doing the drop. After all, if the name is
not valid, then you can also just report that it doesn't exist.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25/05/17 23:26, Peter Eisentraut wrote:
On 5/24/17 21:41, Robert Haas wrote:
This came up in a previous thread. It is up to the publishing end what
slot names it accepts. So running the validation locally is incorrect.That argument seems pretty tenuous; surely both ends are PostgreSQL,
and the rules for valid slot names aren't likely to change very often.Remember that this could be used for upgrades and side-grades, so the
local rules could change or be more restricted in the future or
depending on compilation options.But even if we accept it as true, I still don't like the idea that a
DROP can just fail, especially with no real guidance as to how to fix
things so it doesn't fail. Ideas:1. If we didn't create the slot (and have never connected to it?),
don't try to drop it.That would conceptually be nice, but it would probably create a bunch of
problems of its own. For one, we would need an interlock so that the
first $anything that connects to the slot registers it in the local
catalog as "it's mine now".2. Emit some kind of a HINT telling people about ALTER SUBSCRIPTION ..
SET (slot_name = NONE).The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.). We don't want to give the hint that is effectively
"just forget about the slot then" for all of them. So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).
This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
could check for it and offer hint only for this case.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/25/17 19:16, Petr Jelinek wrote:
The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.). We don't want to give the hint that is effectively
"just forget about the slot then" for all of them. So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
could check for it and offer hint only for this case.
We would have to extend the walreceiver interface a little to pass that
through, but it seems doable.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, May 26, 2017 at 05:05:37PM -0400, Peter Eisentraut wrote:
On 5/25/17 19:16, Petr Jelinek wrote:
The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.). We don't want to give the hint that is effectively
"just forget about the slot then" for all of them. So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
could check for it and offer hint only for this case.We would have to extend the walreceiver interface a little to pass that
through, but it seems doable.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/25/17 17:26, Peter Eisentraut wrote:
Another way to fix this particular issue is to not verify the
replication slot name before doing the drop. After all, if the name is
not valid, then you can also just report that it doesn't exist.
Here is a possible patch along these lines.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Remove-replication-slot-name-check-from-ReplicationS.patchtext/plain; charset=UTF-8; name=0001-Remove-replication-slot-name-check-from-ReplicationS.patch; x-mac-creator=0; x-mac-type=0Download
From 1e09473bb2f571af0a6a8e4ca718d291efa63772 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 30 May 2017 14:57:01 -0400
Subject: [PATCH] Remove replication slot name check from
ReplicationSlotAcquire()
When trying to access a replication slot that is supposed to already
exist, we don't need to check the naming rules again. If the slot
does not exist, we will then get a "does not exist" error message, which
is generally more useful from the perspective of an end user.
---
src/backend/replication/slot.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 5386e86aa6..c0f7fbb2b2 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -331,8 +331,6 @@ ReplicationSlotAcquire(const char *name)
Assert(MyReplicationSlot == NULL);
- ReplicationSlotValidateName(name, ERROR);
-
/* Search for the named slot and mark it active if we find it. */
LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
for (i = 0; i < max_replication_slots; i++)
--
2.13.0
On 5/29/17 22:26, Noah Misch wrote:
On Fri, May 26, 2017 at 05:05:37PM -0400, Peter Eisentraut wrote:
On 5/25/17 19:16, Petr Jelinek wrote:
The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.). We don't want to give the hint that is effectively
"just forget about the slot then" for all of them. So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
could check for it and offer hint only for this case.We would have to extend the walreceiver interface a little to pass that
through, but it seems doable.[Action required within three days. This is a generic notification.]
I have just posted a patch earlier in this thread with a possible
solution. If people don't like that approach, then we can pursue the
approach with error codes discussed above. But I would punt that to
PG11 in that case. I will report back on Friday.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, May 30, 2017 at 3:01 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 5/25/17 17:26, Peter Eisentraut wrote:
Another way to fix this particular issue is to not verify the
replication slot name before doing the drop. After all, if the name is
not valid, then you can also just report that it doesn't exist.Here is a possible patch along these lines.
I don't see how this solves the problem. Don't you still end up with
an error message telling you that you can't drop the subscription, and
no guidance as to how to fix it?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/31/17 09:40, Robert Haas wrote:
On Tue, May 30, 2017 at 3:01 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 5/25/17 17:26, Peter Eisentraut wrote:
Another way to fix this particular issue is to not verify the
replication slot name before doing the drop. After all, if the name is
not valid, then you can also just report that it doesn't exist.Here is a possible patch along these lines.
I don't see how this solves the problem. Don't you still end up with
an error message telling you that you can't drop the subscription, and
no guidance as to how to fix it?
Well, the idea was to make the error message less cryptic.
But I notice that there is really little documentation about this. So
how about the attached documentation patch as well?
As mentioned earlier, if we want to do HINT messages, that will be a bit
more involved and probably PG11 material.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-doc-Add-note-that-DROP-SUBSCRIPTION-drops-replicatio.patchtext/plain; charset=UTF-8; name=0001-doc-Add-note-that-DROP-SUBSCRIPTION-drops-replicatio.patch; x-mac-creator=0; x-mac-type=0Download
From 3d89b959794abe1bd3addeb9c7c1340187a3cef2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 31 May 2017 22:35:33 -0400
Subject: [PATCH] doc: Add note that DROP SUBSCRIPTION drops replication slot
Add some information about what to do when this fails.
---
doc/src/sgml/ref/drop_subscription.sgml | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml
index 4f34a35eef..42068d617b 100644
--- a/doc/src/sgml/ref/drop_subscription.sgml
+++ b/doc/src/sgml/ref/drop_subscription.sgml
@@ -74,6 +74,28 @@ <title>Parameters</title>
</refsect1>
<refsect1>
+ <title>Notes</title>
+
+ <para>
+ When dropping a subscription that is associated with a replication slot on
+ the remote host (the normal state), <command>DROP SUBSCRIPTION</command>
+ will connect to the remote host and try to drop the replication slot as
+ part of its operation. This is necessary so that the resources allocated
+ for the subscription on the remote host are released. If this fails,
+ either because the remote host is not reachable or because the remote
+ replication slot cannot be dropped or does not exist or never existed,
+ the <command>DROP SUBSCRIPTION</command> command will fail. To proceed in
+ this situation, disassociate the subscription from the replication slot by
+ executing <literal>ALTER SUBSCRIPTION ... SET (slot_name = NONE)</literal>.
+ After that, <command>DROP SUBSCRIPTION</command> will no longer attempt any
+ actions on a remote host. Note that if the remote replication slot still
+ exists, it should then be dropped manually; otherwise it will continue to
+ reserve WAL and might eventually cause the disk to fill up. See
+ also <xref linkend="logical-replication-subscription-slot">.
+ </para>
+ </refsect1>
+
+ <refsect1>
<title>Examples</title>
<para>
--
2.13.0
On 01/06/17 04:44, Peter Eisentraut wrote:
On 5/31/17 09:40, Robert Haas wrote:
On Tue, May 30, 2017 at 3:01 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 5/25/17 17:26, Peter Eisentraut wrote:
Another way to fix this particular issue is to not verify the
replication slot name before doing the drop. After all, if the name is
not valid, then you can also just report that it doesn't exist.Here is a possible patch along these lines.
I don't see how this solves the problem. Don't you still end up with
an error message telling you that you can't drop the subscription, and
no guidance as to how to fix it?Well, the idea was to make the error message less cryptic.
But I notice that there is really little documentation about this. So
how about the attached documentation patch as well?As mentioned earlier, if we want to do HINT messages, that will be a bit
more involved and probably PG11 material.
I think the combination of those patches is probably good enough
solution for PG10 (I never understood the need for name validation in
ReplicationSlotAcquire() anyway).
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6/1/17 08:20, Petr Jelinek wrote:
I think the combination of those patches is probably good enough
solution for PG10 (I never understood the need for name validation in
ReplicationSlotAcquire() anyway).
I have committed these two patches and will close this open item.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 26 May 2017 at 23:05, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
On 5/25/17 19:16, Petr Jelinek wrote:
The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.). We don't want to give the hint that is effectively
"just forget about the slot then" for all of them. So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
could check for it and offer hint only for this case.We would have to extend the walreceiver interface a little to pass that
through, but it seems doable.
Just to make it clear for me. If I understand correctly, it should be more
or
less easy to extend it in that way, something like in the attached patch.
Or am I missing something?
Attachments:
drop-subscription-hint.patchtext/x-patch; charset=US-ASCII; name=drop-subscription-hint.patchDownload
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 86eb31d..9f7d73c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -941,10 +941,20 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
res = walrcv_exec(wrconn, cmd.data, 0, NULL);
if (res->status != WALRCV_OK_COMMAND)
- ereport(ERROR,
- (errmsg("could not drop the replication slot \"%s\" on publisher",
- slotname),
- errdetail("The error was: %s", res->err)));
+ {
+ if (res->errcode == ERRCODE_UNDEFINED_OBJECT)
+ ereport(ERROR,
+ (errmsg("could not drop the replication slot \"%s\" on publisher",
+ slotname),
+ errdetail("The error was: %s", res->err),
+ errhint("Use ALTER SUBSCRIPTION ... SET (slot_name = NONE) "
+ "to disassociate the subscription from the slot.")));
+ else
+ ereport(ERROR,
+ (errmsg("could not drop the replication slot \"%s\" on publisher",
+ slotname),
+ errdetail("The error was: %s", res->err)));
+ }
else
ereport(NOTICE,
(errmsg("dropped replication slot \"%s\" on publisher",
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index ebe9c91..1caa051 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -873,6 +873,7 @@ libpqrcv_exec(WalReceiverConn *conn, const char *query,
{
PGresult *pgres = NULL;
WalRcvExecResult *walres = palloc0(sizeof(WalRcvExecResult));
+ const char *errorcode = NULL;
if (MyDatabaseId == InvalidOid)
ereport(ERROR,
@@ -915,6 +916,9 @@ libpqrcv_exec(WalReceiverConn *conn, const char *query,
case PGRES_FATAL_ERROR:
case PGRES_BAD_RESPONSE:
walres->status = WALRCV_ERROR;
+ errorcode = PQresultErrorField(pgres, PG_DIAG_SQLSTATE);
+ walres->errcode = MAKE_SQLSTATE(errorcode[0], errorcode[1], errorcode[2],
+ errorcode[3], errorcode[4]);
walres->err = pchomp(PQerrorMessage(conn->streamConn));
break;
}
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index 31d090c..b0582af 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -186,6 +186,7 @@ typedef struct WalRcvExecResult
{
WalRcvExecStatus status;
char *err;
+ int errcode;
Tuplestorestate *tuplestore;
TupleDesc tupledesc;
} WalRcvExecResult;