[2.x] feat(core): allow to run migrations in isolation - #4864
Open
DavideIadeluca wants to merge 3 commits into
Open
[2.x] feat(core): allow to run migrations in isolation#4864DavideIadeluca wants to merge 3 commits into
DavideIadeluca wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docs PR at flarum/docs#566
Fixes #0000
When Flarum is running in a multi container environment, it's possible that during startup (like during a deployment) multiple containers try to run new migrations at the same time. What can happen is that two containers are provisioned at nearly the identical time, both containers invoke Flarums Migrator and detect that there are new outstanding
upmigrations, one container successfullyupmigrates, the other container tries to up migrate the same new migrations, however will exit because e.g. a column already exists.This becomes an issue in hosting environments like AWS ECS where a non successful exit code of just one single container means that the whole deployment gets rollbacked even though one container ran the new migration successfully.
Changes proposed in this pull request:
Isolatableinterface to allow running them in isolation.MigrateCommandReviewers should focus on:
As Flarum doesn't use Laravels Migrator, this PR (re) implements it similarly to how Laravel implements it. See more information about how Laravel does it at https://laravel.com/docs/13.x/migrations#isolating-migration-execution and https://laravel.com/docs/13.x/artisan#isolatable-commands
The new input option isn't used by default. However we can consider if we wanna pass this input in the
UpdateControllerwhich invokes theMigrateCommand. From what I can tell there should be no negative side effects but technically it could be considered a breaking change which at this point may be undesired.My recommendation tends to not use the new input for the migration command by default. The issue which this PR addresses only occurs in multi container environments where a vendor might already have a heavily customized setup where the migration command is run differently and can be changed to use the
--isolatedinputAbout the PHPStan Stub:
Laravel's
CommandMutexdocblock demands anIlluminate\Console\Command, but its implementation only duck-types the command (getName()andmethod_existschecks), so Flarum's Symfony-based commands are fully compatible at runtime.Screenshot
Necessity
Confirmed
composer test).Required changes: