[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