On a fresh (or fresh enough) install, when the releases have not been synchronized yet.
Visit /admin/config/l10n-server/connectors
For "Drupal.org packages from Rest API", choose "Scan" from "Operations" column.
In the confirm form, click "Confirm".
Expected:
A batch starts.
Every batch iteration only takes a limited amount of time.
When I close the browser tab, the current iteration will finish and then it will stop.
Actual:
A batch starts.
The batch takes forever for the first iteration.
When I close the browser tab, the process continues in the background.
(only ddev stop)
In mysql, "show full processlist" shows repeated queries of this format:
SELECT "base_table"."rid" AS "rid", "base_table"."rid" AS "base_table_rid"
FROM
"l10n_server_release" "base_table"
INNER JOIN "l10n_server_release" "l10n_server_release" ON "l10n_server_release"."rid" = "base_table"."rid"
WHERE "l10n_server_release"."download_link" LIKE 'https://ftp.drupal.org/files/projects/schemadotorg\\_starterkit\\_medical-1.0.0-alpha35.tar.gz' ESCAPE '\\'
It seems this happens in ScannerService::storeReleaseList().
foreach ($this->releases as $release) {
$download_link = "https://ftp.drupal.org/files/projects/{$release['machine_name']}-{$release['version']}.tar.gz";
if ($release_storage->getQuery()->accessCheck(TRUE)->condition('download_link', $download_link)->execute()) {
The number of releases is 205061, they are coming from drupal.org.
(This is on first run, when the 'l10n_drupal_rest.last_sync_time' state value has not been written yet. Subsequent runs can have a lot fewer releases to handle, but I am not sure.)
Interestingly, other methods like parseProjectList() deal with the same number of releases there are other loops that iterate over all these releases and complete much faster. It must be the sql query.
Either we break this loop into batch chunks, or we optimize that query.
Show commandsStart 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:
changes
Comments
Comment #2
donquixote commentedProblem 1:
The 'download_link' column is not indexed.
-> I am trying to have it indexed, by modifying L10nServerReleaseStorageSchema.
Problem 2:
Trying to have it indexed, I find the next problem:
The L10nServerReleaseStorageSchema class is not used, because it is misspelled in the L10nServerRelease entity definition.
-> I fix the entity definition.
With both of these changes, now the process is significantly faster.
Before: ~1 minute per 1000 releases.
After: ~2.5 seconds per 1000 releases.
(I will have to repeat this measurement, there may be some mistake)
The batch in the browser hits a time limit at ~130000 of the total 205061 releases.
But the process continues in the background.
Still this is not an ideal situation.
Comment #3
donquixote commentedAnother smart idea would be to process releases in reverse order (oldest first), stop after a given time limit, and go to the next batch iteration.
We can write the 'l10n_drupal_rest.last_sync_time' state value and in next iteration we can pick up there.
One question would be whether to download and parse the releases.tsv in every iteration, or keep it in the tmp dir.
It seems a good idea to keep the csv between batch iterations and delete it after the final iteration, not sure what could go wrong with this.
Comment #4
donquixote commentedAlternatively we could say that the initial scan should be done with drush, not the UI.
Comment #6
donquixote commentedThe MR fixes the indexing problem, but does not go further.
Setting to "Needs review", even though I don't think this is a complete solution.
Comment #7
donquixote commentedComment #8
donquixote commentedComment #13
fmb commentedI have just merged the indexing part for the time being.
Comment #15
donquixote commentedLet's do this first: #3592848: ScannerService imports date with wrong timezone
Comment #16
donquixote commentedComment #17
pierregermain commentedLeft Code Review on the MR.
Comment #18
drummIn production, we will always do this sort of batching via drush cron or a command. I do not think the web UI should be expected to work since we will never use it. It is fine for drush commands to be slow, although there should be some ability to limit so testing & development is possible & convenient.
Comment #19
donquixote commentedOne thing I am trying to do in this issue is to make the actual service and plugin support incremental operation as part of the interface design.
We can then apply this in the drush command or wherever we need it.
I think the current version of the MR does not do anything for drush, I could change that.
Here, progressive operation actually means multiple things:
- Ability to display progress information during the operation.
- Ability to resume an operation that was interrupted.
- Ability to resume an older operation, even when using a new version of the releases.tsv file, assuming that only new records were added, no old records were changed or removed.
To do this, we start with the oldest record and work our way to newer records from the source tsv.
Comment #20
pierregermain commentedWe change status to Reviewed and tested by the community.
Comment #21
gábor hojtsyRebased the MR onto 3.0.x (two small conflicts with the config schema store from #3621250, resolved) and then reduced it, following drumm's guidance in #16 and a look at how the Drupal 7 site does this.
Production runs the scan from drush or cron, and the initial fill of the site comes from the migration, not from a scan of the full release list. After cutover a scan reads about a day of rows. Development has the short
assets/releases.tsvfixture the site README points the refresh URL at. That leaves no real use for the row limit and resume state, so those are dropped again together with the reworked batch form and its test: the scan interface staysscanHandler()without arguments, as cron and the drush command call it.Kept from the MR: the refactored scanner service with autowiring, the logger channel, the per run project cache, the slimmer result object, replacing the temporary file instead of renaming it, and the header check, which now fails loudly if drupal.org ever changes the column layout. Changed against the MR: malformed rows are skipped with a warning instead of aborting the whole scan, like Drupal 7 skips short rows (extra columns are ignored, a bad date is skipped too, an out of order row is logged), the last sync time stays the timestamp state key with the Drupal 7 rule (the newest release stored), so seeding it from the Drupal 7 variable at cutover is a plain copy, and the download gets the five minute timeout Drupal 7 had. The temporary file is unmanaged now, as it is deleted right after reading.
Validated the strict header assumption against today's live releases.tsv: 218,430 rows, all four columns, all dates well formed, strictly newest first.
Tests: Drupal 7
testRefreshSkipsMalformedRows()first (short row and sandbox skipped, extra columns ignored, last sync time, nothing stored twice), the porttestScanHandlerSkipsMalformedRows()in the connector kernel test. The long links test from #3621243 follows the new log order.One thing to note for the cutover, outside this issue: the scan recognises existing releases by the exact download link, Drupal 7 rows carry
http://links and new ones gethttps://, so the day of overlap at cutover would create duplicates. Filing that separately.LLM was used to find, diagnose explain and fix this issue. With human review.
Comment #23
gábor hojtsyRetitle for more general scope as it does not contain batching anymore based on @drumm's on point feedback :) The rest of the improvements were kept :)
Comment #27
gábor hojtsy#3621483: Scan creates duplicate releases for the ones migrated from Drupal 7 is the followup about how projects are matched prior/post migration.
Comment #28
donquixote commented#23 - Title looked suspicious to me :)
(I will have another look at recently updated tickets soon.)