Currently, batch_limit for drush migrate-import is 1/100 of total items to process. It's fine (or doesn't matter) for sources like SQL but for fairly large file-based (xml in my case) sources it seems to slow down the process with each new batch. First 1-3 batches of 25k items import almost instantly, then it starts to slow down considerably. No big deal for a one-time process but in my case it's a recurrent (daily) migration. The source file isn't large so I think increasing batch_limit would help.

Is there a way other than hacking the code?

CommentFileSizeAuthor
#60 migrate_tools-3067165-58.patch15.23 KBklausi
#42 3067165-reroll.patch16.62 KBaurelianzaha
#38 migrate_tools-batch_size-3067165-38.patch16.99 KBgaards
#37 migrate_tools-batch_size-3067165-37.patch16.99 KBgaards
#36 3067165-36.patch60 KBtimohuisman
#30 migrate_tools-batch_size-3067165-30.patch525 bytesmanuel.adan
#29 3067165-29.patch13.29 KBtimohuisman
#23 3067165-23.patch20.62 KBtimohuisman
#18 migrate_tools-3067165-batch_size-18.patch4.6 KBmarios anagnostopoulos
#8 migrate_tools-3067165-batch_size-8.patch4.59 KBjayelless
#7 migrate_tools-3067165-batch_size-7.patch4.56 KBjayelless
#5 migrate_tools-3067165-batch_size-5.patch4.56 KBrob_pr
#3 migrate_tools-3067165-batch_size-3.patch4.25 KBjayelless

Issue fork migrate_tools-3067165

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

scoff created an issue. See original summary.

scoff’s picture

scoff commented

Same migration, batch_limit 251, 1000 and 2507 (changed in MigrateBatchExecutable.php)

$ date && drush mim xmlsql && date
Thu Jul 11 13:10:13 +10 2019
Processed 251 items (251 created, 0 updated, 0 failed, 0 ignored) - continuing with 'xmlsql'
...
Processed 219 items (1 created, 0 updated, 0 failed, 218 ignored) - done with 'xmlsql'
Thu Jul 11 13:27:30 +10 2019

~17 minutes

$ date && drush mim xmlsql && date
Thu Jul 11 13:34:17 +10 2019
Processed 1000 items (1000 created, 0 updated, 0 failed, 0 ignored) - continuing with 'xmlsql'
...
Processed 68 items (0 created, 0 updated, 0 failed, 68 ignored) - done with 'xmlsql'
Thu Jul 11 13:39:23 +10 2019

~5 minutes

$ date && drush mim xmlsql && date
Thu Jul 11 13:41:23 +10 2019
Processed 2507 items (2507 created, 0 updated, 0 failed, 0 ignored) - continuing with 'xmlsql'
...
Processed 2505 items (2082 created, 0 updated, 0 failed, 423 ignored) - done with 'xmlsql'
Thu Jul 11 13:43:39 +10 2019

~2 minutes

jayelless’s picture

jayelless commented
StatusFileSize
new migrate_tools-3067165-batch_size-3.patch4.25 KB

I have created a patch which allows a new parameter called "batch_size" to be included in the options array provided to the constructor of the MigrateBatchExecutable class. This value is used to set the size of each batch within the limit for each migrate import.

jayelless’s picture

jayelless commented
Title: An option to override batch limit? » An option to override batch size?
rob_pr’s picture

rob_pr commented
Status: Active » Needs review
Related issues: +#2701121: New batch started because of not enough reclaimed memory seems to forget which migrations have already run leading to missing migration dependencies
StatusFileSize
new migrate_tools-3067165-batch_size-5.patch4.56 KB

I re-rolled the patch for the current 4.x-dev and added a few minor fixes.

Since drush migrate:import will use MigrateBatchExecutable soon, I added "--batch-size" as an option.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Version: 8.x-4.x-dev » 8.x-5.x-dev
Status: Needs review » Needs work

Let's re-roll this for 5x please and seems there is still some CS issues.

