SEZ9 commented on PR #11597: URL: https://github.com/apache/seatunnel/pull/11597#issuecomment-6514078201
Following up on the one open point from the earlier review: the operation-thread wait in `GetRunningJobSlotUsageOperation.java` (lines 53-67 at `de507b8e2c7814f2d2f5bba6256c1946e9562db9`). To restate the correction: running the aggregation on a named executor does not, by itself, free the Hazelcast operation thread, because the receiving side immediately waits on that executor's future. So a shared operation thread is held for the full cluster-wide aggregation, which is what the executor was meant to avoid. The earlier wording that suggested otherwise was wrong. For the next revision: 1. Let the forwarded operation complete asynchronously instead of waiting on the future inside the operation: hand off the aggregation and send the response from the future's completion callback (both success and failure paths), so the operation thread returns immediately. 2. Keep the aggregation itself read-only and unchanged in behavior; this is only about where the wait happens, not what is computed. 3. If you believe a blocking wait is actually safe in this specific path, please explain why here so we can decide with the reasoning on record. Once that change is pushed I'll re-check this point specifically. Thanks for staying with it. <!-- streview-comment:2143 --> -- 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]
