LuciferYang opened a new pull request, #9511: URL: https://github.com/apache/paimon/pull/9511
### Purpose close #9508 `SemaphoredDelegatingExecutor` bounds concurrency on a delegate pool with one semaphore permit per submitted task, released by the per-task wrapper when the task finishes. Two paths broke that accounting. `execute()` caught `InterruptedException` from the permit wait, restored the interrupt flag, and then submitted the task anyway. The wrapper released a permit that was never acquired, so `getAvailablePermits()` climbed above `permitCount` and the ceiling this class exists to enforce was raised with nothing logged. It now throws `RejectedExecutionException`, the `Executor` contract's signal that the task will not run. Dropping the task silently is the smaller change but it moves the failure somewhere worse: `execute()` has no failed-future channel, so the callers that reach this class through `CompletableFuture.supplyAsync(task, executor)` would wait on a stage that never completes. All four submit/execute paths now return the acquired permit when the delegate rejects the task. Previously the permit leaked, because the wrapper that would have released it never ran. The release goes through a release-once guard on the wrapper rather than an unconditional `release()` at the call site: a delegate that runs the task in the calling thread (`CallerRunsPolicy` on a saturated bounded pool, or any direct executor) can both run the wrapper and let a `RejectedExecutionException` thrown by the task itself out of the same call, and releasing twice there inflates the count the same way the interrupt path did. On scope: every current construction site (`FileOperationThreadPool`, `ManifestReadThreadPool`, `GlobalIndexReadThreadPool`, `CatalogSplitEnumerator`) wraps a process-wide static pool with an unbounded queue and the default `AbortPolicy`, so the interrupt path is the one reachable in production today. The catch stays narrowed to `RejectedExecutionException`, which is the contract signal for "the delegate will not take this task"; other throwables out of the delegate are left alone, and the release-once guard makes widening that catch safe later if a bounded or custom delegate ever needs it. ### Tests `SemaphoredDelegatingExecutorTest` is new. The class had no test before. - `testInterruptedExecuteRejectsTaskAndKeepsPermitCount`: waits until the submitter is provably parked on a zero-permit semaphore, interrupts it once, and asserts the `RejectedExecutionException` carries the `InterruptedException` as its cause and that the interrupt flag was restored. It then drains the delegate before asserting the task never ran and the permit count is unchanged, so a regression that hands the task to the delegate cannot pass by being sampled too early. - `testRejectedByDelegateReleasesPermit`: a shut-down delegate, asserting the permit returns to 1 after each of the four entry points (`execute`, `submit(Callable)`, `submit(Runnable)`, `submit(Runnable, result)`). - `testInlineExecutionReleasesPermitOnlyOnce`: a `ThreadPoolExecutor` with `corePoolSize` 1, a queue of 1 and `CallerRunsPolicy`, saturated so the wrapper runs in the calling thread while the task's own `RejectedExecutionException` escapes `execute()`. Asserts the permit count is 1, not 2. - `testNormalExecutionKeepsPermitsBalanced`: five tasks over two permits, asserting all five ran and the count is back to two once the delegate terminates. This one passes without the fix as well. It is the first pin on the normal release path, which had none. Verified red before the change: the first two fail against the pre-fix code (`expected: 1` but `0`, and no `RejectedExecutionException` at all), and `testInlineExecutionReleasesPermitOnlyOnce` fails against a version that releases unconditionally in the catch (`expected: 1` but `2`). `mvn -pl paimon-common test` on JDK 8: 12469 tests, 0 failures, 0 errors. checkstyle, spotless, enforcer and rat run clean. -- 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]
