Skip to content

Commit ba95845

Browse files
mdellwegggainey
andcommitted
Relax touch
Co-authored-by: Grant Gainey <ggainey@users.noreply.github.com>
1 parent f8665e1 commit ba95845

File tree

2 files changed

+24
-11
lines changed

2 files changed

+24
-11
lines changed

CHANGES/+touch.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Improve touch performance in highly concurrent content operations.

pulpcore/app/models/content.py

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,25 @@ class BulkTouchQuerySet(models.QuerySet):
106106
def touch(self):
107107
"""
108108
Update the ``timestamp_of_interest`` on all objects of the query.
109-
110-
Postgres' UPDATE call doesn't support order-by. This can (and does) result in deadlocks in
111-
high-concurrency environments, when using touch() on overlapping data sets. In order to
112-
prevent this, we choose to SELECT FOR UPDATE with SKIP LOCKS == True, and only update
113-
the rows that we were able to get locks on. Since a previously-locked-row implies
114-
that updating that row's timestamp-of-interest is the responsibility of whoever currently
115-
owns it, this results in correct data, while closing the window on deadlocks.
116109
"""
110+
111+
# Postgres' UPDATE call doesn't support order-by. This can (and does) result in deadlocks in
112+
# high-concurrency environments, when using touch() on overlapping data sets. In order to
113+
# prevent this, we choose to SELECT FOR UPDATE with SKIP LOCKS == True, and only update
114+
# the rows that we were able to get locks on. Since a previously-locked-row implies
115+
# that updating that row's timestamp-of-interest is the responsibility of whoever currently
116+
# owns it, this results in correct data, while closing the window on deadlocks.
117+
# Django requires "select_for_update" to be used in a transaction even if this is a
118+
# singular query.
119+
# `no_key` translates to `SELECT ... FOR NO KEY UPDATE` and results in a different type of
120+
# lock being used. We don't change any primary/foreign keys.
117121
with transaction.atomic():
118-
sub_q = self.order_by("pk").select_for_update(skip_locked=True)
119-
return self.filter(pk__in=sub_q).update(timestamp_of_interest=now())
122+
sub_q = (
123+
self.filter(timestamp_of_interest__lt=now() - datetime.timedelta(hours=1))
124+
.order_by("pk")
125+
.select_for_update(skip_locked=True, no_key=True)
126+
)
127+
self.filter(pk__in=sub_q).update(timestamp_of_interest=now())
120128

121129

122130
class QueryMixin:
@@ -390,7 +398,9 @@ def from_pulp_temporary_file(cls, temp_file):
390398

391399
def touch(self):
392400
"""Update timestamp_of_interest."""
393-
self.save(update_fields=["timestamp_of_interest"])
401+
# Touch via the queryset.
402+
# We are not interested in the updated value on the python side anyway.
403+
self.__class__.objects.filter(pk=self.pk).touch()
394404

395405

396406
class PulpTemporaryFile(HandleTempFilesMixin, BaseModel):
@@ -624,7 +634,9 @@ def init_from_artifact_and_relative_path(artifact, relative_path):
624634

625635
def touch(self):
626636
"""Update timestamp_of_interest."""
627-
self.save(update_fields=["timestamp_of_interest"])
637+
# Touch via the queryset.
638+
# We are not interested in the updated value on the python side anyway.
639+
self.__class__.objects.filter(pk=self.pk).touch()
628640

629641

630642
class ContentArtifact(BaseModel, QueryMixin):

0 commit comments

Comments
 (0)