Re: Concurrent use of isValid()

On Mon, Mar 10, 2014 at 8:10 PM, Hannes Erven <hannes(at)erven(dot)at> wrote:

> Hi folks,
>
>
> in one of my applications, I recently decided to use Connection.isValid()
> to check and reestablish, if necessary, a connection to the DB. This
> application is a Eclipse RCP desktop app, built on top of Hibernate, and
> pools all its read-only workloads (data views that mostly refresh one
> single record at a time) through a single Connection object per JVM. These
> connections are again routed through a pgbouncer statement pool to reach
> the backends.
>
> According to http://jdbc.postgresql.org/documentation/92/thread.html ,
> pgjdbc is thread-safe, and hence a Connection object can be used
> concurrently between multiple threads.
>
>
> Interesting piece of documentation. I wouldn't exactly call that thread
safe other than the notion that it will block instead of running multiple
threads through the connection.

> In my code, I recently introduced an interesting bug as follows:
>
> - whenever a new read-only Hibernate session needs to be created, check
> whether there already is a connection established. If there is, issue
> Connection.isValid(SHORT_TIMEOUT) and reestablish if necessary, else use
> the existing connection.
>
> - sometimes, random queries (mostly ones that load larger sets of data)
> would throw an Exception like this:
> Caused by: org.postgresql.util.PSQLException: FEHLER: storniere Anfrage
> wegen Benutzeraufforderung
> (Query was canceled due to user's request)
> org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(
> QueryExecutorImpl.java:2161)
> org.postgresql.core.v3.QueryExecutorImpl.processResults(
> QueryExecutorImpl.java:1890)
> org.postgresql.core.v3.QueryExecutorImpl.execute(
> QueryExecutorImpl.java:255)
> org.postgresql.jdbc2.AbstractJdbc2Statement.execute(
> AbstractJdbc2Statement.java:559)
> org.postgresql.jdbc2.AbstractJdbc2Statement.executeWithFlags(
> AbstractJdbc2Statement.java:417)
> org.postgresql.jdbc2.AbstractJdbc2Statement.executeQuery(
> AbstractJdbc2Statement.java:302)
> org.hibernate.jdbc.AbstractBatcher.getResultSet(AbstractBatcher.java:208)
>
>
> The reason for that is that one thread is already executing the
> data-loading query when another thread calls isValid() with a timeout that
> is shorter than the time it takes the already-processing-query to complete.
> Hence, isValid() must wait, cannot complete within its limit and fails,
> *canceling the original query*.
>
> Interestingly, the Exception is logged on the server and also on the
> client in relation to the longer-running query. At least with my setup, the
> correlation to isValid() could not be made through the Exception traces,
> but required extensive digging through the code.
>
>
> I now understand what Kris Jurka meant by "isValid is broken" (
> http://postgresql.1045698.n5.nabble.com/Connection-isValid-
> int-timeout-implementation-td5711754.html )... and this probably isn't by
> any means a fault in pgjdbc.
>
>
> So, I'd ask you for suggestions on
> - should the pgjdbc concurrency documentation be improved to suggest
> calling isValid() on an concurrently active connection might cancel the
> current command
>
> certainly the documentation should be modified. I don't think we test it
well for concurrency

> - should isValid() be modified so it doesn't cancel previously running
> commands
>
> - should isValid() be modified so it behaves more like
> st = createStatement();
> st.setStatementTimeout(X);
> st.execute("SELECT 1");
> (e.g. if the connection is in-use, wait for it to become free and only
> start the timeout when the SELECT 1 command starts?)
>
>
This last suggestion probably makes the most sense.

- is there any sensible way to combine isValid() and concurrent queries,
> especially ones that might take longer that the isValid() timeout?
>
>
> - is it reasonable to share a single per-JVM connection over all threads
> or is it better practice to let all the data views hit pgbouncer?
> The data views bulk load the data to be displayed on initialization, but
> then just refresh one single record at a time.
> (I'm expecting about 30 clients with 10 different data views each, so now
> I have 30 connections hitting pgbouncer, but I could also change it to 300
> connections to pgbouncer.)
>
>
>
> For the moment, my fix is to pass a timeout to isValid() that is way
> longer than any of the bulk data load queries should take, and I'm thinking
> about removing isValid() altogether.
>
> The isValid() calls were introduced because at one point the connections
> to pgbouncer were cut due to an accidentally stopped service and then all
> app instances had to be restarted manually, instead of just automatically
> reconnecting.
>
>
> Thanks for the detailed analysis

Dave

>
> Thanks for any suggestions and insights,
> best regards
>
> -hannes
>
>
> --
> Sent via pgsql-jdbc mailing list (pgsql-jdbc(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-jdbc
>