[openstreetmap/openstreetmap-website] Lock note during status update to avoid race condition (PR #5053)

mmd notifications at github.com
Tue Aug 6 18:42:41 UTC 2024


As discussed in #4528, I'm proposing to add some note id locking as described here: https://api.rubyonrails.org/v7.1.3.4/classes/ActiveRecord/Locking/Pessimistic.html (first two examples).

The change covers destroy, comment, close and reopen methods, since all of them  are evaluating a note's visible and/or closed flag.

The important change is the "FOR UPDATE" addition when reading a note id initially. This way, two parallel processes cannot read the same note id at the same time and then both decide that e.g. reopening is permitted.

```
  TRANSACTION (0.1ms)  BEGIN
  ↳ app/controllers/api/notes_controller.rb:208:in `block in reopen'
  Note Load (14.7ms)  SELECT "notes".* FROM "notes" WHERE "notes"."id" = $1 LIMIT $2 FOR UPDATE  [["id", 3], ["LIMIT", 1]]
  [...]
  Note Update (0.3ms)  UPDATE "notes" SET "updated_at" = $1, "status" = $2, "closed_at" = $3 WHERE "notes"."id" = $4  [["updated_at", "2024-08-06 17:46:42.692378"], ["status", "open"], ["closed_at", nil], ["id", 3]]
  ↳ app/models/note.rb:58:in `reopen'
   [...]
  TRANSACTION (1.6ms)  COMMIT
```

Fixes #4528
You can view, comment on, or merge this pull request online at:

  https://github.com/openstreetmap/openstreetmap-website/pull/5053

-- Commit Summary --

  * Lock note during status update to avoid race condition

-- File Changes --

    M app/controllers/api/notes_controller.rb (46)

-- Patch Links --

https://github.com/openstreetmap/openstreetmap-website/pull/5053.patch
https://github.com/openstreetmap/openstreetmap-website/pull/5053.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5053
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/5053 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240806/4a5c837a/attachment.htm>


More information about the rails-dev mailing list