Skip to content

Commit c64f100

Browse files
fix(jobs): stale_timeout=0 means no grace period
refresh() removes a job whose key has left key_source once the job is older than stale_timeout, a grace period for a key_source that is still being filled. stale_timeout=0 skipped that check instead, so there was no way to ask for the cleanup now: refresh(stale_timeout=0) left every such job in place, and so did populate() with config.jobs.stale_timeout = 0. 0 now means no grace period: on that refresh, every job whose key is no longer in key_source is removed, whatever its age, unless it is marked ignore. That includes reserved jobs, and success records of rows that remain after their key left key_source, which a nonzero stale_timeout keeps for that many seconds. stale_timeout=None still means the configured value (config.jobs.stale_timeout, 3600 seconds by default), so a refresh that does not pass it keeps the grace period. The setting's description, "Seconds before pending job is checked for staleness", now describes the grace period and what 0 does. Skipping the check is no longer expressible as 0; a long stale_timeout keeps such jobs for as long as it says. The tests delete upstream rows and check, on MySQL and PostgreSQL, that a default refresh keeps the pending jobs of the deleted keys, that refresh(stale_timeout=0) removes them, and that populate() with config.jobs.stale_timeout = 0 removes them instead of trying to compute them. Each fails without the change. They replace the test that held 0 to its old meaning. Closes #1577.
1 parent 6a1e171 commit c64f100

3 files changed

Lines changed: 42 additions & 17 deletions

File tree

