Skip to content

SE-3022: Uniquely constrain watched pull requests.

Boros Gábor requested to merge prevent_duplicate_watchers into master

Created by: Kelketek

https://tasks.opencraft.com/browse/SE-3022

The current implementation of get_or_create_from_pr is susceptible to race conditions because of two factors:

  1. The action is not atomic, first checking if the pull requests exists and then creating it if it does not.
  2. There are no unique constraints on the fields involved.

This PR adds in unique constraints into the WatchedPullRequest model. Once made unique, the atomic guards in upstream's 'get_or_create' method can assure that a WatchedPullRequest will never be created more than once.

Notes:

  • This PR requires that the existing redundant WatchedPullRequest instances are removed, or else the migration to apply the unique constraint will fail.
    • I talked with @lgp171188 about the current state of data on production and we worked out a plan of what would need to be done. He has agreed to clear the relevant data from production on July 29, 2020.
  • Since this is a PR fixing a race condition that's difficult to reproduce within a single process, and because the functions have retained their interfaces, practical testing options have been limited. Since there is only one specific behavioral change that it introduces (requiring a unique constraint), I've written a test for that, though I'm not sure how important it is, since unique constraints are well tested upstream. If someone has a better idea for testing this, please let me know.

Merge request reports