From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Fabrízio Mello <fabriziomello(at)gmail(dot)com> |
Cc: | Josh Berkus <josh(at)agliodbs(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: -d option for pg_isready is broken |
Date: | 2013-11-19 14:51:08 |
Message-ID: | CA+Tgmob5ikDQfGorV3qt3N1QCWGtRgOxaJ4xpD8zQoNHorg-rg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Nov 15, 2013 at 9:01 PM, Fabrízio de Royes Mello
<fabriziomello(at)gmail(dot)com> wrote:
> On Wed, Nov 13, 2013 at 9:37 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>> handyrep(at)john:~/handyrep$ pg_isready --version
>> pg_isready (PostgreSQL) 9.3.1
>>
>> handyrep(at)john:~/handyrep$ pg_isready -h john -p 5432 -U postgres -d
>> postgres -q
>> pg_isready: missing "=" after "postgres" in connection info string
>>
>> handyrep(at)john:~/handyrep$ pg_isready --host=john --port=5432
>> --user=postgres --dbname=postgres
>> pg_isready: missing "=" after "postgres" in connection info string
>>
>> handyrep(at)john:~/handyrep$ pg_isready -h john -p 5432 -U postgres
>> john:5432 - accepting connections
>>
>> so apparently the -d option:
>>
>> a) doesn't work, and
>> b) doesn't do anything
>>
>> I suggest simply removing it from the utility.
>>
>> I'll note that the -U option doesn't appear to do anything relevant
>> either, but at least it doesn't error unnecessarily:
>>
>> handyrep(at)john:~/handyrep$ pg_isready -h john -p 5432 -U no_such_user
>> john:5432 - accepting connections
>>
>
> The attached patch fix it.
Well, there still seem to be some problems.
[rhaas pgsql]$ src/bin/scripts/pg_isready -d host=sgdasasg
/tmp:5432 - no attempt
I added some debugging instrumentation and it looks like there's a
problem with this code:
if (strcmp(def->keyword, "hostaddr") == 0 ||
strcmp(def->keyword, "host") == 0)
The problem is that we end up executing the code contained in that
block twice, once for host and once for hostaddr. Thus:
[rhaas pgsql]$ src/bin/scripts/pg_isready -d host=sgdasasg
def->keyword=host pghost_str=sgdasasg
def->keyword=hostaddr pghost_str=/tmp
def->keyword=port pgport_str=5432
/tmp:5432 - no attempt
Oops.
Nevertheless, that's kind of a separate problem, so we could address
in a separate commit. But even ignoring that, this still isn't right:
according to the fine documentation, -d will be expanded not only if
it contains an =, but also if it starts with a valid URI prefix. This
would break that documented behavior.
http://www.postgresql.org/docs/9.3/static/app-pg-isready.html
If you submit an updated patch, please fix the comment just above the
code you're changing to more accurately reflect what this does.
The number of bugs in pg_isready certainly seems out of proportion to
the surface area of the problem it solves. Whee!
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2013-11-19 14:57:20 | Re: better atomics - v0.2 |
Previous Message | Magnus Hagander | 2013-11-19 14:50:40 | Re: New option for pg_basebackup, to specify a different directory for pg_xlog |