kfaraz commented on code in PR #14271:
URL: https://github.com/apache/druid/pull/14271#discussion_r1195918488
##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -165,14 +166,21 @@ public <T> T retryTransaction(final
TransactionCallback<T> callback, final int q
public final boolean isTransientException(Throwable e)
{
- return e != null && (e instanceof RetryTransactionException
- || e instanceof SQLTransientException
- || e instanceof SQLRecoverableException
- || e instanceof UnableToObtainConnectionException
- || e instanceof UnableToExecuteStatementException
- || connectorIsTransientException(e)
- || (e instanceof SQLException &&
isTransientException(e.getCause()))
- || (e instanceof DBIException &&
isTransientException(e.getCause())));
+ if (e == null) {
+ return false;
+ }
+ if (e.getMessage() != null &&
e.getMessage().contains(MAX_ALLOWED_PACKET_ERROR)) {
Review Comment:
For MySQL, the SQLState seems to be `S1000`, error code is `0`, and the root
cause exception is `PacketTooBigException`.
```
org.skife.jdbi.v2.exceptions.CallbackFailedException:
org.skife.jdbi.v2.exceptions.UnableToExecuteStatementException:
com.mysql.jdbc.PacketTooBigException: Packet for query is too large (9952 >
1024). You can change this value on the server by setting the
max_allowed_packet' variable.
```
The solution for this case is simply to check if the cause of
`UnableToExecuteStatementException`, which we should always do anyway because
`UnableToExecuteStatementException` is not transient by itself. Since
`PacketTooBigException` is not transient by any of our other rules, it would be
categorized correctly.
##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -165,14 +166,21 @@ public <T> T retryTransaction(final
TransactionCallback<T> callback, final int q
public final boolean isTransientException(Throwable e)
{
- return e != null && (e instanceof RetryTransactionException
- || e instanceof SQLTransientException
- || e instanceof SQLRecoverableException
- || e instanceof UnableToObtainConnectionException
- || e instanceof UnableToExecuteStatementException
- || connectorIsTransientException(e)
- || (e instanceof SQLException &&
isTransientException(e.getCause()))
- || (e instanceof DBIException &&
isTransientException(e.getCause())));
+ if (e == null) {
+ return false;
+ }
+ if (e.getMessage() != null &&
e.getMessage().contains(MAX_ALLOWED_PACKET_ERROR)) {
Review Comment:
Unfortunately, this is not the exception we had seen in prod. I tried
reproducing it by keeping a high `max_allowed_packet` on the mysql server but a
low value in the connection url, but that that again resulted in the same
`PacketTooLargeException`. In prod, we had seen a nesting of
`CallbackFailedException` -> `UnableToExecuteStatementException` ->
`SQLTransientConnectionException`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
