Thread: Statement.cancel() may cancel queries in the future
With a CVS HEAD driver, this code intermittently fails:
public void testCancel() throws Exception {
Connection con = TestUtil.openDB();
Statement stmt = con.createStatement();
for (int i = 0; i < 100; ++i) {
stmt.executeQuery("select version()").close();
// At this point, we are not running a query
// so cancel() should be a no-op.
stmt.cancel();
}
}
with:
[junit] java.sql.SQLException: ERROR: Query was cancelled.
[junit] at org.postgresql.core.QueryExecutor.executeV2(QueryExecutor.java:289)
[junit] at org.postgresql.core.QueryExecutor.execute(QueryExecutor.java:105)
[junit] at org.postgresql.core.QueryExecutor.execute(QueryExecutor.java:43)
[junit] at org.postgresql.jdbc1.AbstractJdbc1Statement.execute(AbstractJdbc1Statement.java:522)
[junit] at org.postgresql.jdbc2.AbstractJdbc2Statement.execute(AbstractJdbc2Statement.java:48)
[junit] at org.postgresql.jdbc1.AbstractJdbc1Statement.execute(AbstractJdbc1Statement.java:288)
[junit] at org.postgresql.test.jdbc2.MiscTest.testCancel(MiscTest.java:111)
(you may need to run it a few times to see this, it's intermittent).
I'm guessing that what's happening here is that we have:
send query #1 to backend instance A
receive results
open new connection to backend instance B
send cancel on new connection
close new connection
send query #2 to backend instance A
<cancel is delivered to A>
query #2 gets cancelled
I've tried adding an in-progress-query flag to the Statement impl and have
cancel() only actually send the cancel if there is an outstanding query, and
this seems to work ok for the above case. However I don't think this handles
the case where we complete the query and start a new one, just after sending
a cancel but before the cancel is delivered, i.e.:
thread 1: send query #1 to backend
thread 2: send cancel request
thread 1: query completes without being cancelled
(*)
thread 1: send query #2 to backend
<cancellation request arrives>
thread 1: query #2 gets cancelled
We want to block thread 1 at the point marked (*) until we know the cancel
has been processed and it is safe to send another query. However, looking at
the backend code query-cancel is implemented by B sending a signal to A, and
if A is not executing a query the signal is silently eaten. So I'm not sure
how to eliminate this race as we have no way of working out when the signal
has arrived in the above case.
Anyone have ideas on how to handle this?
-O
Oliver Jowett <oliver@opencloud.com> writes:
> We want to block thread 1 at the point marked (*) until we know the cancel
> has been processed and it is safe to send another query. However, looking at
> the backend code query-cancel is implemented by B sending a signal to A, and
> if A is not executing a query the signal is silently eaten. So I'm not sure
> how to eliminate this race as we have no way of working out when the signal
> has arrived in the above case.
When JDBC issues a cancel request, does it simply fire off a few bytes
and close the connection? You could make things a lot closer to
synchronous if you wait for the postmaster to drop the connection,
instead. That would guarantee that the postmaster has completed
executing kill(). I suppose actual delivery of the signal to the
backend might not have happened yet, but it's hard to believe that
it could be postponed past the backend's next successful execution of
recv().
regards, tom lane
On Mon, Sep 15, 2003 at 12:01:47PM -0400, Tom Lane wrote: > Oliver Jowett <oliver@opencloud.com> writes: > > We want to block thread 1 at the point marked (*) until we know the cancel > > has been processed and it is safe to send another query. However, looking at > > the backend code query-cancel is implemented by B sending a signal to A, and > > if A is not executing a query the signal is silently eaten. So I'm not sure > > how to eliminate this race as we have no way of working out when the signal > > has arrived in the above case. > > When JDBC issues a cancel request, does it simply fire off a few bytes > and close the connection? Indeed it does. > You could make things a lot closer to > synchronous if you wait for the postmaster to drop the connection, > instead. That would guarantee that the postmaster has completed > executing kill(). I suppose actual delivery of the signal to the > backend might not have happened yet, but it's hard to believe that > it could be postponed past the backend's next successful execution of > recv(). That sounds reasonable. I'll put together a patch to do that. Thanks. -O
Oliver Jowett <oliver@opencloud.com> writes:
> On Mon, Sep 15, 2003 at 12:01:47PM -0400, Tom Lane wrote:
>> You could make things a lot closer to
>> synchronous if you wait for the postmaster to drop the connection,
>> instead. That would guarantee that the postmaster has completed
>> executing kill(). I suppose actual delivery of the signal to the
>> backend might not have happened yet, but it's hard to believe that
>> it could be postponed past the backend's next successful execution of
>> recv().
> That sounds reasonable. I'll put together a patch to do that.
It looks like libpq has the same issue, as PQrequestCancel() does not
wait around for the postmaster to drop the connection. Anyone want
to fix that?
regards, tom lane
Is this something for 7.5? --------------------------------------------------------------------------- Tom Lane wrote: > Oliver Jowett <oliver@opencloud.com> writes: > > On Mon, Sep 15, 2003 at 12:01:47PM -0400, Tom Lane wrote: > >> You could make things a lot closer to > >> synchronous if you wait for the postmaster to drop the connection, > >> instead. That would guarantee that the postmaster has completed > >> executing kill(). I suppose actual delivery of the signal to the > >> backend might not have happened yet, but it's hard to believe that > >> it could be postponed past the backend's next successful execution of > >> recv(). > > > That sounds reasonable. I'll put together a patch to do that. > > It looks like libpq has the same issue, as PQrequestCancel() does not > wait around for the postmaster to drop the connection. Anyone want > to fix that? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Is this something for 7.5?
I think it qualifies as a bug fix, so if anyone gets around to it in
time, it ought to go in 7.4. Do you want to put it on TODO?
>> It looks like libpq has the same issue, as PQrequestCancel() does not
>> wait around for the postmaster to drop the connection. Anyone want
>> to fix that?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Is this something for 7.5? > > I think it qualifies as a bug fix, so if anyone gets around to it in > time, it ought to go in 7.4. Do you want to put it on TODO? > > >> It looks like libpq has the same issue, as PQrequestCancel() does not > >> wait around for the postmaster to drop the connection. Anyone want > >> to fix that? I will add it as an open item for 7.4. Someone already posted a jdbc fix for it. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073