Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent user pref races #1576

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Prevent user pref races #1576

merged 1 commit into from
Sep 12, 2023

Conversation

dholms
Copy link
Collaborator

@dholms dholms commented Sep 11, 2023

We were getting racing updates to the putPreferences route that would leave the table in an inconsistent state.

This applies a row lock on the user's row in the user_account table to ensure that these updates get linearized. This should be a short-held lock and will block other writes while it is held.


This seemed like the simplest implementation to prevent conflicting writes. Other options include:

  • advisory lock (similar to repo writes)
  • skipping on lock and returning an error instead of waiting & overwriting
  • skipping the lock & retrying
  • doing integrity checks at the end of the transaction to ensure we didn't leave it in a bad state & throwing if we did
  • reworking the data modeling of the user-pref table so that it's a simple atomic UPDATE

Copy link
Collaborator

@devinivy devinivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be rare enough that it's a non-issue 👍 May be worth keeping an eye on, but I think it should be good.

@dholms dholms merged commit 1053c6f into main Sep 12, 2023
10 checks passed
@dholms dholms deleted the user-pref-races branch September 12, 2023 17:37
estrattonbailey added a commit that referenced this pull request Sep 12, 2023
…Actor

* origin:
  Get rate limit ip correctly (#1577)
  Remove temp.upgradeRepoVersion (#1588)
  Fix getRepo `since` (#1579)
  Prevent user pref races (#1576)
  Feature: block lists (#1531)
  Enable appview proxy in dev-env full network (#1580)
  Increase timeline threshold (#1573)
  @atproto/[email protected]
  Fixes and updates to the preferences API (#1575)
  Remove legacy repo sync impl (#1570)
  Move fuzzy matcher to appview (#1566)
  @atproto/[email protected]
  Add personal-details user preference with birth date (#1565)
  enable granular perms for publish action (#1563)
  maintain feed order (#1559)
mloar pushed a commit to mloar/atproto that referenced this pull request Sep 26, 2023
mloar pushed a commit to mloar/atproto that referenced this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants