Fix idempotency when database writes are slow (#21840)
There is an idempotency key generated by clients when authoring a post, and stored in Redis, to ensure that if a user or client retries posting the same status, we don't get a duplicate. Hachyderm.io has been experiencing some filesystem and database performance issues, causing database writes to be slow. This can mean that there are successful posts, but the reverse proxy returns 504 Gateway Timeout before the idempotency status has been updated; users or clients who retry (such as Tusky which retries automatically, see tuskyapp/Tusky#2951) can re-try the same post with the same idempotency key before it has actually been recorded in Redis, leading to duplicate posts. To address this issue, move all of the database updates after the initial transaction that creates the status into the `postprocess_status!` method, so we can insert the idempotency key immediately after the status has been created, significantly reducing the window in which the status could be created but the idempotency key not yet stored. Note: this has not yet been tested; I'm submitting this PR for discussion and to offer to the Hachyderm.io admins to try out to fix the multiple posting problem. Co-authored-by: Brian Campbell <brcampbell@beta.team>
This commit is contained in:
parent
ebf1d74e40
commit
2d12948220
|
@ -37,12 +37,15 @@ class PostStatusService < BaseService
|
||||||
schedule_status!
|
schedule_status!
|
||||||
else
|
else
|
||||||
process_status!
|
process_status!
|
||||||
postprocess_status!
|
|
||||||
bump_potential_friendship!
|
|
||||||
end
|
end
|
||||||
|
|
||||||
redis.setex(idempotency_key, 3_600, @status.id) if idempotency_given?
|
redis.setex(idempotency_key, 3_600, @status.id) if idempotency_given?
|
||||||
|
|
||||||
|
unless scheduled?
|
||||||
|
postprocess_status!
|
||||||
|
bump_potential_friendship!
|
||||||
|
end
|
||||||
|
|
||||||
@status
|
@status
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -66,9 +69,6 @@ class PostStatusService < BaseService
|
||||||
ApplicationRecord.transaction do
|
ApplicationRecord.transaction do
|
||||||
@status = @account.statuses.create!(status_attributes)
|
@status = @account.statuses.create!(status_attributes)
|
||||||
end
|
end
|
||||||
|
|
||||||
process_hashtags_service.call(@status)
|
|
||||||
process_mentions_service.call(@status)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def schedule_status!
|
def schedule_status!
|
||||||
|
@ -92,6 +92,8 @@ class PostStatusService < BaseService
|
||||||
end
|
end
|
||||||
|
|
||||||
def postprocess_status!
|
def postprocess_status!
|
||||||
|
process_hashtags_service.call(@status)
|
||||||
|
process_mentions_service.call(@status)
|
||||||
Trends.tags.register(@status)
|
Trends.tags.register(@status)
|
||||||
LinkCrawlWorker.perform_async(@status.id)
|
LinkCrawlWorker.perform_async(@status.id)
|
||||||
DistributionWorker.perform_async(@status.id)
|
DistributionWorker.perform_async(@status.id)
|
||||||
|
|
Loading…
Reference in New Issue