Problem/Motivation
When you collect a large number of registrations (in the case of our website it's around 5000 registrants) and you manually add an instance to a Series, calculating the value of the computed field availability_count will cause your database server to go down.
The thing is:
If you go to a Series and use the "Add instance" option, after submitting the form you will get this:
SQLSTATE[HY000]: General error: 2006 MySQL server has gone away: INSERT INTO "cache_entity" ("cid", "expire", "created", "tags", "checksum", "data", "serialized") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3....
This is because when making the POST request, the computation of the availability_count takes place, invoking the method Drupal\recurring_events_registration\Plugin\ComputedField\AvailabilityCount::computeValue().
That ends up using the Drupal\recurring_events_registration\RegistrationCreationService service to call retrieveAvailability(), which in turn calls retrieveRegisteredParties(TRUE, FALSE) that uses loadByProperties(). And here is where disaster comes:
Update: Along with the availability_count, there are a couple more computed fields that are also calculated when submitting the "Add instance" form: registration_count and waitlist_count, which, in turn, also call retrieveRegisteredParties(), generating more calls to loadByProperties() and making the process even heavier.
Steps to reproduce
- Have a large number of registrants, say 10000
- Go to a Series and use the "Add instance" option
- Try to save the instance
Proposed resolution
This gives me various considerations:
retrieveAvailability() must not rely on retrieveRegisteredParties() and a full load of the registrant entities just to get a count of the registrants. It should use another method based on Entity Query and the count() method. Something like the following (not tested yet):
public function retrieveRegisteredPartiesCount($include_nonwaitlisted = TRUE, $include_waitlisted = TRUE, $uid = FALSE) {
$query = $this->storage->getQuery();
if ($include_nonwaitlisted && !$include_waitlisted) {
$query->condition('waitlist', 0);
}
elseif (!$include_nonwaitlisted && $include_waitlisted) {
$query->condition('waitlist', 1);
}
if (!$include_waitlisted) {
$query->condition('waitlist', 0);
}
if ($uid) {
$query->condition('user_id', $uid);
}
switch ($this->getRegistrationType()) {
case 'series':
if (!empty($this->eventSeries->id())) {
$query->condition('eventseries_id', $this->eventSeries->id());
}
break;
case 'instance':
if (!empty($this->eventInstance->id())) {
$query->condition('eventinstance_id', $this->eventInstance->id());
}
break;
}
$result = $query->count()->execute();
return $result;
}
- Computed fields should not be calculated during the POST request when creating or editing an entity. I faced something similar in another project. The resulting values of computed fields are actually useful when getting/viewing/reading the entities, but in the POST request generated by a form submission and before the entity is saved, it does not make too much sense to me. I made an adjustment to various computed fields in another project: I verified the method of the request, and it case it is POST, I assigned an empty value to the computed field and do not perform the computation:
/**
* {@inheritdoc}
*/
protected function computeValue() {
// When saving or editing an entity, we are not interested in calculating
// the values for its computed fields. The resulting values of these
// computed fields are actually useful when getting/viewing/reading the
// entities, for example, when retrieving an entity data from a GET request
// to a REST or JSON:API endpoint.
// If the request has the 'POST' method, assign an empty value to the
// computed field and return.
if ($this->requestStack->getCurrentRequest()->getMethod() == 'POST') {
$this->list[0] = $this->createItem(0, NULL);
return;
}
/*
* The ComputedItemListTrait only calls this once on the same instance; from
* then on, the value is automatically cached in $this->items, for use by
* methods like getValue().
*/
if (!isset($this->list[0])) {
$entity = $this->getEntity();
$this->list[0] = $this->createItem(0, $this->getRegistrationCreationService($entity)->retrieveAvailability());
}
}
- Also, notice how the
$this->list[0] is defined just once. Read the comment about ComputedItemListTrait only calling this once
Remaining tasks
Check the changes proposed above and create a patch (done)