Horton

Thread

Commits

  1. Fix more user-visible elog() calls.

  1. cache lookup errors for missing replication origins

    Michael Paquier <michael.paquier@gmail.com> — 2017-09-05T03:59:13Z

    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

  2. Re: cache lookup errors for missing replication origins

    Michael Paquier <michael.paquier@gmail.com> — 2017-09-06T06:51:14Z

    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

  3. Re: cache lookup errors for missing replication origins

    Masahiko Sawada <sawada.mshk@gmail.com> — 2017-10-03T06:16:30Z

    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

  4. Re: cache lookup errors for missing replication origins

    Robert Haas <robertmhaas@gmail.com> — 2017-10-05T12:38:35Z

    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

  5. Re: cache lookup errors for missing replication origins

    Michael Paquier <michael.paquier@gmail.com> — 2017-10-05T12:51:49Z

    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