FIX: Use CDN URL for internal onebox avatars (PR #15077)

This commit will also trigger a background rebake for all existing posts with internal oneboxes

GitHub

Nice catch!

For sites with alot of posts, wouldn’t this migration lock up the table? I think we should run this migration in batches instead.

Alternatively, we can deploy this as it is and see if it causes any problem before running the update query in batches. I think if we don’t run into problems deploying our own sites, other users will not face a similar problem.

Doing this in batches … incorrectly… has huge potential to pour more oil on this fire.

If we opt to do this in batches:

  1. We disable ddl transactions in this migration
  2. We SELECT a bunch of post ids into an array, I guess with some guard of sorts (so we have to batch with an upper / lower bound) - this is 1 full table scan
  3. We then update the batch in a few goes
  4. and got to 2

Very fiddly to get right. I am not sure it is worth worrying about it.

We can heavily optimise this if we use the full text index here that way we avoid the table scan on raw. Has downsides too cause could be missing from index.

We could filter using posts.id or posts.created_at to avoid full table scans multiple times but I guess we can differ this until it actually becomes a problem when we try to deploy.

wouldn’t this migration lock up many rows and could potentially cause problems

Yeah it will lock all matching rows temporarily. But these will all be ‘old’ posts. It will not prevent new inserts, which is the most common thing in the posts table.

We’ve done this before numerous times (grep db/ for "WHERE cooked LIKE"), and I don’t think it’s caused any problems. I’d prefer to merge as-is, and then we can always improve it later if needed.