New batch started because of not enough reclaimed memory seems to forget which migrations have already run leading to missing migration dependencies

Looks like a dupe of #2797505: Migrations fail due to missing dependency when dependency has skipped rows by the source plugin, which has more info.

I do not think this is a duplicate.
The problem is that it's saying "starting new batch", but it's actually not!
That why the next migration is saying that the other migration is "missing" (this is a little vaguely worded, it could also mean that the migration you have a dependency on did not fully complete yet.)

If a new batch is not getting started, that would be a problem with the drush commands, not core.

@jeroen.b - can you describe what you're seeing in more detail? What's telling you a new batch is not being started?

@mikeryan the fact that it immediately stops after saying it's starting a new batch and that it didn't migrate all items.
But this bug might even be in core migrate. When I check the code in:
http://cgit.drupalcode.org/drupal/tree/core/modules/migrate/src/MigrateE...

The only thing it's actually doing after "starting new batch", is breaking out of the import loop and resetting the import status to idle.

It's indeed probably in the Drush commands. In the Drupal 7 command it loops until the status of the migration is completed.
Every batch is imported in a subshell with drush_migrate_invoke_process, which uses drush_invoke_process. The Drupal 8 version does nothing of this kind and runs everything in the same process.

What probably should happen is that the MigrateExecutable of Migrate Tools actually supports running import()/rollback() with batch support (or something like batch support, as in Drupal 7 with drush_invoke_process to split up the migration in parts and start a subshell for every part).

I'm currently using the patch from https://www.drupal.org/node/2701335 which allows me to do the full import without going over memory limit.

I'm encountering a similar issue. Running a large user import consistently stops around 250 as described above. However, running the command a number of times does churn through the entire set of users and subsequent migrations which depend on the users are then able to run.

One thing to note, that the patch from #2701335-13: Run garbage collection during migration memory reclamation is included in the latest core releases, which I am running. The memory consumption seems to decrease by an insignificant amount, about 500K.

I've faced this issue too when was working on a migration (import) of a CSV with 63k rows. Eventually how I solved it was through the approach in #10 - I simply wrote an extra bash script that would repeat `drush migrate-import` until 0 new items were added. But that's basically not right. The migrate_tools module itself should be able to cope with it.

So first, let me show how to reproduce the issue. Given you start with a vanilla Drupal 8.4 core do the following:

1. Download and install all the dependencies: migrate_plus, migrate_source_csv, migrate_tools
2. Import a dummy migration (it just imports a set of nodes -- something to keep our memory consumption grow linearly from the amount of rows in the source. I attach the migration export in the file "migration_export.yml" (you can import it at /admin/config/development/configuration/single/import)
3. Get a dummy CSV source that accompanies the import - I attach it under the name "issue-2701121.txt"
4. Limit the memory for PHP. This part might depend on your php environment (and other things). In my case putting ini_set('memory_limit', '48M'); into settings.php produced acceptable outcome. The idea here is that we wanna impose such a memory limit on PHP that it should exhaust it before it would be able to process 10k nodes from the CSV source.
5. Run drush migrate-import reproduce_issue_2701121.
6. Observe something like the following in the console:
   

   bucefal91@dell /var/www/localhost.com/drupal8/sites/default/files $ drush migrate-import $migration
   Memory usage is 40.83 MB (85% of limit 48 MB), reclaiming memory.                                                                                                                                       [warning]
   Memory usage is now 43.75 MB (91% of limit 48 MB), not enough reclaimed, starting new batch                                                                                                             [warning]
   Processed 93 items (93 created, 0 updated, 0 failed, 0 ignored) - done with 'reproduce_issue_2701121'
   

   whereas expected would be that it kicks off another batch and continues from where it left off (93 nodes).

I will compose a patch that spins off the import process through drush implementation of batches (child OS processes).

Here's a relatively raw patch. I am using the same executable as for batch via browser (MigrateBatchExecutable). Luckily it worked for drush without any adjustments.

With this patch my scenario from #11 looks like the following in the console:

bucefal91@dell /var/www/localhost.com/drupal8/modules/migrate_tools $ drush migrate-import $migration
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Importing Reproduces the issue #2701121 (5%).                                                                                                                                                           [ok]
Processed 0 items (0 created, 0 updated, 0 failed, 0 ignored) - done with 'reproduce_issue_2701121' 

Note: the percentage progress (and so is the last finish message) are broken in Drush batch API implementation - I believe the bug for it is https://github.com/drush-ops/drush/pull/1484 (though I did not dig too deep there, I bailed out as soon as I asserted that it's Drush fault and not ours).
Seems like feedback becomes obsolete with this new patch because basically batch API does the intermediate messages itself.

Another note: my drush is version 8.0.1. I will try it on Drush 9 in the nearest future (my patch includes a fix for Drush 9, but I just haven't tried it :) )

Now I remove the --feedback option from drush interface and improve the batch progress message. Tested via web UI and via drush 8. Drush 8 terminal log:

bucefal91@dell /var/www/localhost.com/drupal8/modules/migrate_tools $ drush migrate-import $migration
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'reproduce_issue_2701121'                                                                                                [ok]
Processed 0 items (0 created, 0 updated, 0 failed, 0 ignored) - done with 'reproduce_issue_2701121'       

I've just tested on Drush 9 and it also works. As far as I can see this latest patch #13 is worth reviewing/testing by others.

Since we don't have any tests on this, can we get any other manual testing to confirm things related to this patch?

But first, this needs a reroll.

re-rolled

FILE: ...pal/modules/contrib/migrate_tools/src/MigrateBatchExecutable.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 196 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: )
 216 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: )

And needs some tests.

Patch in #16 gives me the following errors (Drush 8 / Drupal 8.6 / Migrate Tools 8.4):

TypeError: Argument 3 passed to Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport() must be of the type array, object given, called in /app/vendor/drush/drush/commands/core/drupal/batch.inc on line 163 in Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport() (line 171 of /app/web/modules/contrib/migrate_tools/src/MigrateBatchExecutable.php)
#0 /app/vendor/drush/drush/commands/core/drupal/batch.inc(163): Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport('wpbo_migrate_in...', Array, Object(DrushBatchContext))
#1 /app/vendor/drush/drush/commands/core/drupal/batch.inc(111): _drush_batch_worker()
#2 /app/vendor/drush/drush/includes/batch.inc(98): _drush_batch_command('210')
#3 /app/vendor/drush/drush/commands/core/core.drush.inc(1212): drush_batch_command('210')
#4 /app/vendor/drush/drush/includes/command.inc(422): drush_core_batch_process('210')
#5 /app/vendor/drush/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#6 /app/vendor/drush/drush/includes/command.inc(199): drush_command('210')
#7 /app/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#8 /app/vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#9 /app/vendor/drush/drush/drush.php(12): drush_main()
#10 {main}.
TypeError: Argument 3 passed to Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport() must be of the type array, object given, called in /app/vendor/drush/drush/commands/core/drupal/batch.inc on line 163 in /app/web/modules/contrib/migrate_tools/src/MigrateBatchExecutable.php on line 171 #0 /app/vendor/drush/drush/commands/core/drupal/batch.inc(163): Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport('wpbo_migrate_in...', Array, Object(DrushBatchContext))
#1 /app/vendor/drush/drush/commands/core/drupal/batch.inc(111): _drush_batch_worker()
#2 /app/vendor/drush/drush/includes/batch.inc(98): _drush_batch_command('210')
#3 /app/vendor/drush/drush/commands/core/core.drush.inc(1212): drush_batch_command('210')
#4 /app/vendor/drush/drush/includes/command.inc(422): drush_core_batch_process('210')
#5 /app/vendor/drush/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#6 /app/vendor/drush/drush/includes/command.inc(199): drush_command('210')
#7 /app/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#8 /app/vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#9 /app/vendor/drush/drush/drush.php(12): drush_main()
#10 {main} TypeError: Argument 3 passed to Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport() must be of the type array, object given, called in /app/vendor/drush/drush/commands/core/drupal/batch.inc on line 163 in Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport() (line 171 of /app/web/modules/contrib/migrate_tools/src/MigrateBatchExecutable.php).
Drush command terminated abnormally due to an unrecoverable error.       [error]

The $context parameter in various functions in MigrateBatchExecutable expects an array. During debugging I'm receivinig an ArrayObject however. Patch attached changes this - totally unsure however if this is the right way to go.

Drush 8 on Drupal 8.6 isn't supported. See http://docs.drush.org/en/master/install/. Drush 8 was dropped in 8.3 of Drupal. Try updating to Drush 9 and seeing if #16 works?

1. +++ b/src/Commands/MigrateToolsCommands.php
   @@ -229,7 +230,6 @@ class MigrateToolsCommands extends DrushCommands {
   -   * @option feedback Frequency of progress messages, in items processed
   
   @@ -259,7 +259,7 @@ class MigrateToolsCommands extends DrushCommands {
   +  public function import($migration_names = '', array $options = ['all' => NULL, 'group' => NULL, 'tag' => NULL, 'limit' => NULL, 'idlist' => NULL, 'update' => NULL, 'force' => NULL, 'execute-dependencies' => NULL]) {
   
   @@ -268,7 +268,7 @@ class MigrateToolsCommands extends DrushCommands {
   +    foreach (['limit', 'idlist', 'update', 'force'] as $option) {
   

   Why are we removing feedback support? Oh, I see. It is because we don't need it for Batch. But we do need it for non-batch i.e. in Drush entirely. I don't know I like doing it that way. Or at least we need it until https://github.com/drush-ops/drush/pull/1484 lands. So, we need to find another way to work around this until then.

2. +++ b/src/MigrateBatchExecutable.php
   @@ -103,6 +103,7 @@ class MigrateBatchExecutable extends MigrateExecutable {
   +        'file' => drupal_get_path('module', 'migrate_tools') . '/src/MigrateBatchExecutable.php',
   

   What is this needed for? Seems odd that autoloading doesn't working here.

3. +++ b/src/MigrateBatchExecutable.php
   @@ -192,7 +193,7 @@ class MigrateBatchExecutable extends MigrateExecutable {
   +        '@name' => $migration->id()
   
   @@ -212,7 +213,7 @@ class MigrateBatchExecutable extends MigrateExecutable {
   +      '@name' => $migration->id()
   

   Trailing comma removed?

I have the same issue as in #18 with Drush 9.4.0

I also found some related issues here https://www.drupal.org/node/2664290 with patch https://www.drupal.org/files/issues/2664290-17.patch and here https://github.com/drush-ops/drush/issues/1930 . Maybe removing typehint will fix the problem? But in any case I don't understand how it will work later if it was object and suddenly on the next line of code it is supposed to be array. Very strange.

I confirm, that removing typehint fixes the problem for me.

re #23: can you post a patch with just that change?

Here is the attached file without typehint, it is only applying to 8.x-4.0, didn't have time to change it to dev branch. Also the patch contains fixes for #20

No interdiff? It is really hard to see what was changed without that, especially since this won't apply against HEAD. Was #20.2 addressed, what about 20.1 and 20.3?

Re-rolled patch against HEAD with all remarks fixed from #20 and no typehint.

I'd really like to see a test on this, but let's see how the existing tests work with the changes so far.

This patch applied however I was getting a typehint error again with the setBatchContext function.

/**
   * Sets the current batch content so listeners can update the messages.
   *
   * @param array $context
   *   The batch context.
   */
  public function setBatchContext(array &$context) {
    $this->batchContext = &$context;
  }

I've rerolled the patch with that minor change.

FYI, Drush has had some non-functional re-spawn functionality of its own. Thats about to be removed in https://github.com/drush-ops/drush/pull/3909. So I'm happy to see this issue moving along, as more people will need the memory exhaustion handling of migrateExecutable.

We've gone all over the place. There was a patch at 9k in #25, then one that was 7K in #27 and #30. Now we have #32 that jumps up to 32k and no interdiff. What is being done?

Also NW for phpcs.

I forgot to link to https://www.drupal.org/documentation/git/interdiff in #33.

At this point, the last few patches are not reviewable, without some descriptions and hopefully an interdiff. There's a lot of stray changes in that last patch. Do we need to backup to an earlier patch to get what we are really striving for?

Here is a patch with the same changes in #30, but refactored to apply without errors to the latest dev release. This was tested for one migration and fixes the issue.

Looking at the patch from #36 (previously #30) it looks like it is doing much more than solving the problem at hand: running out of memory due as articulated in #11. So, I'm skipping using that patch.

Looking at the patch from #12, it looks very light but doesn't apply against the latest 8.x-4.x branch. So, I'm attaching a re-roll of that patch here. I did notice that the patch made two code style changes to unrelated files which I've dropped (I didn't create an interdiff because the interdiff process didn't work for me). I have not tested the attached patch.

This issue seems to be experienced by several people at this point. It would be excellent to stabilize a patch that focuses solely on the issue from #11.

The patch from #37 does not work. It errors with TypeError: Argument 3 passed to Drupal\migrate_tools\MigrateBatchExecutable::batchProcessImport() must be of the type array, object given. So, I'm removing that patch from the displayed list.

Important find is that using the latest 8.x-4.x branch the memory issue does not occur! Given that, I believe this issue can be closed as fixed, but I don't want to be too hasty. Can someone else double check and confirm that the 8.x-4.x branch resolves the issue described in this issue, see comment #11 for details?

P.S. Shout out to @qymanab for discovering that the latest branch resolves the issue.

Hello,

Currently when running a migration, Drush 9, I get the error in the issue summary.

The process stops and no batch is triggered.

Opposite to message from comment 38, Using the last dev version of migrate_tools I still have the problem.

And I have the same problem as comment 38, If I apply patch from comment 37.

I confirm that issue occur with latest dev.

The stronger typing is now gone for batch. See #3090025: Remove array type hints for batch context. I think that makes it easier to reroll this and (hopefully) resolve this issue. And we need tests. There new/easier to use tests that recently landed in https://git.drupalcode.org/project/migrate_tools/blob/8.x-4.x/tests/src/... and https://git.drupalcode.org/project/migrate_tools/blob/8.x-4.x/tests/src/.... So let's use those as models and add some tests here.

I re-rolled the patch from #36 and removed the re-initialization of $context['sandbox'] in MigrateBatchExecutable since the newly created array would be ignored by the magic DrushBatchContext wrapper.

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

src/MigrateExecutable.php ✗ 1 more
line 247	Parameter comment must end with a full stop

Firstly, thanks for all the hard work on this! This is becoming a fairly large issue with one of my sites that has some recurring migrations.

I have been testing some local re-rolls of #37 (the patch with minimal updates) as well as #42 (larger patch with changes to logging).

Both seem to work well though I think some more discussion should be had around the logging and batch processing.

Before commenting with yet more patches I have two questions:

1) Is it possible to use the Symfony progress bar that was recently added? This may be more work that it's worth in which case should there be an option to silence the log output when a new batch starts? I'm asking because for large migrations the logs could get noisy.

2) Should the batch processing be behind a cli option such as --batch? I'm mostly considering this because it is nice to have the Symfony progress bar for other migrations that don't run into memory limits.

@jebschiefer, I think it makes sense to put the batching behind a cli option.

Beyond that, the sooner we get this merged the better as far as I'm concerned and I'm happy to test whatever you have. I have a migration of ~225k records from an external API that migrate keeps choking on.

Here is a re-roll of #42 for 5.x.

I will work on uploading an updated patch + interdiff that has batching behind a cli option.

This is #46 with batch processing behind the --batch option.

Updated tests.

It looks like this ticket is explicitly focusing on memory exhaustion via drush, but there is a use case for handling memory exhaustion when not running a migration via CLI: a scheduled job or queue worker. I attempted to use MigrateBatchExecutable, but that didn't work for my use case because I am modifying the migration config entity during runtime and that executable class doesn't preserve those changes on subsequent batch runs. It would be amazing if the solution here would also be able to cover that use-case.

As a work-around, I used the Drupal\migrate_tools\MigrateExecutable, which has a "limit" flag, and then modifying migrate_source_csv to have an offset flag (see Issue #15). I used key-value store to keep track of what the offset was between batches. And, then use a Queue with a RequeueException if the migration is incomplete.

Sort of related, it's worth acknowledging that memory exhaustion and the ability to ensure a migration completes in that scenario is being solved by several in their own way (including me, as mentioned above). Here are two posts with unique solutions:

• Running Large Drupal 8 CSV Migrations in Batches (Palantir)
• Memory Management with Migrations in Drupal 8 (Mediacurrent)

I would love to see this issue resolved, and want to help! I took a quick look at patch from #48. I'll try to make time to test it.

The great gitlab migration is upon us. Please visit https://gitlab.com/drupalspoons/migrate_tools/-/issues/92 and provide your comments there instead.

Reopening as migrate_tools is using D.O to track issues again.

In my use case there are thousands of XML files to be imported. This would soon result in an out-of-memory error without batch processing.
In addition to this issue I had to extend the source plugin from Migrate Plus to be batch-aware. So instead of reading the same files again and again in each run, the offset is stored to the batches sandbox and restored in the next run.

The patch from #48 is still working fine. Let's get this merged!

This seems to be doing very similar things to what is going on in #3067165: Option to override batch size. Shall we combine forces and focus on that one? As I think it provides greater flexibility and features.

There's definitely work from this issue that would need adding into the #3067165: Option to override batch size, if the efforts were to be combined. This issue allows imports through drush to use batching at all, whereas the other issue is simply about controlling the size of each batch. Which is no good at all for migrations that run out of memory, if batching isn't used in the first place!

#3067165-19: Option to override batch size brings the work from this ticket over to that other one, which might be the best place to move forward here. If someone agrees, I suggest then closing this ticket as a duplicate.

I agree with #55, the scope of this has been fully integrated in #3067165: Option to override batch size and it has a working patch, while this one is stale. Closing as duplicate.