Problem/Motivation

In #2828620: Regression: View pane rendering misuses and floods cache the commit from #1910608-36: Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager) will be / was reverted.

That commit used the general cache as temporary storage for a pane $conf, which is quite huge.

The original author used the cache only as a try, stating in #1910608-4: Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager):

Use of cache is because
1) didn't want to try and figure out where that view was coming from and loading that
2) didn't want to pass around the settings in a way a user could manipulate

Proposed resolution

* Figure out how to get the needed data in an ajax call
* Use another temporary data storage, like the session
(Remember that using cache for non-cache data is a problem for ram-based caches which are of limited size)

Remaining tasks

TBD.

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

axel.rutz created an issue. See original summary.

DamienMcKenna’s picture

Issue tags: +Needs tests

I would strongly recommend that any possible solution include tests to ensure there are no further regressions from this.

japerry’s picture

I guess my only concerns with this particular test is how to verify the cache table isn't going crazy?

I've reverted the original issue, and here is the original patch as well as a suggested fix (#57) from #2828620: Regression: View pane rendering misuses and floods cache.

We need to figure out this patch before the next release of ctools.

The last submitted patch, 3: 1910608-ajax-settings-lost-reverted-commit.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: 2843333-ajax-settings-pane-rendering-fix.patch, failed testing.

Jelle_S’s picture

To give you some feedback, I wrote patch #57 in #2828620-57: Regression: View pane rendering misuses and floods cache. We've been using this patch on a production environment where the cache table at one point had grown to ~2GB (very large site). As of right now, that same cache table is 21.4MB and the cache_panel_conf table is just 784KB.

japerry’s picture

Whelp its been 6 months and no work has happened on this patch. We'll release 1.13, and hopefully this issue can get some love before the 1.14 bugfix release.

DamienMcKenna’s picture

This didn't get into 1.13, maybe it will get into 1.14.

We need to try getting this to RTBC.

mpotter’s picture

Status: Needs work » Needs review
FileSize
3.24 KB

Here is an updated patch for ctools 1.14. It builds on #56 from #1910608: Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager). Needed updates based on changes from #2941920: Update 7002 Specified key was too long and new update hook number.

Status: Needs review » Needs work

The last submitted patch, 9: 2843333-ajax-settings-pane-rendering-fix-9.patch, failed testing. View results

mpotter’s picture

Not sure how to deal with failing tests since this patch is dependent on another. Maybe one of the maintainers can apply all these related patches together somehow to run tests?

Jorrit’s picture

Maybe have some kind of integration issue that contains a combined patch which is not supposed to be merged?

joelpittet’s picture

@mpotter, #9 is failing to apply, maybe you made it against something that is not the dev branch?

joelstein’s picture

Status: Needs work » Reviewed & tested by the community

The patch in #9 applies cleanly (after #1910608-56: Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager) is applied) and works great. :-)

joelstein’s picture

Status: Reviewed & tested by the community » Needs work

One scenario that won't work with this patch is if you embed the same content pane twice on a page, for example, if you have a latest blog posts widget that you've configured to appear with different categories. Before this patch is applied, I could do that. After, the multiple content panes with different configuration all have the same cache ID.

How can we resolve that?

joelstein’s picture

In #1910608: Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager), the cache ID is determined by the DOM ID, which was as hash of the view name, request time, and random number. This resulted in a new cache entry on every page load.

In the patch from #9, this was changed to determine the cache ID by several pieces of fixed information, which produces a single cache entry per content pane, but it did not allow for the pane configuration which should result in new cache entries if embedded multiple times with multiple configurations.

Here's an approach that simplifies the approach by setting the DOM ID to a hash of the view name and serialized conf array. This patch allows me to embed the same pane multiple times with different configuration, which is the best of both worlds above.

Status: Needs review » Needs work
DamienMcKenna’s picture

ciss’s picture

@joelstein Did you encounter any problems with your patch in #16 in the 1 1/2 years since it got posted?