I've tried to deal with some of these problems.

My patch have support for following things:

1. Check whether database instance is in the recovery/standby mode and
try to find another one if so.
2. Let cluster management software to have some time to promote one of
the standbys to master. I.e. there can be failover timeout specified to
allow retry after some time if no working master found.

Really there is room for some improvements in handling of connect
timeout (which became much more important thing when ability to try
next host appears). Now it is handled only by blocking-mode connect
functions, not by async state machine. But I decided to publish patch
without these improvements to get feedback from community.
A bit of testing on this turns up a problem.

Consider a connection string that specifies two hosts and a read/write connection:


If the first host is a healthy standby (meaning that I can connect to it but pg_is_in_recovery() returns 't'), the state machine will never move on to the second host.

The problem seems to be in PQconnectPoll() in the case for CONNECTION_AUTH_OK, specifically this code:

  /* We can release the address list now. */
  pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
  conn->addrlist = NULL;
  conn->addr_cur = NULL;

That frees up the list of alternative host addresses. The state machine then progresses to CONNECTION_CHECK_RO (which invokes pg_is_in_recovery()), then CONNECTION_CHECK_RW (waiting for the response from the server). Since we just connected to a standby replica, pg_is_in_recovery() returns 't' and the state changes to CONNECTION_NEEDED. The next call to try_next_address() will fail to find a next address because we freed the list in the case for CONNECTION_AUTH_OK.

A related issue: when the above sequence occurs, no error message is returned (because the case for CONNECTION_NEEDED thinks "an appropriate error message is already set up").

In short, if you successfully connect to standby replica (and specify readonly=0), the remaining hosts are ignored, even though one of those hosts is a master.

And one comment about the code itself - in connectDBStart(), you've added quite a bit of code to parse multiple hosts/ports. I would recommend adding a comment that shows the expected format, and then choosing better variable names (better than 'p', 'q', and 'r'); perhaps the variable names could refer to components of the connection string that you are parsing (like 'host', 'port', 'delimiter', ...). That would make the code much easier to read/maintain.


                -- Korry

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

Reply via email to