You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "smiroslav (via GitHub)" <gi...@apache.org> on 2025/12/16 14:18:35 UTC
[PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
smiroslav opened a new pull request, #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663
(no comment)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626598220
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
Yes, definitely, thank you!
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626020067
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
@smiroslav looking at the [code referenced by the stacktrace](https://github.com/reactor/reactor-core/blob/60e3e4d8e5f10992d8d7acee75a5caa58eda1e68/reactor-core/src/main/java/reactor/core/publisher/BlockingSingleSubscriber.java#L128) you shared in OAK-12039, I don't think this check will work.
As far as I can see, an `IllegalStateException` without any causes is constructed if a timeout occurs.
The case where an `IOException` is expected as the cause _may_ work. I cannot tell immediately from the code. A few more stack traces/thread dumps may be helpful to get a better understanding of the failing code-paths.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3674295968
> I decided not to update reactor-core ...
I hope you're still merging the rest of the PR, because eventually we will update azure and reactor 🙂
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626312739
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
@jsedding thanks for chiming in.
I have updated the issue description to include the code where the timeout exception is thrown and a discussion thread.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav merged PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626020067
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
@smiroslav looking at the [code referenced by the stacktrace](https://github.com/reactor/reactor-core/blob/60e3e4d8e5f10992d8d7acee75a5caa58eda1e68/reactor-core/src/main/java/reactor/core/publisher/BlockingSingleSubscriber.java#L128) you shared in OAK-12039, I don't think this check will work.
As far as I can see, an `IllegalStateException` without any causes is constructed if a timeout occurs.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3665813244
I have changed the test to cause ```TimeoutException``` instead of mocking it.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626448569
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
I wonder why the cause is not shown in the stack trace you shared in the JIRA ticket? Could that stacktrace be from an older version of the reactor-core library?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626442804
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
@smiroslav I completely missed that they create a new `TimeoutException` and set it as the cause. I only saw the new `IllegalStateException`. Sorry, my bad!
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626910736
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
Looks good now!
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3665815945
> I think the fix should also be added to V8 implementation.
@ierandra will address it in a separate PR
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "jsedding (via GitHub)" <gi...@apache.org>.
jsedding commented on code in PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#discussion_r2626459711
##########
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureRepositoryLock.java:
##########
@@ -211,6 +213,26 @@ private boolean isInError() {
return inError;
}
+ /**
+ * Checks if the exception is a transient client-side exception that should be retried.
+ * This includes timeouts and IO/network errors that can occur when communicating with Azure.
+ * <p>
+ * Per Azure SDK documentation, the timeout parameter causes a RuntimeException to be raised.
+ * @param e the exception to check
+ * @return true if this is a transient exception that should be retried
+ */
+ private boolean isTransientClientSideException(Exception e) {
+ Throwable current = e;
+ while (current != null) {
+ if (current instanceof java.util.concurrent.TimeoutException ||
+ current instanceof java.io.IOException) {
+ return true;
+ }
+ current = current.getCause();
+ }
+ return false;
+ }
Review Comment:
Indeed, the `TimeoutException` cause was added [reactor-core 8331346](https://github.com/reactor/reactor-core/commit/8331346774c6fd397aa5646949d5d7800312bb60), which was first released in version v3.6.4.
I think you need to update `reactor-core` before your fix can work. WDYT?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "ierandra (via GitHub)" <gi...@apache.org>.
ierandra commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3664546232
I think the fix should also be added to V8 implementation.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3670898312
I decided not to update reactor-core, as the Azure documentation advises against using a version with a different major or minor version than the one specified in the Azure SDK dependencies.
https://learn.microsoft.com/en-us/azure/developer/java/sdk/troubleshooting-dependency-version-conflict#understand-compatible-dependency-versions
https://github.com/reactor/.github/blob/main/SUPPORT.adoc#our-policy-on-deprecations
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "ierandra (via GitHub)" <gi...@apache.org>.
ierandra commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3671080544
@smiroslav
> The AzureRepositoryLockV8 is already correctly handling client-side timeout exceptions in the correct way:
that is very strange because when we did the analysis we observe the same issue for customers running on v8
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3670910291
> > I think the fix should also be added to V8 implementation.
>
> @ierandra will address it in a separate PR
The AzureRepositoryLockV8 is already correctly handling client-side timeout exceptions in the correct way:
https://github.com/apache/jackrabbit-oak/pull/2667
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12039 Treat transient client-side exceptions as recoverable in AzureRepositoryLock [jackrabbit-oak]
Posted by "smiroslav (via GitHub)" <gi...@apache.org>.
smiroslav commented on PR #2663:
URL: https://github.com/apache/jackrabbit-oak/pull/2663#issuecomment-3671165158
@ierandra, looking at the code, all exceptions that happen in AzureRepositoryLockV8#refreshLease should be logged.
Do you perhaps have the stacktrace?
I suggest we move the discussion for v8 to the JIRA or #2667.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org