Radiancebobo opened a new pull request, #25939: URL: https://github.com/apache/pulsar/pull/25939
### Motivation Some asynchronous helper methods accept a `Supplier<CompletableFuture<T>>` and assume the supplier always returns a future. However, a supplier can also fail synchronously before creating the future. Before this change, those synchronous failures could bypass the expected asynchronous error handling path: - `FutureUtil.Sequencer.sequential` could throw directly instead of returning a failed future. - `FutureUtil.composeAsync` could throw inside the executor task and leave the returned future incomplete. - `Futures.executeWithRetry` in managed-ledger could throw before entering the retry state machine, so a retryable transient failure from the operation setup path would not be retried. These helpers should preserve the `CompletableFuture` contract: callers should receive a completed or exceptionally completed future, rather than having synchronous exceptions escape or leave the returned future hanging. ### Modifications This change updates the future supplier handling paths to convert synchronous supplier failures into failed futures: - Added a private helper in `FutureUtil` to safely invoke `Supplier<CompletableFuture<T>>`. - Updated `FutureUtil.Sequencer.sequential` to use the safe supplier invocation path. - Updated `FutureUtil.composeAsync` to complete the returned future exceptionally when the supplier fails synchronously. - Updated managed-ledger `Futures.executeWithRetry` to route synchronous supplier failures through the existing retry logic. - Added null-future handling so suppliers that return `null` complete exceptionally with `NullPointerException` instead of causing an unchecked synchronous failure. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change added tests and can be verified as follows: - Added `FutureUtilTest.testSequencerReturnsFailedFutureWhenTaskThrowsSynchronously` - Added `FutureUtilTest.testComposeAsyncReturnsFailedFutureWhenSupplierThrowsSynchronously` - Added `FuturesTest.testExecuteWithRetryHandlesSynchronousFailure` Local verification: - `./gradlew :pulsar-common:test --tests org.apache.pulsar.common.util.FutureUtilTest -PtestRetryCount=0 --rerun-tasks` - `./gradlew :managed-ledger:test --tests org.apache.bookkeeper.mledger.util.FuturesTest -PtestRetryCount=0 --rerun-tasks` - `git diff --check` - `git diff --cached --check` ### Does this pull request potentially affect one of the following parts: <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. --> *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment -- 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]
