Recently we had an issue with the rules cache. Our case was that sometimes, after clicking on "Add to cart" button (we are using the Drupal Commerce module) the user was not redirected to the checkout page. This happened because, for some reason, the entry in the cache_rules table for "event_commerce_cart_product_add" was missing, although all the other entries were there. And the thing is that the "rules_get_cache" function does not handle this case, so in our situation it returned always FALSE and we always had to clear the whole rules cache in order to be rebuilt.
To reproduce it, you can just manually delete the entry from the cache_rules table, and then try to invoke that rules event.

CommentFileSizeAuthor
#3 rules_cache-1425652-3.patch887 bytesvasi1186
#1 rules_cache-1425652-1.patch890 bytesvasi1186

Comments

vasi1186’s picture

vasi1186 commented
Version: 7.x-2.0 » 7.x-2.x-dev
Component: Rules Core » Rules Engine
Status: Active » Needs review
StatusFileSize
new rules_cache-1425652-1.patch890 bytes

I attached a patch that should solve this issue.

Status: Needs review » Needs work

The last submitted patch, rules_cache-1425652-1.patch, failed testing.

vasi1186’s picture

vasi1186 commented
Status: Needs work » Needs review
StatusFileSize
new rules_cache-1425652-3.patch887 bytes

sorry, syntax error in the first patch...

Status: Needs review » Needs work

The last submitted patch, rules_cache-1425652-3.patch, failed testing.

vasi1186’s picture

vasi1186 commented
Status: Needs work » Needs review

#3: rules_cache-1425652-3.patch queued for re-testing.

fago’s picture

fago
Primary language German
Location Vienna
commented

Indeed, we should not rely upon cache entries to exist once the data-cache exists.

To fix that I think we have to overhaul quite some of the cache clearing rebuilding though, I fear. Rebuilding just the whole cache on each cache miss and/or potential wrong calls does not look good to me.

Thus, I guess we should
* move component and event-set cache build logic out of rebuildCache(), which really should be used for building the 'data' cache only.
* implement per component / event-set cache build logic
* build this cache on demand

That way, we can save some time during the regular cache rebuild and do more fine-granular cache clears when changes are made.

In addition to that, I think we should introduce a lock which is active during the 'data' cache rebuild and require the data cache when building the compont/event-set cache - so we can be ensure no invalid caches are written due to concurrent requests.

Status: Needs review » Needs work

The last submitted patch, rules_cache-1425652-3.patch, failed testing.

fago’s picture

fago
Primary language German
Location Vienna
commented

Also see the related issue #1106508: Error with components after changing site structure: "Unable to get the component ..."

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs work » Fixed

I've implemented the improvements as outlined in #6 and committed them.

See http://drupalcode.org/project/rules.git/commit/d0a86e0

    Issue #1106508 and #1425652: Improved cache getting and rebuilding:
    
      * Split up cache rebuilding into main cache, component and event-set caches.
      * Improved getting component and event-set caches to trigger cache-rebuilds on cache fails.
      * Improved cache clearing when updating configurations to only clear necessary caches.
      * Improved strategy for building cached event-sets to reduce the number of necessary DB queries.

What I've not implemented are cache-rebuild routines for single event-sets or components - thus cache is always rebuilt for all event-sets or components. Usually we won't need single rebuilds anyways , however they would be fine for speeding up UI changes by reducing the number of necessary rebuilds. The committed changes already greatly reduce the necessary cache rebuilds when working in the UI anyways, so I'm not sure we need that single-rebuild improvement. If we want to, we can still add it as follow-up.

Status: Fixed » Closed (fixed)

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