Problem/Motivation
The actual rollback is very slow because it removes the destination objects and mapping records item by item. In D7, destinations were able to declare a bulkRollback()
method. Migrations with such destinations took advantage of using faster APIs to delete the destination objects.
Proposed resolution
This change doesn't switch any destination plugin to use fast rollback. It only creates the API that allows destination plugins to implement such rollbacks. As a followup, we have to research if some of the existing core destination plugins can take advantage of using fast rollbacks. However the patch contains a testing destination plugin (TestFastRollbackDestination
) proving how such a plugin should be implemented.
Solution: Add back the ability to run fast/bulk rollbacks but make it optional and defaulting to item-by-item rollback.
- A new destination interface
MigrateDestinationFastRollbackInterface
- Destinations able to run fast rollbacks should explicitly demand this by adding the destination setting
fast_rollback: true
in their config, in migration .yml file. - Add event dispatching for such operations.
- A fast rollback can have 2 types: batch or all-in-one rollbacks. By setting the destination setting
fast_rollback_batch_size: 0
, in the migration .yml file, the rollback will run in a single step (all-in-one). When the value is positive integer, that will override the default batch size of 50. - Rework
MigrateExecutable::rollback()
to allow batch and all-in-one fast rollbacks. The actual item-by-item rollback become just a particular case of batch rollbacks where the batch size is 1. - Add a new methods
MigrateIdMapInterface::deleteMultiple()
andMigrateIdMapInterface::deleteAll()
that allows bulk deletion of map items. - Add a test together with a testing destination plugin allowing fast rollbacks.
Remaining tasks
- Decide if some of the core destination plugins can be converted to use fast rollbacks (follow-ups).
User interface changes
None.
API changes
- New interface:
interface MigrateDestinationFastRollbackInterface { public function rollbackMultiple(array $destination_ids); public function rollbackAll(); }
- New methods in
MigrateIdMapInterface
interface:
public function deleteMultiple(array $source_ids, $messages_only = FALSE); public function deleteAll($messages_only = FALSE);
- New migrate events:
MigrateEvents::MAP_DELETE_MULTIPLE
MigrateEvents::MAP_DELETE_ALL
MigrateEvents::PRE_ROW_DELETE_MULTIPLE
MigrateEvents::POST_ROW_DELETE_MULTIPLE
MigrateEvents::PRE_ROW_DELETE_ALL
MigrateEvents::POST_ROW_DELETE_ALL
- New event wrapper classes:
class MigrateMapDeleteAllEvent extends Event {...}
class MigrateMapDeleteMultipleEvent extends Event {...}
class MigrateRowDeleteAllEvent extends Event {...}
class MigrateRowDeleteMultipleEvent extends Event {...}
Data model changes
A destination plugin implementing MigrateDestinationFastRollbackInterface
accepts two new configuration options:
fast_rollback
(bool): If is missed orfalse
, the migration rollback will run on a per-item bases, as the actual behaviour.fast_rollback_batch_size
(int): Has effect only iffast_rollback
istrue
. If is missed the default value isMigrateDestinationFastRollbackInterface::BATCH_SIZE
(50). If is a positive integer, the rollback will run in batches of this size. If is set to zero (0) the rollback will run in a single step.
Comment | File | Size | Author |
---|---|---|---|
#37 | 2821988-37.patch | 33.59 KB | pfrenssen |
Comments
Comment #2
claudiu.cristeaThis is only a PoC
Comment #3
claudiu.cristeaComment #4
claudiu.cristeaComment #5
claudiu.cristeaComment #7
claudiu.cristeawrong namespace
Comment #8
claudiu.cristeaComment #10
mikeryanThis made more sense in D7, when there was for example a node_delete_multiple() which would do bulk table row deletes against node tables. In D8, I don't see such optimizations in the entity storage implementations for the entities themselves - SqlContentEntityStorage::doDeleteFieldItems() does optimize, though, so there would be some gain, according to how many fields your entities have.
Given rollback (especially in high volumes) is a relatively rare operation, I wouldn't be inclined to complicate things unless/until a substantial performance improvement can be demonstrated.
Comment #11
claudiu.cristeaI have a use case where I'm migrating one million of items into a custom table. The rollback iterates million of times and takes a huge amount of time when it simply needs a TRUNCATE.
I agree that, probably, isn't too much gain on entity destinations but, at least. we should let the door open for custom destinations to implement the interface and design their own bulk rollback strategy.
Comment #12
claudiu.cristea@mikeryan,
We need to keep the door open for such rollbacks, even we don't add any use case now in Drupal core. At least we should implement from the patch the following, before Migrate gets stable:
Migrate event constants.
The event objects.
The changes in MigrateExecurable that are allowing bulk rollbacks to be triggered.
The interface.
The MigrateIdMapInterface method addition.
and probably we should add a simple test just to prove that this works + documentation.
What about this plan?
Comment #13
claudiu.cristeaAs I suggested also on IRC, we should keep the door open for such destination plugins that know to perform fast rollbacks.
Comment #15
claudiu.cristeaFixed the batch rollback.
Comment #17
claudiu.cristeaWrong patch.
Comment #19
claudiu.cristeaDestination plugin class was not correctly referred.
Comment #21
claudiu.cristeaComment #22
claudiu.cristeaAdded tests.
@todo: Fix the edge case where the batch size is 1 >>> fallback to normal rollback.
Comment #24
claudiu.cristeaThis looks ready for review.
Comment #25
claudiu.cristeaDocs fixing.
Comment #27
claudiu.cristeaComment #28
claudiu.cristeaComment #29
claudiu.cristeaUpdating IS.
Comment #30
claudiu.cristeaMore Docs & IS fixes.
Comment #32
claudiu.cristeaComment #33
phenaproximaSelf-assigning for review.
Comment #35
pfrenssenRerolled against latest HEAD.
Comment #37
pfrenssenRerolled against 8.4.x.
Comment #38
pfrenssenThis is a feature request so this should be rolled against 8.5.x now. I checked and the patch from #37 applies cleanly.
Comment #39
heddnNW for the small first nit.
Nit: not sure if this is supposed to say, "they were rolled back" or "they were not rolled back". Clarification here would help.
This is a BC break, but the chances of folks building their own map implementation are fairly low. To make this even more BC compatible, perhaps add another interface and implement it on Sql id map that has these methods. I think that the BC break is a bit of a non-starter for some maintainers of migrate, so splitting this out would take away that contention.