From: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
---|---|
To: | Phil Sorber <phil(at)omniti(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org, Thom Brown <thom(at)linux(dot)com> |
Subject: | Re: [WIP] pg_ping utility |
Date: | 2012-10-23 22:12:37 |
Message-ID: | 50871655.8070607@gmx.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 10/22/12 11:47 AM, Phil Sorber wrote:
> On Sun, Oct 21, 2012 at 6:20 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Phil Sorber <phil(at)omniti(dot)com> writes:
>>> Here is the new patch. I renamed the utility from pg_ping to pingdb to
>>> go along with the naming convention of src/bin/scripts.
>>
>> Uh, no, that's not a step forward. Leaving out a "pg" prefix from those
>> script names is universally agreed to have been a mistake. We've not
>> felt that changing the legacy names is worth the amount of pain it'd
>> cause, but that doesn't mean that we should propagate the mistake into
>> brand new executable names.
>>
>> regards, tom lane
>
> Ok. I asked about this and got no response so I assume there were no
> strong opinions. I have reverted to the pg_ping name. Patches
> attached.
Quick review ...
Code:
*************** install: all installdirs
*** 54,59 ****
--- 55,61 ----
$(INSTALL_PROGRAM) clusterdb$(X) '$(DESTDIR)$(bindir)'/clusterdb$(X)
$(INSTALL_PROGRAM) vacuumdb$(X) '$(DESTDIR)$(bindir)'/vacuumdb$(X)
$(INSTALL_PROGRAM) reindexdb$(X) '$(DESTDIR)$(bindir)'/reindexdb$(X)
+ $(INSTALL_PROGRAM) pg_ping$(X) '$(DESTDIR)$(bindir)'/pg_ping$(X)
installdirs:
$(MKDIR_P) '$(DESTDIR)$(bindir)'
Whitespace misaligned
+ exit(3); // Return UNKNOWN
No // comments.
+ while (NULL != conn_opt_ptr->keyword)
Better writte as
while (conn_opt_ptr->keyword != NULL)
or
while (conn_opt_ptr->keyword)
Also, it seems that about 75% of the patch is connection options processing. How about
we get rid of all that and just have them specify a connection string? It would be a break
with tradition, but maybe it's time for something new.
Functionality:
I'm missing the typical ping functionality to ping continuously. If we're going to call
it pg_ping, it ought to do something similar to ping, I think.
From | Date | Subject | |
---|---|---|---|
Next Message | Christopher Browne | 2012-10-23 22:22:44 | Re: [WIP] pg_ping utility |
Previous Message | Murphy, Kevin | 2012-10-23 22:12:12 | Re: Very minor feature suggestion |