Silence strong migration warning #1037
Conversation
|
Well Done! Your tests are still passing. |
| @@ -1,6 +1,6 @@ | |||
| # rubocop:disable Style/NumericLiterals | |||
| unless Rails.env.production? | |||
| StrongMigrations.start_after = 20200205123840 | |||
| StrongMigrations.lock_timeout_limit = 86400.seconds | |||
There was a problem hiding this comment.
This is a timeout of 24h, do we expect a migration to take longer?
There was a problem hiding this comment.
The problem with this check is, that it not actually checks how long a transaction takes but rather what mariadb is configured to (aka when to cancel a transaction).
See: https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_lock_wait_timeout which is fetched by the check here: https://github.com/ankane/strong_migrations/blob/master/lib/strong_migrations/adapters/mysql_adapter.rb#L35
To make this sane, we would need to ship our own mariadb configuration since the default is 31536000 and us setting 86400 will trigger this check and fail always with default configuration.
There was a problem hiding this comment.
Alright, I see. Why not remove that line entirely then instead of setting to 0?
In glue we're setting timeouts for migrations, which I guess was the goal here too when we added that line:
StrongMigrations.lock_timeout = 20.seconds
StrongMigrations.statement_timeout = 2.minutes
Would it make sense to set some limits for RMT too, so that a migration cannot block the db endlessly?
There was a problem hiding this comment.
I agree we should set the lock_timeout and statement_timeout, then also the lock_timeout_limit works. But without you will run into: https://github.com/ankane/strong_migrations/blob/master/lib/strong_migrations/checker.rb#L162 which then is setting by default: https://github.com/ankane/strong_migrations/blob/master/lib/strong_migrations.rb#L59
I'm not sure if setting a statement_timeout is really wanted, since a migration can take a long time which is completely fine.
There was a problem hiding this comment.
Ah, ok that's unexpected that strong migrations sets it's own default and fails then. Maybe a comment explaining this would help.
Regarding the statement_timeout, it's possible to change it to a higher value for specific migrations that are known to take long.
Anyway, strong_migrations isn't used in production anyway it seems in RMT.
Description
Strong migration shows lock timeout limit exceeding with migrations. This PR silences such warnings.
Change Type
Please select the correct option.
Checklist
Please check off each item if the requirement is met.
rubocop.MANUAL.mdfile with any changes to the user experience.package/obs/rmt-server.changes.Other Notes
Please use this space to provide notes or thoughts to the team, such as tips on how to review/demo your changes.