apache/gravitino

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

Open

#10 272 ouverte le 6 mars 2026

Voir sur GitHub
 (4 commentaires) (0 réactions) (1 assigné)Java (887 forks)auto 404
good first issueimprovement

Métriques du dépôt

Stars
 (3 058 stars)
Métriques de merge PR
 (Métriques PR en attente)

Description

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();
  }
}

Guide contributeur