Обсуждение: [PATCH] Support for ping method.

Hi,

Attached is a patch that adds support for a ping method on the
AbstractJdbc2Connection.  This is useful for JBoss (and other
containers) where the container needs to check if the connection is
alive before passing it to the caller.  Currently JBoss does a "SELECT
1", but using the new ping method there is a boost in performance.
Opening and closing 1000 connections from a datasource:

PGTest1 (SELECT 1) 1000 iterations: 2491ms

PGTest2 (ping())   1000 iterations: 63ms

All is does is send a sendSync and wait for a valid response.

Regards,
Michael Barker.

On Thu, 6 Apr 2006, Michael Barker wrote:

> Attached is a patch that adds support for a ping method on the
> AbstractJdbc2Connection.  This is useful for JBoss (and other
> containers) where the container needs to check if the connection is
> alive before passing it to the caller.  Currently JBoss does a "SELECT
> 1", but using the new ping method there is a boost in performance.
> Opening and closing 1000 connections from a datasource:
>

There are a number of problems with this patch.  In no particular order...

1) publicly accessible postgresql specific functions should be exported
in the org.postgresql.PGConnection interface or similar.  Client code
should not attempt to cast a connection to any other interface.

2) The ping method in QueryExecutorImpl must be synchronized to avoid
interfering with other threads use of the backend connection.

3) The error handling is non-existent.  It swallows all kinds of errors
and leaves the protocol stream in an unknown state leaving the caller
responsible for cleanup.  At worst it should close the connection and
throw some sort of exception.

4) It is possible that the first response from a Sync message on a
valid connection will not be a ReadyForQuery message.  Consider the
case of asynchronous Notify messages appearing.  These must be
processed by the driver as it waits for the ReadyForQuery
message to arrive.

5) It lacks V2 protocol support.  While V2 does not have a Sync message,
it does have something similar.  By sending a query of the empty string
you will receive an EmptyQuery message immediately that should not require
any significant backend processing.

Kris Jurka

> On Thu, 6 Apr 2006, Michael Barker wrote:
>
>> Attached is a patch that adds support for a ping method on the
>> AbstractJdbc2Connection.  This is useful for JBoss (and other
>> containers) where the container needs to check if the connection is
>> alive before passing it to the caller.  Currently JBoss does a "SELECT
>> 1", but using the new ping method there is a boost in performance.
>> Opening and closing 1000 connections from a datasource:
>>
>
> There are a number of problems with this patch.  In no particular order...
>

6) I'm not sure what guarantees ping is trying to make.  Is it solely
to ensure that the backend is still connected or is it to ensure that the
connection state is ready to receive new commands.  Consider the
following as the attached test case demonstrates:

conn.setAutoCommit(false);
conn.createStatement().execute("SELECT 1/0"); // Forces transaction abort
conn.ping(); // Returns 0 saying connection is "good".
conn.createStatement().execute("SELECT 1"); // Fails b/c in aborted txn.

If you need the stronger guarantee of transaction state validity then I
think you could try the V2 EmptyQuery attempt on the V3 protocol as well.

When JDK1.6 comes out I imagine that this will be replaced by the
Connection.isValid method which does not seem to require anything
beyond a current backend connection.

http://download.java.net/jdk6/docs/api/java/sql/Connection.html#isValid(int)

Kris Jurka

In a connection pool, generally what has happened is someone or
something takes the database down for maintenance and then every single
dependency downstream takes a long time to recover, or the maximum time
you're allowed to hold it has expired, or some network event has
occurred.  The JBoss connection validators let the connection pool
bounce back from database outages rather quickly.  We don't want the
server to do any undue processing (faster is better).  Mostly we'd like
to send a byte or two and get a byte or two.  If there is a more serious
outage than an invalid connection then it is likely we'll rightly get an
exception when the connection is used or re-established and that's okay.

-Andy

Kris Jurka wrote:
>
>
>> On Thu, 6 Apr 2006, Michael Barker wrote:
>>
>>> Attached is a patch that adds support for a ping method on the
>>> AbstractJdbc2Connection.  This is useful for JBoss (and other
>>> containers) where the container needs to check if the connection is
>>> alive before passing it to the caller.  Currently JBoss does a "SELECT
>>> 1", but using the new ping method there is a boost in performance.
>>> Opening and closing 1000 connections from a datasource:
>>>
>>
>> There are a number of problems with this patch.  In no particular
>> order...
>>
>
> 6) I'm not sure what guarantees ping is trying to make.  Is it solely to
> ensure that the backend is still connected or is it to ensure that the
> connection state is ready to receive new commands.  Consider the
> following as the attached test case demonstrates:
>
> conn.setAutoCommit(false);
> conn.createStatement().execute("SELECT 1/0"); // Forces transaction abort
> conn.ping(); // Returns 0 saying connection is "good".
> conn.createStatement().execute("SELECT 1"); // Fails b/c in aborted txn.
>
> If you need the stronger guarantee of transaction state validity then I
> think you could try the V2 EmptyQuery attempt on the V3 protocol as well.
>
> When JDK1.6 comes out I imagine that this will be replaced by the
> Connection.isValid method which does not seem to require anything beyond
> a current backend connection.
>
> http://download.java.net/jdk6/docs/api/java/sql/Connection.html#isValid(int)
>
>
> Kris Jurka
>
>
> ------------------------------------------------------------------------
>
> import java.sql.*;
>
> public class PingTest {
>
>     public static void main(String args[]) throws Exception {
>         Class.forName("org.postgresql.Driver");
>
>         Connection conn = DriverManager.getConnection("jdbc:postgresql://localhost:5820/jurka","jurka","");
>
>         conn.setAutoCommit(false);
>         Statement stmt = conn.createStatement();
>         try {
>             stmt.execute("SELECT 1/0");
>         } catch (SQLException sqle) {
>             sqle.printStackTrace();
>         }
>
>         System.out.println(((org.postgresql.jdbc2.AbstractJdbc2Connection)conn).ping());
>
>         stmt.execute("SELECT 1");
>     }
> }

Hi Kris,

Dump the patch, we will use an empty select.

Regards,
Michael Barker.

On Mon, 2006-04-17 at 01:37 -0500, Kris Jurka wrote:
>
> On Thu, 6 Apr 2006, Michael Barker wrote:
>
> > Attached is a patch that adds support for a ping method on the
> > AbstractJdbc2Connection.  This is useful for JBoss (and other
> > containers) where the container needs to check if the connection is
> > alive before passing it to the caller.  Currently JBoss does a "SELECT
> > 1", but using the new ping method there is a boost in performance.
> > Opening and closing 1000 connections from a datasource:
> >
>
> There are a number of problems with this patch.  In no particular order...
>
> 1) publicly accessible postgresql specific functions should be exported
> in the org.postgresql.PGConnection interface or similar.  Client code
> should not attempt to cast a connection to any other interface.
>
> 2) The ping method in QueryExecutorImpl must be synchronized to avoid
> interfering with other threads use of the backend connection.
>
> 3) The error handling is non-existent.  It swallows all kinds of errors
> and leaves the protocol stream in an unknown state leaving the caller
> responsible for cleanup.  At worst it should close the connection and
> throw some sort of exception.
>
> 4) It is possible that the first response from a Sync message on a
> valid connection will not be a ReadyForQuery message.  Consider the
> case of asynchronous Notify messages appearing.  These must be
> processed by the driver as it waits for the ReadyForQuery
> message to arrive.
>
> 5) It lacks V2 protocol support.  While V2 does not have a Sync message,
> it does have something similar.  By sending a query of the empty string
> you will receive an EmptyQuery message immediately that should not require
> any significant backend processing.
>
> Kris Jurka
>