deadlock in delete_from_account

[codl]

DETAIL:  Process 21681 waits for ShareLock on transaction 8612212; blocked by process 21685.
Process 21685 waits for ShareLock on transaction 8612213; blocked by process 21681.
HINT:  See server log for query details.
CONTEXT:  while locking tuple (6804,73) in relation "posts"
 [SQL: 'WITH latest AS \n(SELECT posts.created_at AS created_at, posts.updated_at AS updated_at, posts.id AS id, posts.author_id AS author_id, posts.favourite AS favourite, posts.has_media AS has_media, posts.direct AS direct, posts.favourites AS favourites, posts.reblogs AS reblogs, posts.is_reblog AS is_reblog \nFROM posts \nWHERE %(param_2)s = posts.author_id ORDER BY posts.created_at DESC \n LIMIT %(param_3)s)\n SELECT posts.created_at AS posts_created_at, posts.updated_at AS posts_updated_at, posts.id AS posts_id, posts.author_id AS posts_author_id, posts.favourite AS posts_favourite, posts.has_media AS posts_has_media, posts.direct AS posts_direct, posts.favourites AS posts_favourites, posts.reblogs AS posts_reblogs, posts.is_reblog AS posts_is_reblog \nFROM posts \nWHERE %(param_1)s = posts.author_id AND posts.created_at + %(created_at_1)s <= now() AND posts.id NOT IN (SELECT latest.id \nFROM latest) ORDER BY random() \n LIMIT %(param_4)s FOR UPDATE'] [parameters: {
 'param_1': 'REDACTED', 'created_at_1': datetime.timedelta(14), 'param_2': 'REDACTED', 'param_3': 500, 'param_4': 100}]

forget/tasks.py

Lines 233 to 237 in eae4d06

	posts = (
	Post.query.with_parent(account, 'posts')
	.filter(Post.created_at + account.policy_keep_younger <= db.func.now())
	.filter(~Post.id.in_(db.select((latest_n_posts.c.id, )))).order_by(
	db.func.random()).limit(100).with_for_update().all())

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[codl]

hello. i think i've figured this out. it looks like postgres does not lock all the rows in a SELECT .. FOR UPDATE at once atomically. since we order rows by random(), this means one transaction might lock post A and then post B, while another locks post B and then post A

one way to fix this, since we only need a random sample of posts and don't necessarily need all 100 posts that were picked, would be to SELECT ... FOR UPDATE SKIP LOCKED which would just skip rows that were already locked, and avoid a deadlock

however i think there is a larger problem here. it seems unwise to lock a hundred rows and then go off making potentially slow http requests, up to 101 of them in mastodon's case

[codl]

btw the reason why we lock those rows FOR UPDATE is because of an other similar deadlock that would happen without that explicit lock. not sure why that one would happen but it is likely that it was because of locking in random order as well

06bf189