apache/gravitino

[Improvement] Ensure JobManager.close() always shuts down background executors when jobExecutor.close() fails

Open

#10.272 aperta il 6 mar 2026

Vedi su GitHub
 (4 commenti) (0 reazioni) (1 assegnatario)Java (887 fork)auto 404
good first issueimprovement

Metriche repository

Star
 (3058 star)
Metriche merge PR
 (Metriche PR in attesa)

Descrizione

What would you like to be improved?

JobManager.close() calls jobExecutor.close() before shutting down statusPullExecutor and cleanUpExecutor. If jobExecutor.close() throws IOException, method execution exits early, and both schedulers are left running. This can leak background threads during shutdown, causing incomplete cleanup.

How should we improve?

Make shutdown resilient by guaranteeing executor shutdown in a finally block. Capture any IOException from jobExecutor.close(), run statusPullExecutor.shutdownNow() and cleanUpExecutor.shutdownNow() unconditionally, then rethrow the original exception . This preserves failure signaling while ensuring cleanup always runs.

Here's a test to help:

@Test
public void testCloseShouldShutdownExecutorsWhenJobExecutorCloseFails() throws IOException {
  JobExecutor failingJobExecutor = Mockito.mock(JobExecutor.class);
  doThrow(new IOException("close failed")).when(failingJobExecutor).close();

  JobManager manager = new JobManager(config, entityStore, idGenerator, failingJobExecutor);
  try {
    Assertions.assertThrows(IOException.class, manager::close);
    Assertions.assertTrue(manager.statusPullExecutor.isShutdown());
    Assertions.assertTrue(manager.cleanUpExecutor.isShutdown());
  } finally {
    manager.statusPullExecutor.shutdownNow();
    manager.cleanUpExecutor.shutdownNow();
  }
}

Guida contributor