We have a large production site where each product node references multiple product entities. When viewing the catalog or an individual product node, the entirety of the node and its references get cached in cache_form. This includes ALL fields --description, image URLs, etc. This causes the cache_form table to grow by 1.6MB on every product page hit.

My understanding of cache_form is to cache the forms, in this instance the add to cart form should be cached, not the entire node and the referenced nodes.

I have read of many others having this problem but as of yet have seen no solution.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Category: bug » support
Status: Active » Postponed (maintainer needs more info)

Need a bit more information about your site - for example, how many forms are you showing on any given page?

The issue here is that the form cache doesn't just cache a form array but the arguments passed to build an actual form. This would include a full entity object if that made up part of the form's parameters.

jayemel’s picture

Title: Don't store the full node and product in the add to cart form state » Entire product node and product references get cached in cache_form instead of just the add to cart form
Version: 7.x-1.x-dev » 7.x-1.4
Category: task » support
Status: Needs review » Postponed (maintainer needs more info)

On the product page itself. There is exactly one form -- it is an add to cart from with one drop down containing 4 options. These options are references to product entities.

Hitting this page adds 4 (giant) records to cache_form. In their entirety, the four records store every field related to the product node, including "related products" (a custom field), description, ingredients, directions, etc., -- all custom fields. Yet none of these are in a form!

I am unclear on why items that aren't inside or related to the add to cart form are getting cached and why __all__ of the data is getting cached, such as lengthy product descriptions, etc. By design this should hold a minimal amount of data -- i.e. just what is needed to store the user's selection.

rszrama’s picture

Do the various rows in the form cache all get put in at once or only as the form is being refreshed? It sounds like Drupal may be caching the AJAX response for some reason that updates product fields that have been injected into the node display.

jayemel’s picture

Yes, they are all pushed at once, on the page hit. I verify by truncating cache form, hitting the page and then selecting from cache_form.

When choosing an option from the dropdown there is an AJAX call but no new records are added to cache_form.

Again, the issue here for me is not the number of records written to cache_form, but the sheer amount of data that is getting written. We have lengthy product descriptions an like I said above, 4 of these records totals ~1.6MB. This has caused the whole site to come down with heavy traffic.

bojanz’s picture

Title: Entire product node and product references get cached in cache_form instead of just the add to cart form » Don't store the full node and product in the add to cart form state
Version: 7.x-1.4 » 7.x-1.x-dev
Category: support » task
Status: Postponed (maintainer needs more info) » Needs review
FileSize
2.68 KB

Since the add to cart form uses #ajax, two form cache entries will be created as soon as it's displayed (one for the form, the other for the form state).

Here we have:
$form_state['context']['entity'] - the product display node, added by Field API, forwarded along by commerce_cart_field_attach_view_alter().
$form_state['line_item'] - the line item. It is small, and needed.
$form_state['default_product'] - the currently selected product.

I suggest we:
1) Stop storing $form_state['context']['entity'] and $form_state['context']['entity_type']. It is not actually used anywhere by Commerce, it is just there because commerce_cart_field_attach_view_alter() blindly starts building $cart_context by taking the core-supplied $context.
2) Optionally also replace the product in $form_state['default_product'] with just the product id, and load it manually in commerce_cart_add_to_cart_form_attributes_refresh(). This might be too big of an API change.

Attaching a patch demonstrating how that would look like.

jayemel’s picture

Title: Entire product node and product references get cached in cache_form instead of just the add to cart form » Don't store the full node and product in the add to cart form state
Version: 7.x-1.4 » 7.x-1.x-dev
Category: support » task
Status: Postponed (maintainer needs more info) » Needs review

Thank you for the patch. It reduces the storage size per row to 96k from 1.5MB.

It did produce a number of warnings.

Notice: Trying to get property of non-object in commerce_product_bundle_form_alter() (line 131 of sites/all/modules/commerce_product_bundle/commerce_product_bundle.module).

Warning: Invalid argument supplied for foreach() in commerce_product_bundle_form_alter() (line 133 of sites/all/modules/commerce_product_bundle/commerce_product_bundle.module).

Warning: Invalid argument supplied for foreach() in commerce_option_set_reference_form_alter() (line 192 of sites/all/modules/commerce_option/option_set_reference/commerce_option_set_reference.module).

Notice: Trying to get property of non-object in commerce_product_bundle_form_alter() (line 131 of sites/all/modules/commerce_product_bundle/commerce_product_bundle.module).

Warning: Invalid argument supplied for foreach() in commerce_product_bundle_form_alter() (line 133 of sites/all/modules/commerce_product_bundle/commerce_product_bundle.module).

Warning: Invalid argument supplied for foreach() in commerce_option_set_reference_form_alter() (line 192 of sites/all/modules/commerce_option/option_set_reference/commerce_option_set_reference.module).
bojanz’s picture

FileSize
733 bytes

That's because of #2.
Let's try just the change #1 then.

Let me know what the savings are with this patch.

jayemel’s picture

#7 shows a significant drop in storage, down from over 1MB to a few hundred KB on our main catalog page.

I am currently testing and will likely roll this out to production. Are there any side effects to this patch that you can think of?

Thanks again!

bojanz’s picture

No, Commerce doesn't use those at all, the fact that they are stored is just a side effect.
One-line patch of the month!