src/MigrateBatchExecutable.php ✗ 1 more
line 49	Missing @var tag in member variable comment
jayelless’s picture

jayelless commented
StatusFileSize
new migrate_tools-3067165-batch_size-7.patch4.56 KB

Patch re-rolled for 5.x branch. Note that this patch also applies to the latest 4.x releases.

jayelless’s picture

jayelless commented
StatusFileSize
new migrate_tools-3067165-batch_size-8.patch4.59 KB

Patch updated to add missing @var tag.

jayelless’s picture

jayelless commented
Status: Needs work » Needs review
amjad1233’s picture

amjad1233
MJED
Location Brisbane
commented
Status: Needs review » Reviewed & tested by the community

@jlscott Looks good to me. Just deployed to one of our projects seems working as expected.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Reviewed & tested by the community » Closed (duplicate)

The great gitlab migration is upon us. See https://gitlab.com/drupalspoons/migrate_tools/-/issues/71. The latest patch from here is posted to https://gitlab.com/drupalspoons/migrate_tools/-/merge_requests/3.

zcht’s picture

zcht commented

It seems that I would need exactly this patch for my migration :) However, it doesn't seem to work for me when I take the patch from the #8 comment and install it. I'm using the current 8.x-5.x-dev version of the module, under Drupal 8.9.3. I wanted to do the migration with an example size:

drush migrate:import my_taxonomy_migration --batch-size=5

I get the following error:

<?php
 [error]  Error: Class 'Drupal\migrate\Bytes' not found in Drupal\migrate\MigrateExecutable->__construct() (line 109 of /var/www/html/docroot/core/modules/migrate/src/MigrateExecutable.php) #0 /var/www/html/docroot/modules/contrib/migrate_tools/src/MigrateExecutable.php(99): Drupal\migrate\MigrateExecutable->__construct(Object(Drupal\migrate\Plugin\Migration), Object(Drupal\migrate_tools\Drush9LogMigrateMessage))
#1 /var/www/html/docroot/modules/contrib/migrate_tools/src/Commands/MigrateToolsCommands.php(407): Drupal\migrate_tools\MigrateExecutable->__construct(Object(Drupal\migrate\Plugin\Migration), Object(Drupal\migrate_tools\Drush9LogMigrateMessage), Array)
#2 [internal function]: Drupal\migrate_tools\Commands\MigrateToolsCommands->rollback('my_taxonomy_migration...', Array)
#3 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array(Array, Array)
#4 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#5 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#6 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(302): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#7 /var/www/html/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#8 /var/www/html/vendor/symfony/console/Application.php(1005): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /var/www/html/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 /var/www/html/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(49): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 /var/www/html/vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run(Array)
#14 /var/www/html/vendor/drush/drush/includes/preflight.inc(18): require('/var/www/html/v...')
#15 phar:///usr/local/bin/drush/bin/drush.php(141): drush_main()
#16 /usr/local/bin/drush(10): require('phar:///usr/loc...')
#17 {main}.
?>

As a test, I have included in MigrateExecutable.php file -> use Drupal\Component\Utility\Bytes; but that doesn't seem to work either. I get the following error:

