Re: [PDO] PostgreSQL auto-reconnect in PDO contructor for permanent connections: request for comments

[email protected] (Marc Barilley) Fri, 09 Apr 2010 16:41:17 +0200
Newsgroups php.pdo
Message-ID <[email protected]>
Here is a diff with my patch. It's in the -Bur format. Any comment is
welcome.

What it does is:
- define a new constant PDO::PGSQL_ATTR_PING_CONNECTION for the
driver-specific attributes
- this can be set to true or false ; default is true
- when set to true, it makes the driver perform a "select 1" on the link
to thoroughly test the connection


Marc



Wez Furlong a écrit :
> Honestly, the driver is broken if it does not correctly detect a dead
> connection, or if it blocks indefinitely.  It doesn't really matter
> what the user wants in that case; what they get is unusable without
> manually taking the same steps in their code.
>
> The postgres API is blocking by default.  You can configure a timeout
> in your connection string, but this does not apply to all cases (only
> connection time, IIRC).  In all other cases, the calls can block, and
> can block indefinitely depending on the nature of the disconnect, the
> locality of the server, the nature of the network hardware between the
> client and the server.
>
> --Wez.
>
> On Mar 31, 2010, at 7:25 PM, Marc Barilley wrote:
>
>> I don't think this is a bug per se, just a missing feature. Some
>> programmers may want a thorough check of the connection while some
>> may not. Both have their benefits and drawbacks. I think it is up to
>> the programmer to choose if the connection needs to be checked or not.
>> I'm curious to know if you have more precise informations about the
>> conditions when the API can block. This could help :) Thank you.
>>
>> Marc
>>
>>
>> Wez Furlong a écrit :
>>> This thread started out as a bug report with the liveness check in
>>> the postgres driver; the mechanism I'm using in an internal project,
>>> which is similar in architecture to PDO, is this, which has the
>>> advantage of not being a blocking operation--I've seen the postgres
>>> API block indefinitely in some stale/retired connection cases.  This
>>> code will give 5 attempts up to 5 seconds each to determine if the
>>> connection is still viable.
>>>
>>> --Wez.
>>>
>>>  int tries = 5;
>>>  PGresult *res;
>>>  struct pollfd pfd;
>>>  GET_DRV(drv);
>>>
>>>  PQsendQuery(D->server, "SELECT 1;");
>>>
>>>  while (tries--) {
>>>    res = PQgetResult(D->server);
>>>    if (res) {
>>>      break;
>>>    }
>>>    pfd.fd = PQsocket(D->server);
>>>    pfd.events = POLLIN|POLLHUP|POLLERR;
>>>    poll(&pfd, 1, 5000);
>>>  }
>>>  if (res && PQstatus(D->server) == CONNECTION_OK) {
>>>    PQclear(res);
>>>    return 1;
>>>  }
>>>  if (res) {
>>>    PQclear(res);
>>>  }
>>>  PQreset(D->server);
>>>  return 0;
>>>
>>>
>>>
>>
>
>
>
>
php-5.3.2.diff (text/x-patch, 3.9 KB)
diff -Bur orig/php-5.3.2/ext/pdo_pgsql/pdo_pgsql.c patch/php-5.3.2/ext/pdo_pgsql/pdo_pgsql.c
--- orig/php-5.3.2/ext/pdo_pgsql/pdo_pgsql.c	2010-01-03 10:23:27.000000000 +0100
+++ patch/php-5.3.2/ext/pdo_pgsql/pdo_pgsql.c	2010-04-09 16:13:06.000000000 +0200
@@ -13,6 +13,7 @@
   | [email protected] so we can mail you a copy immediately.               |
   +----------------------------------------------------------------------+
   | Author: Edin Kadribasic <[email protected]>                             |
+  |          Marc Barilley <[email protected]>                           |
   +----------------------------------------------------------------------+
 */
 
@@ -86,6 +87,7 @@
 PHP_MINIT_FUNCTION(pdo_pgsql)
 {
 	REGISTER_PDO_CLASS_CONST_LONG("PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT", PDO_PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT);
+	REGISTER_PDO_CLASS_CONST_LONG("PGSQL_ATTR_PING_CONNECTION", PDO_PGSQL_ATTR_PING_CONNECTION);
 	php_pdo_register_driver(&pdo_pgsql_driver);
 	return SUCCESS;
 }
