Copilot commented on code in PR #9532:
URL: https://github.com/apache/seatunnel/pull/9532#discussion_r2182102352
##########
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/CoordinatorService.java:
##########
@@ -230,10 +230,14 @@ private void startPendingJobScheduleThread() {
while (true) {
try {
pendingJobSchedule();
- } catch (InterruptedException e) {
- throw new RuntimeException(e);
- } finally {
- pendingJob.release();
+ } catch (Throwable e) {
+ logger.severe("Error in pending job schedule
thread", e);
+ try {
+ Thread.sleep(3000L);
+ } catch (InterruptedException ex) {
+ logger.severe("Pending job schedule thread
interrupted", ex);
+ Thread.currentThread().interrupt();
Review Comment:
Catching `Throwable` will also capture `Error` types (e.g.,
`OutOfMemoryError`) that are usually unrecoverable. It's better to catch
`Exception` or handle `InterruptedException` separately to allow proper
shutdown and error propagation.
```suggestion
} catch (InterruptedException ex) {
logger.severe("Pending job schedule thread
interrupted", ex);
Thread.currentThread().interrupt();
break; // Exit the loop on interruption
} catch (Exception e) {
logger.severe("Error in pending job schedule
thread", e);
try {
Thread.sleep(3000L);
} catch (InterruptedException ex) {
logger.severe("Pending job schedule thread
interrupted during sleep", ex);
Thread.currentThread().interrupt();
break; // Exit the loop on interruption
```
##########
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/CoordinatorService.java:
##########
@@ -230,10 +230,14 @@ private void startPendingJobScheduleThread() {
while (true) {
try {
pendingJobSchedule();
- } catch (InterruptedException e) {
- throw new RuntimeException(e);
- } finally {
- pendingJob.release();
+ } catch (Throwable e) {
Review Comment:
The removal of `pendingJob.release()` means the queue may never be signaled
after processing or error, potentially causing consumers to block indefinitely.
Consider invoking `pendingJob.release()` in a finally block or after handling
the exception to ensure the queue continues to be unblocked.
--
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]
