Skip to content

Commit ca83c7e

Browse files
author
bghira
committed
Prevent training service tests from leaking running jobs
1 parent f04b4b1 commit ca83c7e

3 files changed

Lines changed: 42 additions & 7 deletions

File tree

simpletuner/simpletuner_sdk/server/services/local_gpu_allocator.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def __init__(self):
5353
self._job_repo = None
5454
self._reconciled = False
5555

56-
async def reconcile_on_startup(self, max_entries: int = 20) -> Dict[str, int]:
56+
async def reconcile_on_startup(self, max_entries: Optional[int] = None) -> Dict[str, int]:
5757
"""Reconcile LOCAL jobs with actual process status.
5858
5959
Called on startup to check running LOCAL jobs and handle orphaned ones:
@@ -62,7 +62,8 @@ async def reconcile_on_startup(self, max_entries: int = 20) -> Dict[str, int]:
6262
- Jobs without PIDs (legacy) are marked as failed
6363
6464
Args:
65-
max_entries: Maximum number of running entries to check (default 20).
65+
max_entries: Optional maximum number of running entries to check.
66+
When omitted, all running local jobs are reconciled.
6667
6768
Returns:
6869
Dictionary with stats: {"orphaned": N, "adopted": N, "no_pid": N}
@@ -73,17 +74,14 @@ async def reconcile_on_startup(self, max_entries: int = 20) -> Dict[str, int]:
7374
return stats
7475

7576
job_repo = self._get_job_repo()
76-
# Only get LOCAL running jobs - this is bounded by the number of
77-
# GPUs on this machine, so should be small (typically 0-8)
7877
running_jobs = await job_repo.get_running_local_jobs()
7978

8079
if not running_jobs:
8180
self._reconciled = True
8281
return stats
8382

84-
# Limit entries to check
85-
jobs_to_check = running_jobs[:max_entries]
86-
if len(running_jobs) > max_entries:
83+
jobs_to_check = running_jobs if max_entries is None else running_jobs[:max_entries]
84+
if max_entries is not None and len(running_jobs) > max_entries:
8785
logger.warning(
8886
"Found %d running local jobs, only checking first %d",
8987
len(running_jobs),

tests/test_local_gpu_allocator.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,27 @@ def test_reconcile_marks_preboot_job_failed(self):
204204
self.assertEqual(repo.failed, [("job-preboot", "Process ended after system reboot")])
205205
self.assertEqual(repo.released, ["job-preboot"])
206206

207+
def test_reconcile_processes_all_running_jobs_by_default(self):
208+
allocator = LocalGPUAllocator()
209+
jobs = [
210+
_StubJob(
211+
f"job-{index}",
212+
started_at="2026-01-27T00:00:00+00:00",
213+
)
214+
for index in range(35)
215+
]
216+
repo = _StubJobRepo(jobs)
217+
allocator._job_repo = repo
218+
219+
with patch.object(allocator, "_get_boot_time_utc", return_value=None):
220+
stats = asyncio.run(allocator.reconcile_on_startup())
221+
222+
self.assertEqual(stats["orphaned"], 0)
223+
self.assertEqual(stats["adopted"], 0)
224+
self.assertEqual(stats["no_pid"], 35)
225+
self.assertEqual(len(repo.failed), 35)
226+
self.assertEqual(len(repo.released), 35)
227+
207228

208229
class TestLocalGPUAllocatorAvailability(unittest.TestCase):
209230
"""Tests for GPU availability checking."""

tests/test_training_service.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ def load_config(self, name: str):
6565
return dict(self._config), {}
6666

6767

68+
class DummyJobRepository:
69+
def __init__(self):
70+
self.jobs = []
71+
72+
async def add(self, job):
73+
self.jobs.append(job)
74+
return job
75+
76+
6877
def _mock_is_truthy(value: Any) -> bool:
6978
if value is None:
7079
return False
@@ -79,8 +88,14 @@ def _mock_is_truthy(value: Any) -> bool:
7988
class TrainingServiceTests(unittest.TestCase):
8089
def setUp(self) -> None:
8190
self._saved_state = copy.deepcopy(training_service.APIState.state)
91+
self.job_repo = DummyJobRepository()
8292
self._save_state_patch = patch.object(training_service.APIState, "save_state", return_value=None)
8393
self._save_state_patch.start()
94+
self._job_repo_patch = patch(
95+
"simpletuner.simpletuner_sdk.server.services.cloud.storage.job_repository.get_job_repository",
96+
return_value=self.job_repo,
97+
)
98+
self._job_repo_patch.start()
8499
# Mock cache/scan service checks so stale singleton state doesn't block training
85100
mock_cache_svc = MagicMock()
86101
mock_cache_svc.get_active_status.return_value = None
@@ -99,6 +114,7 @@ def setUp(self) -> None:
99114

100115
def tearDown(self) -> None:
101116
training_service.APIState.state = self._saved_state
117+
self._job_repo_patch.stop()
102118
self._save_state_patch.stop()
103119
self._cache_svc_patch.stop()
104120
self._scan_svc_patch.stop()

0 commit comments

Comments
 (0)