Skip to content

Commit d204365

Browse files
author
Nils Bars
committed
Fix exercise build DB session handling to prevent lock contention
- Expunge exercise and related objects from the session before the long-running Docker build to prevent lazy loads from holding the database advisory lock - Use session.merge() instead of session.add() when saving build results for detached objects - Move BUILDING status and commit into build() so the advisory lock is released before spawning the build thread
1 parent 724af88 commit d204365

2 files changed

Lines changed: 33 additions & 13 deletions

File tree

webapp/ref/core/image.py

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,24 @@ def __run_build_by_id(app, exercise_id: int):
460460
f"[BUILD] Exercise loaded: {exercise.short_name}, "
461461
f"template_path={exercise.template_path}"
462462
)
463+
# Expunge the exercise and all related objects so they become
464+
# fully detached Python objects. This prevents any attribute
465+
# access during the long-running Docker build from triggering
466+
# a lazy load, which would open a new transaction and hold
467+
# the database advisory lock for the entire build duration.
468+
#
469+
# We also manually wire up back-references since joinedload
470+
# only populates forward relationships, not reverse ones.
471+
entry_service = exercise.entry_service
472+
services = list(exercise.services)
473+
app.db.session.expunge(exercise)
474+
if entry_service:
475+
app.db.session.expunge(entry_service)
476+
entry_service.exercise = exercise
477+
for svc in services:
478+
app.db.session.expunge(svc)
479+
svc.exercise = exercise
480+
app.db.session.commit()
463481
ExerciseImageManager.__run_build(app, exercise)
464482
_log_build(f"[BUILD] Build thread finished for exercise_id={exercise_id}")
465483
except Exception as e:
@@ -534,7 +552,7 @@ def __run_build(app, exercise: Exercise):
534552
exercise.build_job_status = ExerciseBuildStatus.FINISHED
535553

536554
_log_build("[BUILD] Committing build result to DB...")
537-
app.db.session.add(exercise)
555+
exercise = app.db.session.merge(exercise)
538556
app.db.session.commit()
539557
_log_build("[BUILD] Build result committed to DB")
540558

@@ -556,6 +574,17 @@ def build(self, wait: bool = False) -> None:
556574
# instance issues.
557575
exercise_id = self.exercise.id
558576

577+
# Set BUILDING status after delete_images (which sets NOT_BUILD),
578+
# then commit to release the database advisory lock before starting
579+
# the build thread. The thread needs to acquire this lock to access
580+
# the database, so we must release it first or the thread will block
581+
# until the caller's transaction completes.
582+
from ref import db
583+
584+
self.exercise.build_job_status = ExerciseBuildStatus.BUILDING
585+
self.exercise.build_job_result = None
586+
db.session.commit()
587+
559588
_log_build(f"[BUILD] Starting build thread for exercise_id={exercise_id}")
560589
t = Thread(
561590
target=ExerciseImageManager.__run_build_by_id,
@@ -565,12 +594,6 @@ def build(self, wait: bool = False) -> None:
565594

566595
if wait:
567596
_log_build("[BUILD] Waiting for build thread to complete...")
568-
# Commit the current transaction to release the database advisory lock.
569-
# The build thread needs to acquire this lock to access the database,
570-
# so we must release it before joining or we'll deadlock.
571-
from ref import db
572-
573-
db.session.commit()
574597
t.join()
575598
_log_build("[BUILD] Build thread completed")
576599

webapp/ref/view/exercise.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,11 @@ def exercise_build(exercise_id):
5353
flash.success("Container already build")
5454
return redirect_to_next()
5555
else:
56-
# Start new build
56+
# Start new build. build() handles setting BUILDING status,
57+
# deleting old images, and committing before spawning the thread.
5758
current_app.logger.info(
58-
f"Starting build for exercise {exercise}. Setting state to {ExerciseBuildStatus.BUILDING}"
59+
f"Starting build for exercise {exercise}."
5960
)
60-
exercise.build_job_status = ExerciseBuildStatus.BUILDING
61-
exercise.build_job_result = None
62-
db.session.add(exercise)
63-
db.session.commit()
6461
flash.info("Build started...")
6562
mgr.build()
6663
return redirect_to_next()

0 commit comments

Comments
 (0)