diff -Bur orig/php-5.3.2/ext/pdo_pgsql/pgsql_driver.c patch/php-5.3.2/ext/pdo_pgsql/pgsql_driver.c
--- orig/php-5.3.2/ext/pdo_pgsql/pgsql_driver.c	2010-02-03 20:48:04.000000000 +0100
+++ patch/php-5.3.2/ext/pdo_pgsql/pgsql_driver.c	2010-04-09 16:12:52.000000000 +0200
@@ -15,6 +15,7 @@
   | Authors: Edin Kadribasic <[email protected]>                            |
   |          Ilia Alshanestsky <[email protected]>                        |
   |          Wez Furlong <[email protected]>                                   |
+  |          Marc Barilley <[email protected]>                           |
   +----------------------------------------------------------------------+
 */
 
@@ -457,6 +458,12 @@
 static int pdo_pgsql_check_liveness(pdo_dbh_t *dbh TSRMLS_DC)
 {
 	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
+	// This sends a kind of ping over the connection to make sure we catch CONNECTION_BAD
+	if (H->ping_connection) {
+		PGresult *pg_result;
+		pg_result = PQexec(H->server, "select 1");
+		PQclear(pg_result);
+	}
 	if (PQstatus(H->server) == CONNECTION_BAD) {
 		PQreset(H->server);
 	}
@@ -635,6 +642,10 @@
 			return 1;
 #endif
 
+		case PDO_PGSQL_ATTR_PING_CONNECTION:
+			convert_to_boolean(val);
+			((pdo_pgsql_db_handle *)dbh->driver_data)->ping_connection = Z_BVAL_P(val);
+			return 1;
 		default:
 			return 0;
 	}
@@ -662,6 +673,7 @@
 	int ret = 0;
 	char *conn_str, *p, *e;
 	long connect_timeout = 30;
+	int ping_connection = 1;
 
 	H = pecalloc(1, sizeof(pdo_pgsql_db_handle), dbh->is_persistent);
 	dbh->driver_data = H;
@@ -680,7 +692,9 @@
 
 	if (driver_options) {
 		connect_timeout = pdo_attr_lval(driver_options, PDO_ATTR_TIMEOUT, 30 TSRMLS_CC);
+		ping_connection = pdo_attr_lval(driver_options, PDO_PGSQL_ATTR_PING_CONNECTION, 1 TSRMLS_CC);
 	}
+	H->ping_connection = ping_connection;
 
 	/* support both full connection string & connection string + login and/or password */
 	if (dbh->username && dbh->password) {
diff -Bur orig/php-5.3.2/ext/pdo_pgsql/php_pdo_pgsql_int.h patch/php-5.3.2/ext/pdo_pgsql/php_pdo_pgsql_int.h
--- orig/php-5.3.2/ext/pdo_pgsql/php_pdo_pgsql_int.h	2010-01-03 10:23:27.000000000 +0100
+++ patch/php-5.3.2/ext/pdo_pgsql/php_pdo_pgsql_int.h	2010-04-09 16:12:45.000000000 +0200
@@ -15,6 +15,7 @@
   | Authors: Edin Kadribasic <[email protected]>                            |
   |          Ilia Alshanestsky <[email protected]>                        |
   |          Wez Furlong <[email protected]>                                   |
+  |          Marc Barilley <[email protected]>                           |
   +----------------------------------------------------------------------+
 */
 
@@ -51,6 +52,7 @@
 	int		disable_native_prepares;
 #endif
 	unsigned int stmt_counter;
+	unsigned int ping_connection:1;
 } pdo_pgsql_db_handle;
 
 typedef struct {
@@ -93,6 +95,7 @@
 
 enum {
 	PDO_PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT = PDO_ATTR_DRIVER_SPECIFIC,
+	PDO_PGSQL_ATTR_PING_CONNECTION
 };
 
 struct pdo_pgsql_lob_self {