Skip to content

Commit

Permalink
Use ordered queries to reduce deadlocks [RHELDST-22078]
Browse files Browse the repository at this point in the history
There are still DB deadlocks occurring from time to time. These all seem
to be related to multiple rhsm-pulp repos updating records for the same
files concurrently (e.g. multiple repos having the same RPMs will all
need to update the same paths under /origin/ around the same time).

While the relevant code is all prepared to retry on deadlock anyway and
so this should not be noticeable beyond a delay, it would be nice to fix
this.

This commit tries to fix it by always applying a consistent order for
all queries obtaining row locks on items. The change is speculative as I
can't reproduce the deadlocks on demand, but it's consistent with the
advice given in postgres docs[1]:

> The best defense against deadlocks is generally to avoid them by being
> certain that all applications using a database acquire locks on
> multiple objects in a consistent order

[1] https://www.postgresql.org/docs/current/explicit-locking.html#LOCKING-DEADLOCKS
  • Loading branch information
rohanpm committed Dec 19, 2023
1 parent dcdc0ed commit 07defcf
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
1 change: 1 addition & 0 deletions exodus_gw/models/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def resolve_links(
func.coalesce(Item.link_to, "") # pylint: disable=E1102
!= ""
)
.order_by(Item.web_uri)
.all()
)
else:
Expand Down
2 changes: 2 additions & 0 deletions exodus_gw/routers/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ def update_publish_items(
[i.web_uri for i in items if not i.link_to]
)
)
.order_by(models.Item.web_uri)
.all()
)

Expand All @@ -290,6 +291,7 @@ def update_publish_items(
}
for item in items
]
items_data.sort(key=lambda item: item["web_uri"])

LOG.debug(
"Adding %s items into '%s'",
Expand Down
6 changes: 4 additions & 2 deletions exodus_gw/worker/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,10 @@ def item_select(self):
# Returns base of the SELECT query to find all items for commit.
#
# Can be overridden in subclasses.
return select(Item).where(
Item.publish_id == self.publish.id, Item.dirty == True
return (
select(Item)
.where(Item.publish_id == self.publish.id, Item.dirty == True)
.order_by(Item.web_uri)
)

@property
Expand Down

0 comments on commit 07defcf

Please sign in to comment.