adutra commented on code in PR #3566:
URL: https://github.com/apache/polaris/pull/3566#discussion_r2741241710
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -312,7 +312,8 @@ private boolean isRetryable(SQLException e) {
// Additionally, one might check for specific error messages or other
conditions
return e.getMessage().toLowerCase(Locale.ROOT).contains("connection
refused")
- || e.getMessage().toLowerCase(Locale.ROOT).contains("connection
reset");
+ || e.getMessage().toLowerCase(Locale.ROOT).contains("connection reset")
+ || e.getMessage().toLowerCase(Locale.ROOT).contains("acquisition
timeout");
Review Comment:
I tend to agree with @dimas-b here.
Increasing the acquisition timeout seems better, because it simply means
each thread will stay longer on the waiting line during spikes, but they will
eventually get a connection. This is efficient and preserves queue fairness.
Instead, if you keep the timeout short and implement a retry, threads will
get kicked out of the waiting line during spikes. They will throw an exception
(which is expensive), and then try to re-enter the queue. This effectively
turns your queuing system into a polling system. Threads lose their original
position on the line, which hurts queue fairness.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
