(2012/05/01 0:30), Ryan Kelly wrote:
On Mon, Apr 30, 2012 at 09:02:33AM -0400, Alvaro Herrera wrote:
Well, do *you* want it?
Of course. That way I can stop patching my psql and go back to using the
one that came with my release :)

Here is result of my review of v4 patch.

Submission
----------
- The patch is in context diff format
- It needs rebase for HEAD, but patch command takes care automatically.
- Make finishes cleanly, and all regression tests pass

Functionality
-------------
After applying this patch, I could cancel connection attempt at start-up by pressing Ctrl+C on terminal just after invoking psql command with an unused IP address. Without this patch, such attempt ends up with error such as "No route to host" after uninterruptable few seconds (maybe the duration until error would depend on environment).

Connection attempt by \connect command could be also canceled by pressing Ctrl+C on psql prompt.

In addition, I tried setting PGCONNECT_TIMEOUT to 0 (infinite), but psql gave up after few seconds, for both start-up and re-connect. Is this intentional behavior?

Detail of changes
-----------------
Basically this patch consists of three changes:

1) defer setup of cancel handler until start-up connection has established
2) new libpq API PQconnectTimeout which returns connect_timeout value of current session 3) use asynchronous connection API at \connect psql command, this requires API added by 2)

These seem reasonable to achieve canceling connection attempt at start-up and \connect, but, as Ryan says, codes added to do_connect might need more simplification.

I have some random comments.

- Checking status by calling PQstatus just after PQconnectStartParams is necessary. - Copying only "select()" part of pqSocketPoll seems not enough, should we use poll(2) if it is supported? - Don't we need to clear error message stored in PGconn after PQconnectPoll returns OK status, like connectDBComplete?

Regards,
--
Shigeru HANADA

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to