Skip to content

Commit 7cf53f6

Browse files
Michael Bunsenclaude
authored andcommitted
feat(jobs): add JobLog child table for append-only per-job logs
Replaces the jobs_job.logs JSON-field UPDATE path in JobLogHandler.emit with an INSERT on a new JobLog child table. Under concurrent async_api load every log line triggered UPDATE jobs_job SET logs = ... on the shared job row; inside ATOMIC_REQUESTS a single batched /result POST stacked N such UPDATEs in one tx, blocking ML workers on the same row (issue #1256). - JobLog model + migration 0021_joblog (FK job, level, message, created_at) - JobLogHandler.emit -> JobLog.objects.create (gated by JOB_LOG_PERSIST_ENABLED flag introduced in PR #1261) - JobSerializer.logs -> SerializerMethodField that reads JobLog rows on detail endpoint and falls back to the legacy jobs_job.logs JSON for jobs created before this table existed. List endpoint keeps the cheap legacy shape to avoid N+1. - No UI changes, no new endpoint. On-wire {stdout, stderr} shape preserved. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ef78fb5 commit 7cf53f6

3 files changed

Lines changed: 119 additions & 32 deletions

File tree

ami/jobs/migrations/0021_joblog.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
from django.db import migrations, models
2+
import django.db.models.deletion
3+
4+
5+
class Migration(migrations.Migration):
6+
dependencies = [
7+
("jobs", "0020_schedule_job_monitoring_beat_tasks"),
8+
]
9+
10+
operations = [
11+
migrations.CreateModel(
12+
name="JobLog",
13+
fields=[
14+
("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")),
15+
("created_at", models.DateTimeField(auto_now_add=True)),
16+
("updated_at", models.DateTimeField(auto_now=True)),
17+
("level", models.CharField(max_length=20)),
18+
("message", models.TextField()),
19+
("context", models.JSONField(blank=True, default=dict)),
20+
(
21+
"job",
22+
models.ForeignKey(
23+
on_delete=django.db.models.deletion.CASCADE, related_name="log_entries", to="jobs.job"
24+
),
25+
),
26+
],
27+
options={
28+
"ordering": ["-created_at", "-pk"],
29+
"indexes": [models.Index(fields=["job", "-created_at"], name="jobs_joblog_job_id_e4aa59_idx")],
30+
},
31+
),
32+
]

ami/jobs/models.py

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,31 @@ class JobLogs(pydantic.BaseModel):
322322
stderr: list[str] = pydantic.Field(default_factory=list, alias="stderr", title="Error messages")
323323

324324

