Remove whole table locks on push rule add/delete (#16051)

The statements are already executed within a transaction thus a table
level lock is unnecessary.
This commit is contained in:
Nick Mills-Barrett 2023-11-13 16:57:44 +00:00 committed by GitHub
parent 69afe3f7a0
commit 0e36a57b60
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 16 deletions

1
changelog.d/16051.misc Normal file
View file

@ -0,0 +1 @@
Remove whole table locks on push rule modifications. Contributed by Nick @ Beeper (@fizzadar).

View file

@ -449,26 +449,28 @@ class PushRuleStore(PushRulesWorkerStore):
before: str, before: str,
after: str, after: str,
) -> None: ) -> None:
# Lock the table since otherwise we'll have annoying races between the
# SELECT here and the UPSERT below.
self.database_engine.lock_table(txn, "push_rules")
relative_to_rule = before or after relative_to_rule = before or after
res = self.db_pool.simple_select_one_txn( sql = """
txn, SELECT priority, priority_class FROM push_rules
table="push_rules", WHERE user_name = ? AND rule_id = ?
keyvalues={"user_name": user_id, "rule_id": relative_to_rule}, """
retcols=["priority_class", "priority"],
allow_none=True,
)
if not res: if isinstance(self.database_engine, PostgresEngine):
sql += " FOR UPDATE"
else:
# Annoyingly SQLite doesn't support row level locking, so lock the whole table
self.database_engine.lock_table(txn, "push_rules")
txn.execute(sql, (user_id, relative_to_rule))
row = txn.fetchone()
if row is None:
raise RuleNotFoundException( raise RuleNotFoundException(
"before/after rule not found: %s" % (relative_to_rule,) "before/after rule not found: %s" % (relative_to_rule,)
) )
base_priority_class, base_rule_priority = res base_rule_priority, base_priority_class = row
if base_priority_class != priority_class: if base_priority_class != priority_class:
raise InconsistentRuleException( raise InconsistentRuleException(
@ -516,9 +518,18 @@ class PushRuleStore(PushRulesWorkerStore):
conditions_json: str, conditions_json: str,
actions_json: str, actions_json: str,
) -> None: ) -> None:
# Lock the table since otherwise we'll have annoying races between the if isinstance(self.database_engine, PostgresEngine):
# SELECT here and the UPSERT below. # Postgres doesn't do FOR UPDATE on aggregate functions, so select the rows first
self.database_engine.lock_table(txn, "push_rules") # then re-select the count/max below.
sql = """
SELECT * FROM push_rules
WHERE user_name = ? and priority_class = ?
FOR UPDATE
"""
txn.execute(sql, (user_id, priority_class))
else:
# Annoyingly SQLite doesn't support row level locking, so lock the whole table
self.database_engine.lock_table(txn, "push_rules")
# find the highest priority rule in that class # find the highest priority rule in that class
sql = ( sql = (