[openstreetmap/openstreetmap-website] Lock changesets during a diff upload (PR #6379)

[Pablo Brasero]

pablobm left a comment (openstreetmap/openstreetmap-website#6379)

I'm not convinced that this change fixes the issue :thinking:

I have been abusing the test (thank you for providing it!) and I can't get it to fail without the change. For example, I expected that adding a sleep after `Changeset.find` (pre-change) would make the test fail, but it doesn't.

In deeper inspection, I see that the changeset **object** passed to `DiffReader` is not the same one whose counts are increased. As in: it represents the same changeset _conceptually_, but it is not the same object in memory.

When `DiffReader#commit` calls `Node#from_xml_node`, the created node gets `node.changeset_id = pt["changeset"].to_i`. ActiveRecord creates a new `Changeset` in memory at this point. This one has the correct counts, and they remain correct because we are already inside a transaction. When we get to `changeset.num_created_nodes += 1` (in `Node#create_with_history`), the increase will be correct.

Admittedly, my understanding of ACID is not great so I might be missing something. Thoughts @tomhughes?

It might still be worth merging this and see if it helps, but I suspect we'll continue seeing the issue.

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

Message ID: <openstreetmap/openstreetmap-website/pull/6379/c3338112709 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20250926/a912f987/attachment-0001.htm>