Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce cache bypasses - do not bypass custom metadata cache in setGroupTree #14292

Merged
merged 1 commit into from
May 25, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

When editing a form with custom data on it the cached metadata is ignored &I rebuilt. This changes it to use it

Before

More DB queries

After

Less DB queries

Technical Details

We have been running this patch in production for some months. When you load up a contribution edit page
(for example) the custom data metadata is loaded without hitting the cache. This means that
unnecessary queries are done. The only reason to bypass the cache would be that it is stale
or parameter dependent. However, the staleness is/ should be dealt with when editing custom
fields & if it is not then there will be other impacts. The entity id is NOT passed into the part of
the code that goes into the cached value

In order to demonstrate the params that go into the cache are in the cachekey I also did an extraction

Comments

@civibot
Copy link

civibot bot commented May 22, 2019

(Standard links)

We have been running this patch in production for some months. When you load up a contribution edit page
(for example) the custom data metadata is loaded without hitting the cache. This means that
unnecessary queries are done. The only reason to bypass the cache would be that it is stale
or parameter dependent. However, the staleness is/ should be dealt with when editing custom
fields & if it is not then there will be other impacts. The entity id is NOT passed into the part of
the code that goes into the cached value
@eileenmcnaughton
Copy link
Contributor Author

I've added 'ok without test' as I could see lots of tests error when I had an enotice in this! - also, it's just caching

@eileenmcnaughton
Copy link
Contributor Author

test this please

2 similar comments
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

test this please

@mattwire mattwire merged commit b1cee37 into civicrm:master May 25, 2019
@eileenmcnaughton eileenmcnaughton deleted the cachedtree branch May 25, 2019 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants