This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 6d6c3034f6 [#10728] revert(core): revert #10627 to fix TestJobManager
CI failure (#10729)
6d6c3034f6 is described below
commit 6d6c3034f6a3c3f95105de404cbdb0741168c248
Author: Jerry Shao <[email protected]>
AuthorDate: Thu Apr 9 19:22:57 2026 +0800
[#10728] revert(core): revert #10627 to fix TestJobManager CI failure
(#10729)
### What changes were proposed in this pull request?
Reverts #10627 ([#10623] fix(core): reverse cleanup ordering in
JobManager.cleanUpStagingDirs).
### Why are the changes needed?
PR #10627 introduced a regression that causes
`TestJobManager#testCleanUpStagingDirsDeletesDirectoryBeforeEntity` to
fail in CI. The test expects `entityStore.delete()` to never be called
at a specific point, but after reversing the cleanup ordering, the
entity delete is being called repeatedly, causing a
`ConditionTimeoutException`.
Fix: #10728
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Reverts to the previous behavior. The existing unit test
`TestJobManager#testCleanUpStagingDirsDeletesDirectoryBeforeEntity`
should pass once this revert is applied.
---
.../java/org/apache/gravitino/job/JobManager.java | 8 ++--
.../org/apache/gravitino/job/TestJobManager.java | 48 ----------------------
2 files changed, 4 insertions(+), 52 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/job/JobManager.java
b/core/src/main/java/org/apache/gravitino/job/JobManager.java
index 7d79ffa416..95b90e89dc 100644
--- a/core/src/main/java/org/apache/gravitino/job/JobManager.java
+++ b/core/src/main/java/org/apache/gravitino/job/JobManager.java
@@ -648,17 +648,17 @@ public class JobManager implements JobOperationDispatcher
{
finishedJobs.forEach(
job -> {
try {
+ entityStore.delete(
+ NameIdentifierUtil.ofJob(metalake, job.name()),
Entity.EntityType.JOB);
+
String jobStagingPath =
stagingDir.getAbsolutePath()
+ String.format(JOB_STAGING_DIR, metalake,
job.jobTemplateName(), job.id());
File jobStagingDir = new File(jobStagingPath);
if (jobStagingDir.exists()) {
FileUtils.deleteDirectory(jobStagingDir);
+ LOG.info("Deleted job staging directory {} for job {}",
jobStagingPath, job.name());
}
-
- entityStore.delete(
- NameIdentifierUtil.ofJob(metalake, job.name()),
Entity.EntityType.JOB);
- LOG.info("Deleted job staging directory {} for job {}",
jobStagingPath, job.name());
} catch (IOException e) {
LOG.error("Failed to delete job and staging directory for job
{}", job.name(), e);
}
diff --git a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
index 172925ab0e..2dc4af741f 100644
--- a/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
+++ b/core/src/test/java/org/apache/gravitino/job/TestJobManager.java
@@ -633,54 +633,6 @@ public class TestJobManager {
});
}
- @Test
- public void testCleanUpStagingDirsDeletesDirectoryBeforeEntity() throws
IOException {
- JobEntity finishedJob = newJobEntity("shell_job",
JobHandle.Status.SUCCEEDED);
- BaseMetalake mockMetalakeEntity =
- BaseMetalake.builder()
- .withName(metalake)
- .withId(idGenerator.nextId())
- .withVersion(SchemaVersion.V_0_1)
- .withAuditInfo(AuditInfo.EMPTY)
- .build();
- when(entityStore.list(Namespace.empty(), BaseMetalake.class,
Entity.EntityType.METALAKE))
- .thenReturn(ImmutableList.of(mockMetalakeEntity));
- mockedMetalake
- .when(() -> MetalakeManager.listInUseMetalakes(entityStore))
- .thenReturn(ImmutableList.of(metalake));
- when(jobManager.listJobs(metalake,
Optional.empty())).thenReturn(ImmutableList.of(finishedJob));
-
- // Create the staging directory so FileUtils.deleteDirectory is actually
invoked
- String jobStagingPath =
- testStagingDir
- + File.separator
- + metalake
- + File.separator
- + finishedJob.jobTemplateName()
- + File.separator
- + JobHandle.JOB_ID_PREFIX
- + finishedJob.id();
- File jobStagingDir = new File(jobStagingPath);
- Assertions.assertTrue(jobStagingDir.mkdirs());
-
- // Simulate IOException when deleting the staging directory
- try (MockedStatic<org.apache.commons.io.FileUtils> mockedFileUtils =
- mockStatic(org.apache.commons.io.FileUtils.class)) {
- mockedFileUtils
- .when(() ->
org.apache.commons.io.FileUtils.deleteDirectory(any(File.class)))
- .thenThrow(new IOException("Simulated IO failure"));
-
- Awaitility.await()
- .atMost(3, TimeUnit.SECONDS)
- .untilAsserted(
- () -> {
- jobManager.cleanUpStagingDirs();
- // Entity should NOT be deleted because directory deletion
failed
- verify(entityStore, never()).delete(any(), any());
- });
- }
- }
-
@Test
public void testUpdateShellJobTemplateEntity() {
String jobTemplateName = "old_shell_job";
