There is a concern that the configuration synchronisation process could potentially cause php to exceed the execution time limit or to exhaust its memory limit.
The rough outline as to how the sync worked is:
Build the storage comparer.
When accessing the page we scan through all the files in the staging directory. Files which are the same as their counterparts in the active config are ignored. Files which differ or don't have counterparts are added to a list of files to write. Files in the active config which don't have a counterpart in the stage are added to a list to delete.
The comparison of the files which are present in both staging and active is done by parsing both files into PHP and comparing the objects for equivalence. Presumably we do this, rather than comparing the files directly, because simple re-orderings of variables shouldn't count as being different.
Each file is loaded in its entirety and the process will fall over if the file size is greater than PHP's memory limit. YAML does lend itself to partial loading so this might be addressable.
Once the files have been compared the variables are destroyed and the memory is freed.
In theory the list of changes files could exceed PHPs memory limit but this seems unlikely. It is possible though that the number of files could reach the point where file IO time could cause PHP to exceed the execution time limit. Therefore this is a candidate for being batched also - we may also want to cache the result so that the user doesn't have to sit through a batch process every time they access the sync page.
Show change summary.
The storage comparer is used to build a change summary. At this point the user has to click continue to proceed.
Prioritise the change set.
Fire a prioritise event which allows modules to order the change set if necessary. For example system config tends to be handled earlier.
Validate the changes
Fire a validate event to allow modules to validate the changes.
There doesn't seem to be any easy way to batch this process or any other event so responsibility will have to devolve to the individual modules to batch potentially long running actions here. Question: can we start a batch process from an active batch process? In theory it should be fine but should check.
Attempt to lock the sync process.
This is probably the cheapest of the set up operations so should probably happen earlier.
Invoke configuration owners
Give modules the opportunity to take over the import process. We actually pass a copy of the old and new config at this step which means we're doing two more file loads here for each changed file. Again making it a good candidate for being in a batch.
Handle any other configuration changes
We load the config entity for each item in the change list and delete the entities which are not present in staging and overwrite the active configurations of the ones that are.
Presumably we load and write in this way rather than just copying the directory across as the configuration storage system is plugable and so not necessarily file based.
Again this is quite an intensive IO task and will probably need to be batched.
Fire an import event.
Notify modules that an import has happened.
- Release the lock.
- Reset the classes.
- Clear all caches.
- Delete the staged configuration files.
This commit attempts to address the import tasks which take place in steps 6 and 7.
The solution this commit implements is to use batch API to split the task over multiple page loads where necessary. Each batch task will be a single operation (create/update/delete) on a single configuration entity. (Note to self: This doesn't mean only one of these will take place per page load. Batch API will try to execute as many as possible while keeping an eye on the running time and will trigger a page load when it approaches the PHP execution time out.) This should solve the problem of the process potentially taking too long.
The downside to this approach is that configuration updates which are codependent could be split up onto separate batch page loads; in which case Drupal will load with a mixed and broken configuration on the second batch page, potentially killing the site.
It would be nicer if we could write the imported config into another store (active-pending say) which we could then switch to quickly at the very end. This would solve the problem by making the configuration change atomic and would additionally allow us to easily roll back in the case of an error. I assume the reason we didn't go for this solution is that the "active" config is special, in that Drupal and modules might have state associated with it not entirely captured by the configuration system. If it's not possible to entirely capture Drupal's state in config then we can't simple swap config states and we would need to run some other sort of import process and we would face the same problem again.
Another solution was to try and group codependent configuration changes so that they all happen at the same time. However, after a quick discussion we decided that this wasn't worth while and might not even be possible to do programatically - of course, there's nothing preventing the end user from doing this manually themselves.
Given that, it was decided that this would be the best solution with the caveat that the user be forewarned to backup and take their site offline before hand and having drush (configured to run for a longer period of time) as the preferred method.
This is a rough early attempt in order to get some feedback on the approach. Things still to address are:
- The order in which configuration is imported differs in the batch
process from the normal process.
In the normal process all configuration entities which are handled by modules will be imported first and then the remaining list will be handled by the importer.
In the batched process the entities will be imported in the order they appear in the change list regardless of the method used.
We should standardise on an order and use that for both.
- When we set up the batch job we pass the importer as an argument to the function. The object is serialised and stored in the database. The problem is that this doesn't happen across page loads so we end up getting an importer with out of date state on subsequent page loads.
I've worked around this by shifting the importer to the batch api context sandbox on the first call to the worker but this feels a little ugly. There might be a better way of handling this.
- Currently, the batch process needs to know too much about how the import works. Ideally it should just be calling a batch() function and the specifics of what's happening will be handled by the object.
- The code relies on the simplified change set from this issue:
- There are a number of serialisation bugs which prevent the importer from being serialised. These need to be fixed properly. The issues are:
When serialising the database connection the object clones itself and unsets the connection, schema and driver objects to save on space as they can be reinitiated easily. This stripped down clone object is then serialised and destroyed, and the serialised value is returned.
However, the mysql version implements a deconstructor which tidies up the sequences table and so expects there to be a connection available.
- As mentioned above when we unserialise the connection we recreate the PDO connection object. However we don't bother to re-set the custom statement class object so the statements returned aren't what Drupal expects.
- The Drupal Kernel inherits the serialize and unserialize functions of the Symfony kernel which has a different signature. This will cause an error to be thrown when we try to unserialize it.
- When serialising the database connection the object clones itself and unsets the connection, schema and driver objects to save on space as they can be reinitiated easily. This stripped down clone object is then serialised and destroyed, and the serialised value is returned.
PASSED: [[SimpleTest]]: [MySQL] 64,579 pass(es). View
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
PASSED: [[SimpleTest]]: [MySQL] 64,443 pass(es). View