[META] Update doc comment on BatchProcessor to specify why use the batch system

High-level observation

🤔 Is it really safe to do the whole stage life cycle in a single request during cron updates, but using multiple requests (because batch) while using the UI?

Crash in real world

Thanks for creating this issue, @tedbow. Earlier today, while pairing with @Omkar.Podey, I actually SAW IT CRASH MID-REQUEST, and then it was impossible to delete the stage using the UI, because it was presumed to still be applying! (see \Drupal\package_manager\StageBase::isApplying()) 😨

(To be fair, XDebug was on, but even without XDebug, we have to assume at least the network could be slow. Fetching a Drupal core update is multiple megabytes.)

PHP's max_execution_time and set_time_limit()

PHP's max_execution_time defaults to 30s, which is really risky/close. 😱 Even for batch processing this is risky; for this very reason e.g. the migration system has special measures in place: \Drupal\migrate_drupal_ui\Batch\MigrateUpgradeImportBatch::run() — introduced by #2281691: User interface for migration-based upgrades.

The issue summary says:

Cron sets a long time limit to be able to do more operations. We could do this in the UI but then user has no feedback for the entire cycle.

But I was unable to find it doing anything relating to max_execution_time. I dug deeper. Apparently \Drupal\Core\Cron::run calls \Drupal\Component\Utility\Environment::setTimeLimit() with $time_limit = 240, which in turn calls PHP's set_time_limit() (so: 4 minutes instead of the default of 0.5).

TIL!

php-tuf/composer-stager

And not only Drupal core's Cron class uses it, so do \PhpTuf\ComposerStager\Infrastructure\Service\FileSyncer\PhpFileSyncer::sync() and \PhpTuf\ComposerStager\Infrastructure\Service\FileSyncer\RsyncFileSyncer::sync().

Limitations of set_time_limit()

But …

1. set_time_limit() may be disabled by the site administrator — see the docblock on \Drupal\Component\Utility\Environment::setTimeLimit()
2. even if it is available, it can only be used to extend the time limit so much: mod_fastcgi, mod_fcgid, php-fpm etc. all have their own time limits too!

Conclusion

So I think we need:

1. a new validator that verifies that set_time_limit() is available
2. a mechanism that ensures that the stage life cycle is executed across multiple cron requests — we should probably look at examples in core like locale_cron() which only calls locale_cron_fill_queue() and then has a corrresponding @QueueWorker plugin: \Drupal\locale\Plugin\QueueWorker\LocaleTranslation

More investigation needed. But given how this will very likely result in problems on real-world test sites, bumping to Critical.

In addition to the 2 issues above:

3. Get a mechanism in place to ensure a FailureMarker gets created if the PHP execution time is exceeded — without that, there's just a silent failure which is nearly impossible to debug. That's what @Omkar.Podey ran into over at #3355446: [PP-1] Implement experimental cron updates for contrib in automatic_updates_extensions.

for reference to the

impossible to delete stage

in #2

Issue for the third follow-up: #3357941: Write helpful message to FailureMarker if script execution time was exceeded during stage life cycle.

I think we should convert this to a meta, since it's not clear how to document all this until the follow-ups are done? 🤔 Tentatively retitling to [META] and recategorizing as Task.