DanielLeens commented on PR #11597: URL: https://github.com/apache/seatunnel/pull/11597#issuecomment-6538153661
@SEZ9 thanks for restating the point; the asynchronous revision is pushed as `38f1ca01d7bace05a0dec8627c5143f6002ddca6`. **What changed** - `seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/operation/GetRunningJobSlotUsageOperation.java` (`lines 39-69 at 38f1ca01d7b`): the operation now extends `AsyncOperation` instead of `Operation`. `doRun()` submits `RunningJobSlotUsageBuilder.build(service)` to the named executor via `CompletableFuture.supplyAsync(...)` and returns a `PassiveCompletableFuture<String>` immediately. The old `awaitResponse(...)` / `future.get()` on the operation thread and the `response` field are gone. - `seatunnel-engine/seatunnel-engine-server/src/test/java/org/apache/seatunnel/engine/server/operation/GetRunningJobSlotUsageOperationTest.java`: the two completed-future tests are replaced by lifecycle tests that drive `run()` with a deferred executor (the task is captured, never executed on its own). `shouldReleaseOperationThreadBeforeAggregationCompletes` asserts under `assertTimeoutPreemptively` that `run()` returns while the task is still pending and no response has been sent, then runs the task and expects exactly one `"[]"` response. `shouldSendBuilderFailureAsExceptionalResponse` throws `SeaTunnelEngineRetryableException` from the builder seam and asserts the same instance (not the `CompletionException` wrapper) is the single response. With the previous blocking implementation the first test cannot return and fails on the timeout. **Why the operation thread is now released** `AsyncOperation.run()` is final: it calls `doRun()`, and since `returnsResponse()` is `false` the Hazelcast `OperationRunner` does not wait for or read a response from the operation thread; `run()` returns as soon as the future is handed back. The response is delivered from the future's completion callback (`future.whenComplete(withTryCatch(...doSendResponse(f != null ? peel(f) : r)))`): on success `sendResponse(json)`, on failure `peel(...)` unwraps the `CompletionException` from `supplyAsync` and `sendResponse(originalException)`, with the existing `HazelcastSerializationException` fallback. `beforeRun()` registers the operation in `LiveOperationRegistry` and `doSendResponse` deregisters it, so the invocation stays alive while the executor works. The caller path (`RunningJobSlotUsageService` -> `NodeEngineUtil.sendOperationToMasterNode(...).join()`) is unchanged, as are the String JSON response, `GET_RUNNING_JOB_SLOT_USAGE_OPERATION = 18`, the factory id and the absence of paylo ad fields, so the wire contract does not move. Aggregation itself is untouched (read-only, same `RunningJobSlotUsageBuilder`). **Merge note** Merged `origin/dev` at `3ac2d1cec6aa7b3741c4dacdd23020f778e23c02` (merge commit, no conflicts); `git diff origin/dev --name-status` still shows exactly the PR's 15 files with no deletions. **Validation** Only `./mvnw spotless:apply` / `spotless:check -pl seatunnel-engine/seatunnel-engine-server` were run locally; compile and tests are left to CI per the validation policy. Fork Build for this head: https://github.com/DanielLeens/seatunnel/actions/runs/41162604738 (queued at posting time). -- 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]