325+
class JobLog(BaseModel):
326+
"""Append-only per-job log row.
327+
328+
Replaces the ``jobs_job.logs`` JSON-field UPDATE path that caused row-lock
329+
contention under concurrent async_api load (issue #1256). Each log emit
330+
becomes a cheap INSERT on this child table instead of a refresh+UPDATE of
331+
the shared parent row. Legacy JSON-field logs are still served by the
332+
serializer for jobs created before this table existed.
333+
"""
334+
335+
project_accessor = "job__project"
336+
337+
job = models.ForeignKey("Job", on_delete=models.CASCADE, related_name="log_entries")
338+
level = models.CharField(max_length=20)
339+
message = models.TextField()
340+
# Freeform bag for future per-line metadata (stage, worker id, counters, ...)
341+
# without requiring a schema migration. Kept nullable/empty-default so it
342+
# costs nothing on existing rows.
343+
context = models.JSONField(blank=True, default=dict)
344+
345+
class Meta:
346+
ordering = ["-created_at", "-pk"]
347+
indexes = [models.Index(fields=["job", "-created_at"])]
348+
349+
325350
class JobLogHandler(logging.Handler):
326351
"""
327352
Class for handling logs from a job and writing them to the job instance.
@@ -337,41 +362,24 @@ def emit(self, record: logging.LogRecord):
337362
# Log to the current app logger (container stdout).
338363
logger.log(record.levelno, self.format(record))
339364

340-
# Gated by ``JOB_LOG_PERSIST_ENABLED`` (default True). Persisting every
341-
# log line to ``jobs_job.logs`` becomes a row-lock contention point
342-
# under concurrent async_api load — each call triggers
343-
# ``UPDATE jobs_job SET logs = ...`` on the shared job row, and inside
344-
# ``ATOMIC_REQUESTS`` a single batched ``/result`` POST stacks N such
345-
# UPDATEs in one tx, blocking every ML worker on the same row for the
346-
# duration of the request. Deployments hitting that pattern can set the
347-
# flag to False to short-circuit here until PR #1259 lands an
348-
# append-only ``JobLog`` child table. See issue #1256.
365+
# Escape hatch: when False, skip the per-job DB write entirely. Container
366+
# stdout still captures every line above, so ops observability is
367+
# unchanged; only the per-job UI log view loses new entries for the
368+
# duration the flag is off. Default is True. See issue #1256.
349369
if not getattr(settings, "JOB_LOG_PERSIST_ENABLED", True):
350370
return
351371

352-
# Write to the logs field on the job instance.
353-
# Refresh from DB first to reduce the window for concurrent overwrites — each
354-
# worker holds its own stale in-memory copy of `logs`, so without a refresh the
355-
# last writer always wins and earlier entries are silently dropped.
356-
# @TODO consider saving logs to the database periodically rather than on every log
372+
# Append-only insert on the JobLog child table. Unlike the legacy
373+
# jobs_job.logs JSONB update path, this does not contend with
374+
# _update_job_progress on the parent row.
357375
try:
358-
self.job.refresh_from_db(fields=["logs"])
359-
timestamp = datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S")
360-
msg = f"[{timestamp}] {record.levelname} {self.format(record)}"
361-
if msg not in self.job.logs.stdout:
362-
self.job.logs.stdout.insert(0, msg)
363-
364-
# Write a simpler copy of any errors to the errors field
365-
if record.levelno >= logging.ERROR:
366-
if record.message not in self.job.logs.stderr:
367-
self.job.logs.stderr.insert(0, record.message)
368-
369-
if len(self.job.logs.stdout) > self.max_log_length:
370-
self.job.logs.stdout = self.job.logs.stdout[: self.max_log_length]
371-
372-
self.job.save(update_fields=["logs"], update_progress=False)
376+
JobLog.objects.create(
377+
job_id=self.job.pk,
378+
level=record.levelname,
379+
message=self.format(record),
380+
)
373381
except Exception as e:
374-
logger.error(f"Failed to save logs for job #{self.job.pk}: {e}")
382+
logger.error(f"Failed to save log for job #{self.job.pk}: {e}")
375383

376384

377385
@dataclass

ami/jobs/serializers.py

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,46 @@
1313
from ami.ml.schemas import PipelineProcessingTask, PipelineTaskResult, ProcessingServiceClientInfo
1414
from ami.ml.serializers import PipelineNestedSerializer
1515

16-
from .models import Job, JobLogs, JobProgress, MLJob
16+
from .models import Job, JobLog, JobProgress, MLJob
1717
from .schemas import QueuedTaskAcknowledgment
1818

19+
JOB_LOG_LEVELS_STDERR = {"ERROR", "CRITICAL"}
20+
JOB_LOG_TIMESTAMP_FORMAT = "%Y-%m-%d %H:%M:%S"
21+
JOB_LOGS_DEFAULT_LIMIT = 1000
22+
23+
24+
def _legacy_logs_shape(job: Job) -> dict[str, list[str]]:
25+
legacy = getattr(job, "logs", None)
26+
return {
27+
"stdout": list(getattr(legacy, "stdout", []) or []),
28+
"stderr": list(getattr(legacy, "stderr", []) or []),
29+
}
30+
31+
32+
def serialize_job_logs(job: Job, *, limit: int = JOB_LOGS_DEFAULT_LIMIT) -> dict[str, list[str]]:
33+
"""Return ``{stdout, stderr}`` in the shape the UI already parses.
34+
35+
Reads joined ``JobLog`` rows first (newest-first, capped at ``limit``). Jobs
36+
created before the table existed and jobs written while
37+
``JOB_LOG_PERSIST_ENABLED=False`` have no rows and fall back to the legacy
38+
``jobs_job.logs`` JSON column so their UI log panel stays populated.
39+
"""
40+
entries = list(
41+
JobLog.objects.filter(job_id=job.pk)
42+
.only("created_at", "level", "message")
43+
.order_by("-created_at", "-pk")[:limit]
44+
)
45+
if entries:
46+
return {
47+
"stdout": [
48+
f"[{entry.created_at.strftime(JOB_LOG_TIMESTAMP_FORMAT)}] {entry.level} {entry.message}"
49+
for entry in entries
50+
],
51+
"stderr": [entry.message for entry in entries if entry.level in JOB_LOG_LEVELS_STDERR],
52+
}
53+
54+
return _legacy_logs_shape(job)
55+
1956

2057
class JobProjectNestedSerializer(DefaultSerializer):
2158
class Meta:
@@ -49,7 +86,7 @@ class JobListSerializer(DefaultSerializer):
4986
source_image_single = SourceImageNestedSerializer(read_only=True)
5087
data_export = DataExportNestedSerializer(read_only=True)
5188
progress = SchemaField(schema=JobProgress, read_only=True)
52-
logs = SchemaField(schema=JobLogs, read_only=True)
89+
logs = serializers.SerializerMethodField()
5390
job_type = JobTypeSerializer(read_only=True)
5491
# All jobs created from the Jobs UI are ML jobs (datasync, etc. are created for the user)
5592
# @TODO Remove this when the UI is updated pass a job type. This should be a required field.
@@ -147,6 +184,16 @@ class Meta:
147184
"dispatch_mode",
148185
]
149186

187+
def get_logs(self, obj: Job) -> dict[str, list[str]]:
188+
# List responses skip the JobLog query to avoid N+1 — the UI only renders
189+
# logs on the detail page, so returning the (typically empty for new jobs)
190+
# legacy JSON shape is acceptable. Detail responses go to the joined table
191+
# and fall back to the legacy shape for pre-migration jobs.
192+
view = self.context.get("view")
193+
if getattr(view, "action", None) == "list":
194+
return _legacy_logs_shape(obj)
195+
return serialize_job_logs(obj)
196+
150197

151198
class JobSerializer(JobListSerializer):
152199
# progress = serializers.JSONField(initial=Job.default_progress(), allow_null=False, required=False)

0 commit comments

Comments
 (0)