Core: Use Failsafe in ClientPoolImpl retry logic#10458
Core: Use Failsafe in ClientPoolImpl retry logic#10458amogh-jahagirdar wants to merge 1 commit intoapache:mainfrom
Conversation
amogh-jahagirdar
force-pushed
the
introduce-failsafe
branch
from
32e506a to
c0554b6
Compare
| AtomicReference<C> clientReference = new AtomicReference<>(); | ||
| clientReference.set(get()); | ||
|
|
||
| RetryPolicy<Object> retryPolicy = | ||
| RetryPolicy.builder() | ||
| .handle(reconnectExc) | ||
| .withMaxRetries(retry ? maxRetries : 0) | ||
| .withDelay(Duration.ofMillis(connectionRetryWaitPeriodMs)) | ||
| .onRetry( | ||
| exc -> { | ||
| C currentClient = clientReference.get(); | ||
| clientReference.set(reconnect(currentClient)); | ||
| }) | ||
| .build(); |
There was a problem hiding this comment.
This change will also impact the JDBC catalog, since that uses ClientPoolImpl as well. CC @jbonofre if you were interested in taking a look at this, and would like your perspective on introducing Failsafe here.
|
@amogh-jahagirdar: Is there a way to avoid changing the type of the exception thrown? |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
2 participants
This change is stemming from #10433 (comment).
This PR introduces Failsafe https://failsafe.dev/ as a dependency and integrates that into
ClientPoolImplwhere there is currently custom retry logic. The intent is to simplify the custom retry logic that currently exists by just using this dependency, which could also be used in other areas in the Iceberg codebase.