Skip to content

Commit

Permalink
Minor locking tweaks to reduce contention
Browse files Browse the repository at this point in the history
A couple of places where a Publish was locked FOR UPDATE can safely use
shared locks rather than exclusive locks:

- when adding publish items, we want to lock the Publish to ensure
  it remains in PENDING state during our update. But we won't actually
  change the state of the Publish, so we only need a read/shared lock.

- similarly during commit, we want to lock the Publish to ensure it's in
  PENDING state. In phase2 commit, we actually then update the state of
  the Publish, so we need an exclusive lock. In phase1 commit we don't
  modify the Publish and it's safe to use a read/shared lock.

When using the postgresql dialect, "read=True" here maps to "FOR SHARE"
which is documented at [1].

This change is meant to improve performance by eliminating some
scenarios where a request would have had to wait to obtain an exclusive
lock on the publish; mainly during Pulp publish of many repos, where
many update and phase1 commit requests will be happening concurrently
for the same publish.

[1] https://www.postgresql.org/docs/current/explicit-locking.html#LOCKING-ROWS
  • Loading branch information
rohanpm committed Dec 12, 2023
1 parent 53ed4e3 commit ba69e97
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions exodus_gw/routers/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def update_publish_items(

db_publish = (
db.query(models.Publish)
.with_for_update()
.with_for_update(read=True)
.filter(
models.Publish.id == publish_id,
models.Publish.env == env.name,
Expand Down Expand Up @@ -433,7 +433,9 @@ def commit_publish(

db_publish = (
db.query(models.Publish)
.with_for_update()
# Publish should be locked, but if doing a phase1 commit we will only
# be reading from the publish and not writing to it.
.with_for_update(read=(commit_mode_str == models.CommitModes.phase1))
.filter(
models.Publish.id == publish_id,
models.Publish.env == env.name,
Expand Down

0 comments on commit ba69e97

Please sign in to comment.