[openstreetmap/openstreetmap-website] Backfill block deactivates_at (PR #5106)

Andy Allan notifications at github.com
Fri Aug 23 09:59:54 UTC 2024


Adding `sleep` feels a bit cargo-culty to me too. The original [documentation in ActiveRecord](https://github.com/rails/rails/commit/d13623ca46c82120c398f4634e206422fc3ad7ea0) has been there since 2009, and the world of databases has changed a lot since then (e.g. PCIe, SSD), and even the example hinted more at a non-realistic scenario (the example uses a 50s wait between batches, in the context of people entering a club).

I'm also not bothered by "CPU spikes". If the db has available CPU capacity then lets use it. So the only situation would be if the queries from the migration completes saturates the CPU (or disk IO, or networking, or similar). In this case it certainly won't, but perhaps it would happen if we e.g. did `Node.in_batches`. But in that case, I think `0.01` is a number plucked from thin air, and if the database can't handle the normal queries plus the updates, then I think backpressure from the db (i.e. these queries taking longer to complete) will regulate the migration better than arbitrary sleeps. The whole point of using batches is to give the db repeated opportunities to run other queries, it doesn't need a gap to choose the next in its queue.

So my recommendation is to remove the sleep, so that the next time a contributor is looking to do something similar, they are less likely to include it in their migration too.

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

Message ID: <openstreetmap/openstreetmap-website/pull/5106/c2306741639 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240823/ae70714b/attachment-0001.htm>


More information about the rails-dev mailing list