bojanz’s picture

Title: Don't store the full node and product in the add to cart form state » Don't store the full node in the add to cart form state

Retitling. This should be ready for committing.

@jmljunior
Any feedback since deploying?

rszrama’s picture

FileSize
1.12 KB

Mmm, I'm torn. I tend to agree with bojanz that the $context data isn't important to preserve; we certainly don't use it anywhere. However, I'd hate to remove things unnecessarily and leave no way for that data to be recovered by that one person in the world who may have been using the entity info in the $cart_context to alter the form. : P

My proposal: just unset the $cart_context['entity'] and add in a $cart_context['entity_id'], allowing the entity to be reloaded later if need be. This also preserves the language code that came in from that original context array.

Whaddya think?

bojanz’s picture

It is a good middle ground, I like it.

rszrama’s picture

On a quick test, it did reduce my cached form state a few KB. Obviously not as huge a performance concern for me - crazy that a node array would explode up to 1.5 MB. : P

Waiting on the word from test bot, though I'm sure it doesn't break anything.

rszrama’s picture

Status: Needs review » Fixed

Just committed this.

Status: Fixed » Closed (fixed)

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

jayemel’s picture

We have had no issues after applying the patch in #7, only benefits.

rszrama’s picture

Glad to hear it. : )

gdana’s picture

Is it possible that this fix only helps in single node pages? I have a problem similar to the one in the description but it happens only on multi product views with add to cart forms attached to each product. The records saved in the cache form table are averging 600-700 Kb, meaning a view with 30 products causes around 30 megas to be saved in the cache_form.

Would it be safe to try and add Bojan's patch on top of the solution committed?

Thanks a lot for this great module.

gdana’s picture

Sorry, duplicate

gdana’s picture

Sorry, duplicate

rszrama’s picture

Let's deal with the Views related issue separately where you've already posted it.

gdana’s picture

All right, do you mean here #2057073: Cache form table saves very large records when forms are displayed in a view? I would just like to point out that I've tried Bojan's suggestion as a temporary solution and it significantly reduces the size of the data saved with the views forms to manageable levels (although not to the expected/desired levels).

rszrama’s picture

That's expected, though you won't get any smaller than his patch affords (since his patch just removes all context information completely). The remaining size is owing to Drupal itself I'd assume.

gdana’s picture

Yes, I understand this, and I guess it's good enough but since his patch was on a dev version without your patch, and I'm actually using the latest non-dev version (updated to 7.x-1.8 hoping it would solve the problem), I'm wondering how does it work together. Should I comment out

$cart_context['entity_id'] = $entity_id;
unset($cart_context['entity']);

Or can I just ignore these assuming it just adds an entity ID (and doesn't unset something that isn't set now anyway after assigning an empty array)?

And in general, isn't it common practice to have multiple products in views? Shouldn't this be fixed? The issue is mentioned here as well: drupal commerce problem cacheform table. Obviously I'm trying to avoid changing the code in the module itself and I'm not sure what is the best practice here, with the risk of sounding a bit stupid (probably too late anyway after all the duplicates) it doesn't seem like a function I'm supposed to override in my own module. Thanks again...

bojanz’s picture

There is nothing in my patches that is useful anymore. We committed everything that was.
Everything else was an API break, meaning that it breaks contrib modules.

Yes, Drupal will cache your forms if they use AJAX. And a cache entry per product is at least a few hundred kb. That is all unavoidable.
By examining the complete cached entry, we might be able to find certain things that are not needed, and remove them.
I'm not very optimistic, since as you've seen above, bigger gains often require API breaks, and nobody has the resources to then fix all contrib modules that break.

Feel free to open a new issue, but I'm afraid you'll have to do the legwork yourself.

gdana’s picture

This is very strange, since assigning the empty array instead of the context variable reduces the amount of data very significantly, everytime. For example, in a view with 35 products, the difference can be between ~30MB (when =$context) and no more than 5-6MB (when =array()). Could it be that in views the context variable includes other information, not only of the entity, but from the view itself (e.g. the entire view structure)?

rszrama’s picture

What we're both saying is yes, the patch does this, but it breaks other things. You should not use any patch from this issue that was not committed. Please let's let this issue rest in peace and follow-up in your other issue. We'll just need a similar fix there to the one committed here. Thanks.

Marko B’s picture

Issue summary: View changes

I googled this as currently my cache_forms is constantly at around 10 GB because of all the cached infos from DC. And it would be even bigger if I didnt cut expiration timestamp cache_form_expiry to much more lower number.

Looking for some suggestions what to do with this and how to properly handle this size? Use some other storage then mysql? suggestions?

rszrama’s picture

Are you running the latest version of Commerce?

Marko B’s picture

I am using 7.x-1.10 for now. I see now some things have been done in 1.11 https://www.drupal.org/node/2332333 so I will try it. Should help to some level, right?

rszrama’s picture

Yep, that's why I asked. : D

Marko B’s picture

Thanx on suggestion. 10 GB is my current cache_form with heavy cleaning on each hour. Product display is done with panels and lots of views so I guess this is why this happens.

Marko B’s picture

Much better, from 10 GB it dropped to 1 GB.

rszrama’s picture

Gangsta. Quite the improvement!