1/124 [>---------------------------]   0% [error]  Error: Call to undefined method Drupal\Component\Utility\Bytes::toNumber() in Drupal\migrate\MemoryManager->__construct() (line 68 of /var/www/html/docroot/core/modules/migrate/src/MemoryManager.php) #0 /var/www/html/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php(259): Drupal\migrate\MemoryManager->__construct(Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher), '0.9', '0.85')
#1 /var/www/html/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php(173): Drupal\Component\DependencyInjection\Container->createService(Array, 'migrate.memory_...')
#2 /var/www/html/docroot/core/lib/Drupal.php(158): Drupal\Component\DependencyInjection\Container->get('migrate.memory_...')
#3 /var/www/html/docroot/core/modules/migrate/src/MigrateExecutable.php(150): Drupal::service('migrate.memory_...')
#4 /var/www/html/docroot/core/modules/migrate/src/MigrateExecutable.php(466): Drupal\migrate\MigrateExecutable->getMemoryManager()
#5 /var/www/html/docroot/core/modules/migrate/src/MigrateExecutable.php(338): Drupal\migrate\MigrateExecutable->checkStatus()
#6 /var/www/html/vendor/drush/drush/includes/drush.inc(206): Drupal\migrate\MigrateExecutable->rollback()
#7 /var/www/html/vendor/drush/drush/includes/drush.inc(197): drush_call_user_func_array(Array, Array)
#8 /var/www/html/docroot/modules/contrib/migrate_tools/src/Commands/MigrateToolsCommands.php(412): drush_op(Array)
#9 [internal function]: Drupal\migrate_tools\Commands\MigrateToolsCommands->rollback('my_taxonomy_migration...', Array)
#10 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array(Array, Array)
#11 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#12 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#13 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(302): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#14 /var/www/html/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#15 /var/www/html/vendor/symfony/console/Application.php(1005): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#16 /var/www/html/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#17 /var/www/html/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#18 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#19 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(49): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#20 /var/www/html/vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run(Array)
#21 /var/www/html/vendor/drush/drush/includes/preflight.inc(18): require('/var/www/html/v...')
#22 phar:///usr/local/bin/drush/bin/drush.php(141): drush_main()
#23 /usr/local/bin/drush(10): require('phar:///usr/loc...')
#24 {main}.

Any idea how I could fix it? My taxonomy migrations are very small, but I would like to use this patch for my node migrations, keeping the batch size as small as possible, since the node migration is very large (>60k nodes).

Thanks a lot for the great work :)

mrweiner made their first commit to this issue’s fork.

rob_pr opened merge request !7

rob_pr’s picture

rob_pr commented
Status: Closed (duplicate) » Needs review

I updated the patch from #8 for the current 5.x-dev and started a merge request on drupalcode.org.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Needs work

I like the direction this is going. Can we add test coverage? Either to drush or the UI for adjusting the batch size?

marios anagnostopoulos’s picture

marios anagnostopoulos commented

Would it be a good addition to allow for a more global settings? For example in the migration group, which will be overridden per migration if the setting in this patch is filled.

To implement that we would need changes in both the migrate tools & migrate plus modules though, and I have not idea how we should handle this in the issue queue.

I believe it is worth it though, and am willing to work on it if you can guide me on how to patches that are dependent to one another. Any thoughts?

Edit: Maybe we could also consider adding setting in the migration configurations or is it overkill?

marios anagnostopoulos’s picture

marios anagnostopoulos commented
StatusFileSize
new migrate_tools-3067165-batch_size-18.patch4.6 KB

Additionally the patch in #8 does not apply for some reason, some line numbers in the patch are off (I am using 5.1.0 of migrate_tools and no other patch for the module, also checked it with the 8.x-5.x branch. Maybe I am doing something wrong, but for anyone facing the same issue, here is reroll i guess.

james.williams made their first commit to this issue’s fork.

james.williams’s picture

james.williams
Location Coventry, UK
commented

I've just brought across the work from #2701121: New batch started because of not enough reclaimed memory seems to forget which migrations have already run leading to missing migration dependencies , which expands the scope of this issue a bit, but as @heddn suggested in #2701121-53: New batch started because of not enough reclaimed memory seems to forget which migrations have already run leading to missing migration dependencies , combining forces on these two very related issues should be well worth it.

