pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
6+ messages / 3 participants
[nested] [flat]
* [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
@ 2025-08-20 08:30 "vlsi (@vlsi)" <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: vlsi (@vlsi) @ 2025-08-20 08:30 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Previously, StatementCancelTimerTask might propagate runtime errors to java.util.Timer which causes the Timer to cancel itself. We don't want that behavior, so we should avoid throwing errors from TimerTask implementations.
Note statement.cancelIfStillNeeded has already been ignoring SQLExceptions from statement.cancel() for quite a while, however, it did not ignore runtime errors.
Fixes https://github.com/pgjdbc/pgjdbc/issues/3530
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
@ 2025-08-20 12:31 "lfgcampos (@lfgcampos)" <[email protected]>
4 siblings, 0 replies; 6+ messages in thread
From: lfgcampos (@lfgcampos) @ 2025-08-20 12:31 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Hi @vlsi, those are amazing news!
Is there any explanation though why it works on `v42.7.4` but not on others?
I tried to look into the changes and changelog but can't really understand what/when this problem was introduced.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
@ 2025-08-21 09:42 "davecramer (@davecramer)" <[email protected]>
4 siblings, 0 replies; 6+ messages in thread
From: davecramer (@davecramer) @ 2025-08-21 09:42 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
LGTM, this will be difficult to test.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
@ 2025-08-22 16:27 "vlsi (@vlsi)" <[email protected]>
4 siblings, 0 replies; 6+ messages in thread
From: vlsi (@vlsi) @ 2025-08-22 16:27 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I'm leaning towards adding a test with mock that triggers an exception from `.cancel()`
Will try adding it shortly.
----
>Is there any explanation though why it works on v42.7.4 but not on others?
https://github.com/pgjdbc/pgjdbc/commit/bac3d0add5626006ce41db53618da2190bf00910 landed at 42.7.6, and it changed `int` to `byte[]`. Previously, an unset `cancelKey` was just `0`, so it did not cause any harm. After PR #3592 the `null` `byte[]` could cause NPE.
Interestingly, `castNonNull` caused NPE, so writing code in a way that does not require `castNotNull` seems the right way to go.
At the same time, backend should always supply `cancel key` in the startup messages, so I don't understand how is it possible that you have `null` `cancelKey`. I wonder if you could capture logging for `org.postgresql.core.QueryExecutorBase` in order to check if it does indeed end up with null cancel keys.
Do you use a load balancer?
It does not explain the reason 42.7.5 fails though. Just to double-check: does 42.7.5 fail for you as well?
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
@ 2025-08-22 17:03 "lfgcampos (@lfgcampos)" <[email protected]>
4 siblings, 0 replies; 6+ messages in thread
From: lfgcampos (@lfgcampos) @ 2025-08-22 17:03 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> I'm leaning towards adding a test with mock that triggers an exception from `.cancel()` Will try adding it shortly.
>
> > Is there any explanation though why it works on v42.7.4 but not on others?
>
> [bac3d0a](https://github.com/pgjdbc/pgjdbc/commit/bac3d0add5626006ce41db53618da2190bf00910) landed at 42.7.6, and it changed `int` to `byte[]`. Previously, an unset `cancelKey` was just `0`, so it did not cause any harm. After PR #3592 the `null` `byte[]` could cause NPE. Interestingly, `castNonNull` caused NPE, so writing code in a way that does not require `castNotNull` seems the right way to go.
>
> At the same time, backend should always supply `cancel key` in the startup messages, so I don't understand how is it possible that you have `null` `cancelKey`. I wonder if you could capture logging for `org.postgresql.core.QueryExecutorBase` in order to check if it does indeed end up with null cancel keys. Do you use a load balancer?
>
> It does not explain the reason 42.7.5 fails though. Just to double-check: does 42.7.5 fail for you as well?
We bumped from `~.4` to `~.7` so I can't say if the `~.5` works or not.
Some info that may or may not help:
- yes, we have as LB and RDS Proxy
- we are using a mix of quarkus/pure java, meaning our connections are also a mix of hikari and agroal
Regarding capturing the log for `org.postgresql.core.QueryExecutorBase`, that is a tricky one since the bump caused a couple of hours long outage on our prod
Also, quite interesting, it does not happen on dev - probably related to the timeout after all, it would at least explain it
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error
@ 2025-08-22 17:31 "vlsi (@vlsi)" <[email protected]>
4 siblings, 0 replies; 6+ messages in thread
From: vlsi (@vlsi) @ 2025-08-22 17:31 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> that is a tricky one since the bump caused a couple of hours long outage on our prod
Sorry to hear that.
I mean the log would be helpful for either old or new (after the cancellation issue gets fixed) version.
The old (e.g. 42.7.4) logs the key: https://github.com/pgjdbc/pgjdbc/blob/REL42.7.4/pgjdbc/src/main/java/org/postgresql/core/QueryExecut...
I wonder if `ckey={1}` comes out as `0`.
Sure there's no hurry to logging it, however, if the cancellation key is not set, then it might be the cancellation does not work in your case. It might be caused by the balancer chewing the cancellation key from within the startup message.
An alternative option would be trying to execute `pg_sleep(..)` **including** your balancers and proxies, to verify if you can get `PSQLException: ERROR: canceling statement due to user request` when `setQueryTimeout(...)` kicks in.
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-08-22 17:31 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-08-20 08:30 [pgjdbc/pgjdbc] PR #3778: fix: avoid IllegalStateException: Timer already cancelled when StatementCancelTimerTask.run throws a runtime error "vlsi (@vlsi)" <[email protected]>
2025-08-20 12:31 ` "lfgcampos (@lfgcampos)" <[email protected]>
2025-08-21 09:42 ` "davecramer (@davecramer)" <[email protected]>
2025-08-22 16:27 ` "vlsi (@vlsi)" <[email protected]>
2025-08-22 17:03 ` "lfgcampos (@lfgcampos)" <[email protected]>
2025-08-22 17:31 ` "vlsi (@vlsi)" <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox