Problem/Motivation

SourcePluginBase supports a configuration option 'cache_key', the key to be used for caching source counts. This option is currently not documented.

Proposed resolution

Document the key alongside 'cache_counts'.

Remaining tasks

Do it.

User interface changes

None.

API changes

None.

Data model changes

None.

Original problem statement

see MigrationListBuilder.php
This part:
$row['total'] = $source_plugin->count();

When there is alot of data, the page will be unusable and timeout. Since I was having processing issues timing out with a large file (8,000+records), I split them out into smaller files < 1000 records each. So first off... there should be some sort of batch process involved with migrating large files but couldn't find anything in documentation around that so I assume that is functionality that doesn't exist yet. The root of this problem however is that whether i have 1 migration with 8000+ records or 8 migrations in a group of <1000 records each, that count function kills the loading of the page.

I realize stats are important and necessary but are there any ideas around somehow storing those in a table which would be set at the start or end of migration? Evaluating the total on multiple lists dynamically on this page is a huge issue.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

apmsooner created an issue. See original summary.

apmsooner’s picture

I should have mentioned, I'm using the migrate_plus module xml parser.

mikeryan’s picture

Category: Bug report » Support request
Priority: Major » Normal
Status: Active » Fixed

If you set cache_counts: true in your source plugin configuration then the count will only be retrieved once (well, or if you do a cache rebuild). Or, if you set skip_count: true it won't do the counting at all. See https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Plugi....

apmsooner’s picture

FileSize
99.43 KB

Thanks. I'm able to get past the memory issues but cache_counts: true seems to have some issues with multiple migration lists within the group. It's as if all lists are using the same variable or something and the numbers are all out of whack and not making any sense at all. See attached screenshot and notice all the miscalculated numbers.

mikeryan’s picture

Title: Count function on Migrations group page causing memory issues » cache_key source plugin configuration not documented
Project: Migrate Tools » Drupal core
Version: 8.x-4.0-beta3 » 8.5.x-dev
Component: Code » migration system
Category: Support request » Bug report
Priority: Normal » Minor
Issue summary: View changes
Status: Fixed » Active
Related issues: +#2751829: Default source plugin cache key is insufficiently unique

Ah - the cache key is derived from the source plugin ID. SQL source plugin IDs are (usually) unique, but your XML source plugins are going to pretty much all have an ID of "url". There is an explicit 'cache_key' config option you can use - we should document that.

source:
  plugin: url
  cache_counts: true
  cache_key: a_string_unique_to_this_migration_like_maybe_the_migration_id

The default cache key should be more unique - oh, and looks like I previously opened an issue for that: #2751829: Default source plugin cache key is insufficiently unique.

mikeryan’s picture

Issue tags: +Novice
apmsooner’s picture

Thanks Mike, Setting the cache_key to the migration id worked out fine and now the numbers make some sense.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

etecjdo’s picture

Hey there,

I am on the Drupal Europe and will try to fix this issue.

Regards.

gnuschichten’s picture

I'm at drupal europe and I going to work on this issue.

tstoeckler’s picture

Issue tags: +DrupalEurope

Awesome @etecjdo and @gnuschichten, thanks! I am mentoring at Drupal Europe.

etecjdo’s picture

Hey there,

I have looked up the file and added a class documentation for the new cache-key.

Patch is attached.

Regards.

gnuschichten’s picture

Status: Active » Needs review
gnuschichten’s picture

We are finished here and move to the next issue.

tstoeckler’s picture

Status: Needs review » Needs work

Thanks @etecjdo and @gnuschichten!

I reviewed the patch and found one minor issue:

+++ b/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php
@@ -16,6 +16,7 @@
+ * - cache_key: (optional) This cache key is used to cache the value of cache_counts.

This should wrap at 80 characters, so cache_counts should be on the next line.

Maybe you can fix it?

Ada Hernandez’s picture

Status: Needs work » Needs review
FileSize
731 bytes
818 bytes

##15 cache_counts on the next line

quietone’s picture

Status: Needs review » Needs work

@etecjdo, Welcome to Drupal! Thank you for the patch. Good work.

+++ b/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php
@@ -16,6 +16,8 @@
+ * - cache_key: (optional) This cache key is used to cache the value of
+ *   cache_counts.

The beginning 'This cache key' seems unnecessary here, we know it is the cache_key being described. Then I saw the same structure used in the description for the high water property and thought maybe this should stay as is. But then the high water does take a bit more to explain So, let's change this.

This is supposed to be a unique key as well, maybe that should be mentioned. I don't know maybe, "A uniquely named key used to cache the value of cache_counts"? Only a suggestion.

quietone’s picture

Status: Needs work » Needs review
FileSize
806 bytes
806 bytes

Based on mikeryan's comment in #5 I came up with this documentation:

* - cache_key: (optional) Uniquely named key used to cache cache_counts.

The last submitted patch, 18: 2973713-18.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 18: 2973713-18.patch, failed testing. View results

quietone’s picture

Version: 8.6.x-dev » 8.7.x-dev
quietone’s picture

Status: Needs work » Needs review
FileSize
751 bytes

Changed to:
- cache_key: (optional) Uniquely named cache key used for cache_counts.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Yup.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Crediting @apmsooner for creating the issue, @mikeryan and @tstoeckler for issue comments, and @gnuschichten and @tstoeckler for working together at DrupalEurope on this.

Committed and pushed e1832a2c2b to 8.7.x and 9efe560f76 to 8.6.x. Thanks!

  • alexpott committed e1832a2 on 8.7.x
    Issue #2973713 by quietone, Adita, etecjdo, apmsooner, mikeryan,...

  • alexpott committed 9efe560 on 8.6.x
    Issue #2973713 by quietone, Adita, etecjdo, apmsooner, mikeryan,...

Status: Fixed » Closed (fixed)

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