I've stuck with the 'batch-size' option from this issue, rather than using the 'batch' option from the other one. I've then added a single commit of my own work on top, to allow specifying the 'batch-size' option as either a percentage of the total number of items, or an absolute number of items to process in each iteration. This resolves the 'todo' comment in the existing code, which was placed there in the original issue that added batching in any form (see #2470882-16: Implement running migration processes through the UI), so that the limit can be set usefully for both large and small migrations.

Meanwhile, I'm hiding all the patches here, as the merge request is probably the thing that should be used nowadays. (The most recent patch was simply a re-rolled version that had nothing but trivial differences to the merge request.)

Leaving as 'needs work', as comment 16 sensibly asked for more test coverage.

Note that any potential credit for people from #2701121: New batch started because of not enough reclaimed memory seems to forget which migrations have already run leading to missing migration dependencies should be brought across, should this work get accepted.

radelson’s picture

radelson commented

MigrateBatchExecutable is not compatible with the --sync option.

Not directly related with this issue description but it seems apt to mention it here. The --sync option isn't supported from the UI but the patch is introducing support for the Drush command, which advertise the --sync option.

Maybe we shouldn't support the --sync option when used with the --batch-size option and postpone the work to make it compatible ? The command could fail and warn the user that the flags aren't compatible.

We replaced our existing migrations to use batching and we ended up working around that limitation by running them first without batching and with the --limit option. We sync first and then import in a second step.

HitchShock made their first commit to this issue’s fork.

timohuisman’s picture

timohuisman
Primary language Dutch
Location Leiden, Netherlands
commented
StatusFileSize
new 3067165-23.patch20.62 KB

We use composer patches to apply patches. Merge request patch urls are updated for every commit on the branch, which makes it a moving target. Attached is a patch containing the current state of the merge request.

andycarlberg made their first commit to this issue’s fork.

andycarlberg opened merge request !19

andycarlberg’s picture

andycarlberg
he/him
commented
Version: 8.x-5.x-dev » 6.0.x-dev
Status: Needs work » Needs review

I rerolled this against the 6.x version and opened a new PR. I'm updating the version on this issue accordingly.

I haven't rerolled against a major version before. If I should have opened a new issue, let me know and I will do so.

gpolonus made their first commit to this issue’s fork.

grifstuf’s picture

grifstuf commented

To fix a type error, we had to add a boolean to integer type cast in the MigrateBatchExecutable class. This is because drush brings in CLI option flags as booleans but the class requires integers for the update and force options as opposed to booleans. This type cast did not happen automatically because the MigrateBatchExecutable class has strict_types enabled for that file.

While this fixes the type error encountered when running these batch migrations for now, it is fair to say that the MigrateBatchExecutable class should be able to accept booleans as well as integers. The values for the update and force options are used for their falsiness or truthiness, so semantically the class should accept booleans as well as integers for these options. It might be worth having another issue to fix this tech debt for anyone else wishing to interface with this class in the future.

timohuisman’s picture

timohuisman
Primary language Dutch
Location Leiden, Netherlands
commented
StatusFileSize
new 3067165-29.patch13.29 KB

This patch contains the current state of the merge request.

manuel.adan’s picture

manuel.adan
Location 🌌
commented
Title: An option to override batch size? » Option to override batch size
Component: Drush commands » Code
Category: Support request » Feature request
StatusFileSize
new migrate_tools-batch_size-3067165-30.patch525 bytes

The new option might take some time to be fully implement. A "security" max limit could be useful in the meantime.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Assigned: Unassigned » naveenvalecha

Assigning myself to review & test the PR.

jardasmahel made their first commit to this issue’s fork.

jardasmahel’s picture

jardasmahel commented

The tests are passing on my local however they are failing in CI.
Can someone share some wisdom on it?

lando phpunit --group=migrate_tools
PHPUnit 8.5.31 by Sebastian Bergmann and contributors.

Testing
............................

......
.. 36 / 36 (100%)

Time: 4.84 minutes, Memory: 353.00 MB

OK (36 tests, 328 assertions)

HTML output was generated
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...
https://drupal-contributions.lndo.site/sites/simpletest/browser_output/D...

eleonel made their first commit to this issue’s fork.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Needs review » Reviewed & tested by the community

PR - https://git.drupalcode.org/project/migrate_tools/-/merge_requests/19
Looks good to me.

timohuisman’s picture

timohuisman
Primary language Dutch
Location Leiden, Netherlands
commented
StatusFileSize
new 3067165-36.patch60 KB

This patch contains the current state of the merge request. See "Patches from drupal.org merge request URLs are dangerous?" for more information.

gaards’s picture

gaards commented
StatusFileSize
new migrate_tools-batch_size-3067165-37.patch16.99 KB

The latest patch provided here didn't apply, so I created a new one from the latest diff of merge request 19.

gaards’s picture

gaards commented
StatusFileSize
new migrate_tools-batch_size-3067165-38.patch16.99 KB

Updated the patch with a small fix to the option name which was incorrect, causing the batch size to not work (thanks to olli for spotting it).

thisisalistairsaccount’s picture

thisisalistairsaccount commented

Hoping if things are all good here that we could get this into a future release for the module? This would be beneficial for some people we're working with and would love to see it included :)

Kfootm70’s picture

Kfootm70 commented

Much like the above comment, is there a date of release for this module please? Thanks team!

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Assigned: naveenvalecha » Unassigned

forgot to unassign from myself

aurelianzaha’s picture

aurelianzaha commented
StatusFileSize
new 3067165-reroll.patch16.62 KB

attached is the patched re-rolled against 6.0.x

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Status: Reviewed & tested by the community » Needs work

I'm ignoring the patches (though I understand why some people are adding them).

We will use the MR. Since there are 2 open. we will use the more recent reroll.

But, the latest MR (19) needs a merge conflict resolved, so moving to needs work:

https://git.drupalcode.org/project/migrate_tools/-/merge_requests/19/con...

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Status: Needs work » Needs review

Back to needs review.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

I've compared MRs 7 and 19 to see what has changed other than coding standards and formatting and these are the things I found.

-    $migration_dependencies = $migration_plugin->getMigrationDependencies();
+    $migration_dependencies = $migration_plugin->getMigrationDependencies(TRUE);

-      $migration_dependencies = $migration->getMigrationDependencies();
+      $migration_dependencies = $migration->getMigrationDependencies(TRUE);

+   * @option batch-size Optionally use batch iterations, with a limit on the
+   *   number of items to process in each batch iteration.

The description is substantially shorter in the latest MR, but then that makes it much easier to read. Perhaps the extra details could be moved into the README or some other documentation.

+      // Integer cast required because MigrateBatchExecutable requires the
+      // `update` and `force` options to strictly be integers.
+      $options['update'] = (int) $options['update'];
+      $options['force'] = (int) $options['force'];

+    // If this batch is run via Drush, we need to initialize the progress bar
+    // for the background process.
+    if (PHP_SAPI === 'cli') {
+      $output = Drush::output();
+      $output->setDecorated(TRUE);
+      // Initialize the Symfony Console progress bar.
+      \Drupal::service('migrate_tools.migration_drush_command_progress')->initializeProgress(
+        $output,
+        $migration,
+        $options,
+      );
+    }
+    
+    $executable = new MigrateBatchExecutable($migration, $message, $options);

   public function onPostRowDelete(MigrateRowDeleteEvent $event) {
     if ($this->feedback && ($this->deleteCounter) && $this->deleteCounter % $this->feedback == 0) {
-      $this->rollbackMessage(FALSE);
+      $this->progressMessageEmit(FALSE);
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

I got confirmation from @heddn via Slack that these changes above look okay so this can move forward with a round of testing for the *new* MR. Once that happens and this is RTBC with the new MR, then we can see if @heddn or one of the other comaintainers can get this in.

emixaam’s picture

emixaam commented

Sucessfully tested MR 19 on a large custom migration (42000 nodes) with --batch-size of 1000 and 2000, with and without the --update flag, in a local environment and on a dev server. migrate-import correctly processes the given amount of items, and the duration for each batch stays similar.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue tags: +Needs manual testing

Hmm... that's disappointing. Anyone else able to test?

amaisano’s picture

amaisano commented

What's disappointing? emixaam's comment seems ideal, no?

I found that the `--force` command does not mix well with `--batch-size` either... it fails all migrations if their dependencies are not fulfilled. If I run a migration without --batch-size using --force, it works as expected, ignoring the dependencies.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

I must have misread it before so please ignore my previous comment.

amaisano’s picture

amaisano commented

Besides the Array to String conversion, this patch also is unable to accept custom drush migrate:import parameters:

$ drush mim --tag=node_translation --continue-on-failure --language=es,it --batch-size=1000
 [warning] Array to string conversion Process.php:341
>  [error]  Migration failed with source plugin exception: The &quot;language&quot; option does not exist. in /data/3ds/swd10/releases/4.19.12-RC/src/vendor/symfony/console/Input/Input.php line 140

I assume this will also affect the --migrate-debug flag.

These are both added using official drush extension setups:

services:
  sw_migrate.commands:
    class: \Drupal\sw_migrate\Commands\SwMigrateCommands
    tags:
      - { name: drush.command }
<?php

namespace Drupal\sw_migrate\Commands;

use Consolidation\AnnotatedCommand\CommandData;
use Drush\Commands\DrushCommands;

/**
 * Extends migrate-import with a language-filtering option.
 */
class SwMigrateCommands extends DrushCommands {

  /**
   * Register new options for the migrate:import command.
   *
   * @hook command migrate:import
   * @option language Enable language filtering
   * @usage drush migrate-import --language=fr,it
   *   Run translation-based migration(s) in the specified languages only.
   */
  public function additionalOptionsMigrateImport(CommandData $commandData, $options = ['language' => NULL]) {
    // No action required here. The new options will be examined during the
    // source plugin if required.
  }

}

... so I am surprised that adding the --batch-size parameter is suddenly ignoring them?

ccjjmartin’s picture

ccjjmartin commented

I am not sure how the merge requests are performing as they wouldn't apply to the last release of the module but the patch in #42 was applied to 6.0.4 and for me it significantly slows down the migration when I use the batch size option. Over an 8 minute time period before the patch I was able to update 10,421 users. Over another 8 minute time period with the batch size option I was only able to process 2,001 users. So it was almost 5x faster without this option. I have 200,000 users to migrate though so I am not sure if there is added overhead for breaking this down into 1,000 user chunks. I am performing a sql database to sql database migration so I am not sure if this would perform better with xml or csv sources.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Needs work

This could use a rebase.

knowak changed the visibility of the branch migrate_tools-3067165-3067165-6.0.x to hidden.

knowak changed the visibility of the branch migrate_tools-3067165-3067165-6.0.x to active.

knowak changed the visibility of the branch migrate_tools-3067165-6.0.x to hidden.

emixaam’s picture

emixaam commented

@knowak I believe the "Merge remote-tracking branch 'origin/6.0.x' into migrate_tools-3067165-6.0.x" (commit 642257de) removed the `batch-size` option from `MigrateToolsCommands.php`, so using the latest version of Merge request !19 cause this error:

./vendor/bin/drush mim my_migration --batch-size=10

  The "--batch-size" option does not exist.

Adding `'batch-size' => self::REQ,` in the import function (MigrateToolsCommands.php) seems to restore the functionality.

klausi made their first commit to this issue’s fork.

klausi opened merge request !103

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Needs review
StatusFileSize
new migrate_tools-3067165-58.patch15.23 KB

Sorry, had to create a new merge request to target the latest 6.1.x branch. I just merged the 6.1.x branch, no other changes.

Also uploading stable patch file for composer patches.

klausi changed the visibility of the branch 3067165-6.0.x to hidden.

klausi changed the visibility of the branch migrate_tools-3067165-3067165-6.0.x to hidden.

klausi changed the visibility of the branch 3067165-an-option-to to hidden.

  • heddn committed bf849bf5 on 6.1.x authored by klausi 
    feat: #3067165 Option to override batch size

By: rob_pr
By: heddn
By:...
heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Fixed
Issue tags: -Needs manual testing

Thanks everyone for your long patience with this issue.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

mstrelan’s picture

mstrelan commented

This broke out tests, opened #3590774: Extending MigrateBatchExecutable broken since 6.1.4