Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Migrations have required migration_dependencies to indicate when one has a hard dependency on another - for example, a term migration should be run before a node migration that references those terms. MigrateExecutable::import() enforces this by calling checkRequirements() at the beginning of an import operation, which among other things makes sure all the dependent migrations have processed all their source rows.
Rollback should be doing the same thing - since things should be deleted in the reverse order they were imported, each migration should check that all migrations which depend on it have already been rolled back (specifically, that their processed count is 0).
Proposed resolution
Add a checkRollbackRequirements() method to the Migration plugin and MigrationInterface, which is the flip side of checkRequirements - verifying that "later" migrations have no processed rows remaining and throwing RequirementsException if verification fails. MigrateExecutable::rollback() should catch the exception and return MigrationInterface::RESULT_FAILED.
Remaining tasks
Write a fail test (rollback a migration when another migration dependent on it has not been rolled back).
User interface changes
N/A
API changes
checkRollbackRequirements() added to MigrationInterface. This is a BC break and needs careful consideration. In practice, it's unlikely anyone is implementing their own plugin implementing MigrationInterface, so adding an implementation to the Migration plugin class should prevent anything from breaking in the real world.
Data model changes
N/A
Original issue summary
When running migrate rollback operations you can get your database in inconsistent state.
This may happen on two reasons:
First, migrations are arranged by group and this breaks dependencies. See https://www.drupal.org/node/2830357, it needs to be resolved.
Second, when running migration rollbacks, if some migration fails the rollback process ignores this and just CONTINUES. In this case, the subsequent migration on which the failed one depends on, would be rolled back which could bring two major problems:
1) the failed migration could be unable to rollback ever.
2) any further migration imports could be impossible.
Example:
Let's put migration A_child depends on migration B_parent.
And A_child creates entities of type A which reference entities created by B_parent (A -> B)
Now let's set A_child status to some not-Idle value by running its import and immediately stopping it for example.
Now let's rollback both with `drush mr --all` for example.
What happens?
A_child is skipped because its status is wrong and only B_parent is rolled back.
Now what we have in the db? Right, A entities now reference to non-existing B entities.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | migrate_tools-2838995.patch | 4.72 KB | OnkelTem |
Comments
Comment #2
OnkelTem CreditAttribution: OnkelTem at Adyax commentedAttaching a patch.
It adds tracking of result statues of rollback operations and prevents running rollbacks of "parent" migrations if "children" ones failed.
NOTE: The patch is created from a branch with this patch applied first: https://www.drupal.org/node/2830357
Comment #3
mikeryanThe base problem here is that the core rollback() support does not verify dependencies are complete on rollback() as it does for import() - moving to the core queue to deal with that.
Actually, I don't believe there's a data integrity issue here - the deletion of the B entities should remove all references to them (e.g., if you manually delete a term through the UI, then any entity references to that term by all nodes will be removed).
Comment #4
mikeryanComment #6
heddnI think we agree that adding things to the interface would be a BC break. But does this *have* to happen? Is this going to block migrate going stable? We can always add another RollbackableInterface or such at a later date and do an instance_of check. It isn't pretty, but it would work.
So, I'm removing the BC break tag. And I'm also marking this as a feature, not a bug. Again, this is a nice to have. If you *really* need to rollback, drop the database, truncate tables, do a fresh install. My point is, there are work arounds.
Comment #7
freelockJust adding a couple scenarios we've hit with this -- largely related to multiple migrations targeting the same entities.
I know there's an issue related to D6 term_node migrations getting rolled into the main migration -- term_node migrations currently target the same nodes that were previously migrated by a node migration. When I last worked with this, it ended up being extremely dangerous -- if you rolled back a term_node migration, it deleted the entire nodes that were actually created during the node_migration. That suggests we need to do away with the term_node migrations before this issue could work -- #2597650: Term references should be incorporated into D6 node migration.
But we have an entirely custom migration that relies on this ability to work as well -- we are migrating in data from several different CSV files, some of which essentially are getting added to the original entities as multiple values of a field. We modeled this on the term_node migration, and it works great for helping to normalize the data into something that works well with Drupal.
I guess for these types of migration, the only answer I can think of is "you can't reliably roll back one of these migrations." You have to roll back all, or you end up with missing entities. So if there is a dependency requirement for rolling back, it needs to consider this scenario, and not leave us in a state where we can't roll back all the migrations that overload an entity.
Comment #12
m.stentaBumping this to 9.3.x - encountering this in testing the farmOS 2.x migrations. Not critical, but makes rollbacks harder because we're using Entity Reference Integrity to prevent entities from being deleted if other entities reference them.