safe rename_column seems to be missing? about safe-pg-migrations HOT 13 CLOSED

It would not, safety_assured is only a method of strong_migration that you use around an operation to indicate that you have taken into account strong_migration recommendations.

However, I just had a look at the code again, it appears that we set a global lock timeout around every migrations: https://github.com/doctolib/safe-pg-migrations/blob/master/lib/safe-pg-migrations/base.rb#L32.

So you have nothing to do, it's bundled already :)

from safe-pg-migrations.

Hi @baueric.

I will just quote https://github.com/ankane/strong_migrations#removing-a-column here, they are describing the retrocompatibility issue quite well:

Bad

Renaming a column that’s in use will cause errors in your application.

class RenameSomeColumn < ActiveRecord::Migration[7.0]
  def change
    rename_column :users, :some_column, :new_name
  end
end

Good

A safer approach is to:

1. Create a new column
2. Write to both columns
3. Backfill data from the old column to the new column
4. Move reads from the old column to the new column
5. Stop writing to the old column
6. Drop the old column

safe-pg-migrations is not able to solve this retrocompatibility, as it only acts on the context of one migration during one rollout. Here there are two rollouts needed: one between 3. and 4., one between 5. and 6.

My main concern is doing a rename_column without a lock timeout, is it a limitation of postgres that we can't use a lock_timeout with an alter table rename column?

So you can definitively add a timeout around rename column, but this won't scale as you will still have some errors at some point.

from safe-pg-migrations.

@ChrisCPO @baueric yes it's exactly that. When you deploy your application, everything won't happen at once. With rails it will look like this:

1. run migrations
2. update the code

Between steps 1 and 2, your application is in a weird shape: you have a database using the most recent schema definition, but the code is still using the old schema definition. If your migration is a rename_column, then the column was renamed, but the code is still using the old name.

If you database is small, steps 1 and 2 will happen very close to each other. So it probably doesn't matter. However, the bigger your database is, the longer each steps are taking.

from safe-pg-migrations.

Ahh but execute is protected, seems this can be closed.

from safe-pg-migrations.

Hi @ChrisCPO,

safe-pg-migrations doesn't make rename_column safe. It is not something we could actually do, because it requires a management of retro-compatibility over several rollouts.

Regarding the safe execute, it's a bit missleading. I need to check , but I think this was added because we use execute in the gem, and we don't want to have to add safety_assured around each execute in the gem. I'll explore a bit to see if we can be a bit more clever on that point

Regarding your question, it's more because strong_migration checks directly the name of the current method. So even if execute is marked as safe, as long as rename_column is not included in SAFE_METHODS, it would not be safe.

from safe-pg-migrations.

@ThHareau I don't quite understand what you mean by "requires a management of retro-compatibility over several rollouts". Could you help me and explain it a bit further?

My main concern is doing a rename_column without a lock timeout, is it a limitation of postgres that we can't use a lock_timeout with an alter table rename column?

from safe-pg-migrations.

Awesome thank you @ThHareau for explaining it clearly.

We use column renaming as a preliminary step to removing a column completely. At the point of the rename migration there is a small likelihood the column is still being accessed (reads or writes). I would still like to take advantage of the lock timeout from this gem to ensure I don't lock up my production app while the alter table is queued. So in that case I could just do this in the migration:

safety_assured { rename_column :users, :some_column, :new_name }

Will that effectively set the lock timeout for the operation?

from safe-pg-migrations.

Ah ok, I didn't realize this usage of safety_assured in this gem was just checking if the method is defined before calling it. So basically you designed this gem play nice if you also have strong_migrations installed.

Appreciate the help here.

from safe-pg-migrations.

Yes exactly, we cannot fix every issues highlighted by strong_migrations so we make sure both can be used together

from safe-pg-migrations.

A little self promotion: you can look at an alternative kinda "effortless" renaming of the tables/columns: https://github.com/fatkodima/online_migrations#renaming-a-column
That gem also supports lock retries - https://github.com/fatkodima/online_migrations#lock-timeout-retries.

from safe-pg-migrations.

@ThHareau
I'm still a little confused on:

My main concern is doing a rename_column without a lock timeout, is it a limitation of postgres that we can't use a lock_timeout with an alter table rename column?

So you can definitively add a timeout around rename column, but this won't scale as you will still have some errors at some point.

What errors are normally raised here, are they from postgres or rails expecting the column name active record's attr_accessors etc? If it is rails, seems like a workaround could be to implement a custom reader/writer that checks for the right column name, but then why does rename column not take out an exclusive lock? Wait I think I understand it does take out an exclusive lock but the queued requests would have the old column name, not the new one, because these are opened before and in the lock queue with the old rails cache of the table schema?

from safe-pg-migrations.

@ChrisCPO I believe the errors would be in the case that the column is accessed by the production application between the migration running (column is renamed) and the new production code is deployed. The fully safe approach quoted above uses multiple deploys with write duplication to avoid data loss and a scenario where the column doesn't exist for production code.

from safe-pg-migrations.

I'll close this issue if it's ok, feel free to reopen if needed :)

from safe-pg-migrations.