‎src/datajoint/jobs.py‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,10 @@ def refresh(
338338
Priority for new jobs (lower = more urgent).
339339
Default from ``config.jobs.default_priority``.
340340
stale_timeout : float, optional
341-
Seconds after which jobs are checked for staleness.
342-
Jobs older than this are removed if key not in key_source.
343-
Default from ``config.jobs.stale_timeout``. Set to 0 to skip the
344-
age-based check; the cleanup of success and error records whose
345-
keys have left both key_source and the target runs regardless.
341+
Grace period in seconds for jobs whose keys are no longer in
342+
key_source: such jobs are removed once older than this. 0 means no
343+
grace period, and removes them on this refresh. Default None uses
344+
``config.jobs.stale_timeout``.
346345
orphan_timeout : float, optional
347346
Seconds after which reserved jobs are considered orphaned.
348347
Reserved jobs older than this are deleted and re-added as pending.
@@ -363,7 +362,8 @@ def refresh(
363362
3. Remove vanished jobs, judged against the unrestricted key_source:
364363
365364
a. success and error records whose keys are in neither key_source nor target, at any age
366-
b. stale jobs (except ``ignore``): older than stale_timeout, keys not in key_source
365+
b. stale jobs (except ``ignore``): keys not in key_source, older than stale_timeout
366+
(at any age when stale_timeout is 0)
367367
368368
4. Remove orphaned jobs: reserved jobs older than orphan_timeout (if specified)
369369
"""
@@ -433,11 +433,14 @@ def refresh(
433433
.delete_quick(get_count=True)
434434
)
435435

436-
# 3b. Remove stale jobs (not ignore status) - use server CURRENT_TIMESTAMP for consistent timing
436+
# 3b. Remove stale jobs (not ignore status) once older than stale_timeout, a grace period
437+
# for a key_source that is still being filled. stale_timeout=0 means no grace period.
438+
# Age is judged on the server's CURRENT_TIMESTAMP for consistent timing.
439+
stale_jobs = vanished & "status != 'ignore'"
437440
if stale_timeout > 0:
438441
stale_interval = self.adapter.interval_expr(stale_timeout, "second")
439-
stale_jobs = vanished & f"created_time < CURRENT_TIMESTAMP - {stale_interval}" & "status != 'ignore'"
440-
result["removed"] += stale_jobs.delete_quick(get_count=True)
442+
stale_jobs = stale_jobs & f"created_time < CURRENT_TIMESTAMP - {stale_interval}"
443+
result["removed"] += stale_jobs.delete_quick(get_count=True)
441444

442445
# 4. Handle orphaned reserved jobs - use server CURRENT_TIMESTAMP for consistent timing
443446
if orphan_timeout is not None and orphan_timeout > 0:

‎src/datajoint/settings.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,12 @@ class JobsSettings(BaseSettings):
294294

295295
auto_refresh: bool = Field(default=True, description="Auto-refresh jobs queue on populate")
296296
keep_completed: bool = Field(default=False, description="Keep success records in jobs table")
297-
stale_timeout: int = Field(default=3600, ge=0, description="Seconds before pending job is checked for staleness")
297+
stale_timeout: int = Field(
298+
default=3600,
299+
ge=0,
300+
description="Grace period in seconds before refresh() removes a job whose key is no longer in key_source. "
301+
"0 means no grace period: such jobs are removed on the next refresh.",
302+
)
298303
default_priority: int = Field(default=5, ge=0, le=255, description="Default priority for new jobs (lower = more urgent)")
299304
version_method: Literal["git", "none"] | None = Field(
300305
default=None, description="Method to obtain version: 'git' (commit hash), 'none' (empty), or None (disabled)"

‎tests/integration/test_jobs_refresh.py‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
restriction -- reserved by other workers, or recording errors -- are not stale.
77
``success`` and ``error`` records whose key has left both ``key_source`` and the
88
target are removed on refresh, in one statement rather than one per key.
9+
``stale_timeout=0`` means no grace period for other jobs whose key has left
10+
``key_source``: they are removed on that refresh, not kept indefinitely.
911
"""
1012

1113
import time
@@ -166,19 +168,34 @@ def make(self, key):
166168
assert _statuses(Gated.jobs) == {0: "success", 1: "success"}
167169

168170

169-
def test_stale_timeout_zero_still_skips_stale_check(pipeline):
170-
"""``stale_timeout=0`` keeps skipping the age-based check for pending jobs."""
171+
def test_stale_timeout_zero_means_no_grace_period(pipeline):
172+
"""``stale_timeout=0`` removes vanished pending jobs at once; the default keeps them for its grace period."""
173+
Raw, Proc = pipeline
174+
Raw.insert([dict(raw_id=i, v=float(i)) for i in range(3)])
175+
Proc.jobs.refresh()
176+
(Raw & "raw_id in (1, 2)").delete(prompt=False)
177+
178+
result = Proc.jobs.refresh() # stale_timeout=None: the configured grace period applies
179+
assert _statuses(Proc.jobs) == {0: "pending", 1: "pending", 2: "pending"}
180+
assert result["removed"] == 0
181+
182+
result = Proc.jobs.refresh(stale_timeout=0)
183+
assert _statuses(Proc.jobs) == {0: "pending"}
184+
assert result["removed"] == 2
185+
186+
187+
def test_configured_stale_timeout_zero_means_no_grace_period(pipeline):
188+
"""``config.jobs.stale_timeout = 0`` makes the refresh in ``populate()`` remove vanished jobs at once."""
171189
Raw, Proc = pipeline
172190
Raw.insert([dict(raw_id=i, v=float(i)) for i in range(2)])
173191
Proc.jobs.refresh()
192+
assert Proc.jobs.reserve({"raw_id": 0}) # keeps populate() from computing raw_id=0
174193
(Raw & {"raw_id": 1}).delete(prompt=False)
175-
_backdate(Proc.jobs)
176194

177-
Proc.jobs.refresh(stale_timeout=0)
178-
assert _statuses(Proc.jobs) == {0: "pending", 1: "pending"}
195+
with dj.config.override(jobs={"stale_timeout": 0}):
196+
Proc.populate(reserve_jobs=True, suppress_errors=True)
179197

180-
Proc.jobs.refresh()
181-
assert _statuses(Proc.jobs) == {0: "pending"}
198+
assert _statuses(Proc.jobs) == {0: "reserved"}
182199

183200

184201
def test_refresh_cleanup_is_set_based(pipeline, monkeypatch):

0 commit comments

Comments
 (0)