Thread: [HACKERS] cache lookup errors for missing replication origins
Hi all,
Cache lookup errors with elog() can be triggered easily by users at
SQL level using replication origin functions:
=# select pg_replication_origin_advance('popo', '0/1');
ERROR: XX000: cache lookup failed for replication origin 'popo'
LOCATION: replorigin_by_name, origin.c:229
=# select pg_replication_origin_drop('popo');
ERROR: XX000: cache lookup failed for replication origin 'popo'
LOCATION: replorigin_by_name, origin.c:229
=# select pg_replication_origin_session_setup('popo');
ERROR: XX000: cache lookup failed for replication origin 'popo'
LOCATION: replorigin_by_name, origin.c:229
=# select pg_replication_origin_progress('popo', true);
ERROR: XX000: cache lookup failed for replication origin 'popo';
LOCATION: replorigin_by_name, origin.c:229
A cache lookup means that an illogical status has been reached, but
those code paths don't refer to that. So I think that the error in
replorigin_by_name should be changed to that:
ERROR: 42704: replication slot "%s" does not exist
As far as I can see, replorigin_by_oid makes no use of its missing_ok
= false in the backend code, so letting it untouched would have no
impact. replorigin_by_name with missing_ok = false is only used with
SQL-callable functions, so it could be changed without any impact
elsewhere (without considering externally-maintained replication
modules).
Thanks,
--
Michael
On Tue, Sep 5, 2017 at 12:59 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > ERROR: 42704: replication slot "%s" does not exist s/slot/origin/ > As far as I can see, replorigin_by_oid makes no use of its missing_ok > = false in the backend code, so letting it untouched would have no > impact. replorigin_by_name with missing_ok = false is only used with > SQL-callable functions, so it could be changed without any impact > elsewhere (without considering externally-maintained replication > modules). As long as I don't forget, attached is a patch added as well to the next CF. replorigin_by_oid should have the same switch from elog to ereport in my opinion. Additional regression tests are included. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Wed, Sep 6, 2017 at 3:51 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Tue, Sep 5, 2017 at 12:59 PM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> ERROR: 42704: replication slot "%s" does not exist > > s/slot/origin/ > >> As far as I can see, replorigin_by_oid makes no use of its missing_ok >> = false in the backend code, so letting it untouched would have no >> impact. replorigin_by_name with missing_ok = false is only used with >> SQL-callable functions, so it could be changed without any impact >> elsewhere (without considering externally-maintained replication >> modules). > > As long as I don't forget, attached is a patch added as well to the > next CF. replorigin_by_oid should have the same switch from elog to > ereport in my opinion. Additional regression tests are included. The patch passes the regression test and I found no problems in this patch. I've marked it as Ready for Committer. 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 Tue, Oct 3, 2017 at 2:16 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: > The patch passes the regression test and I found no problems in this > patch. I've marked it as Ready for Committer. Committed and back-patched to 9.5, which was as far as it applied cleanly. -- 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 Thu, Oct 5, 2017 at 9:38 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Oct 3, 2017 at 2:16 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote: >> The patch passes the regression test and I found no problems in this >> patch. I've marked it as Ready for Committer. > > Committed and back-patched to 9.5, which was as far as it applied cleanly. Thanks all. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers