-
Notifications
You must be signed in to change notification settings - Fork 77
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
[Feature Req] Cache settings #90
Comments
Hello there! Thanks for opening your first issue on this repo! Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that. Backpack communication mediums:
Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch. Thank you! -- |
Hmm you're right @rk . Thank you for the suggestion. Since settings are changed so rarely, it makes a lot of sense for them to get cached. We just need to make sure that once a Setting is Updated, we wipe the cache - so that changing settings would still have immediate effect. I see there's already a PR for this here #91 Cheers! |
It does need the logic for clearing once a setting is updated, but it looks appropriate. 👍 It caches both the table check and the resulting fields separately, which is good. The event is really simple to add though: Setting::saved(function () {
Cache::forget('backpack_settings_cache');
}); |
Fixed by #91 . I'll take another look at it before we launch Backpack 4.1 (this or next week), merge it and tag it as a new version. Let's move the conversation there please. |
I'd like to provide some feedback on this. I have a project where I have to cache one route because of the high traffic it receives. The route was cached via redis, so it shouldn't touch the database at all. However, this package still made queries to database, bringing performance down significantly. Here's some ideas on how to get cache config and use it: |
In my situatuion we've moved DB server to remote host and now for each http request it produces 2 requests to DB. Even if this pages uses none of settings. We have a lot of settings and all data generates certain amount of traffic. Wich is also paid. Another suggestion, when loading settings in ServiceProvider: Setting::all(); Just take key and value - for most cases it covers 99% of usage. |
So, the current implementation of the
SettingsServiceProvider::boot()
method prevents caching of the settings, forcing 2 queries (one to detect the table, one to load settings) on the DB. It's more efficient for larger sites to cache all settings in memcached or redis, instead of querying the DB on every page hit.I looked into extending the provider, but that might be a bit messy.
If you define a static method for it on the
Setting
model, then using a config value to point at the FQCN. We could then override the Setting class and extend it to allow caching.Just some food for thought.
The text was updated successfully, but these errors were encountered: