ChiliProject is not maintained anymore. Please be advised that there will be no more updates.
We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.
Setting Cache is not invalidated properly (Bug #780)
With #350 a new setting caching methodology was introduced. It is now using the default cache store instead of some home-backed class-instance hack.
Unfortunately, it seems to miserably fail to detect an outdated cache. I can observe the behavior when using the default
ActiveSupport::Cache::MemoryStore as well as two un-synchronized memcached servers and
The problem is, that in the setting
chiliproject/setting-cleared_on is only set if the cache was cleared in the past. Otherwise
Time.now is assumed. (See source on GitHub) This results in the fact, that the cache is actually never ever cleared.
Furthermore, if the clear_cache was ever triggered in conjunction with memcached, the code will break, since the
ActiveSupport::Cache::MemCacheStore does not support
delete_matched (see source on APIDock).
I would like to propose an auto-expiring key, which would ensure, that outdated settings do not need to be cleared, but are automatically not used anymore. The Cache GC may then clear them, if memory cache becomes scarce.
[#780] using a changing cache key to avoid the need for explicit cache invalidation
This should work on all cache stores, since only the basic cache operations fetch and delete are used. The methods clear_cache and check_cache can no longer be supported. To sweep the whole cache Rails.cache.clear may be used. To invalidate the Settings only, you may use Setting.first.try(:touch).
The pull request is at https://github.com/chiliproject/chiliproject/pull/144.
I have tested the code manually using MemoryStore and MemCacheStore (using a single memcached and using multiple memcached). It seems to work fine. Concerning the importance of this functionality I would be happy if somebody would have a critical look on it nonetheless.
- Assignee deleted (
- Status changed from Open to Ready for review
Unfortunately the invalidation resolution is one second. I.e. multiple changes to Settings within one second might happen unnoticed. This should not matter in real world use cases, but it seems to be a pain in the test suite.
With our plugins we are running into nasty race conditions and tests that work on this machine but not on that. Adding calls to Rails.cache.clear here and there reduces the errors, but this is a solution far from perfect.
Unfortunately, timestamp columns in the DB - at least for MySQL - only store the timestamp up to the second - not milli seconds or such. Therefor I could not come up with something, that would be more accurate.
For the tests it might help, if we could disable settings caching.
Anybody how's reviewing this code, should please keep these thoughts in mind.
I merged the pull request into unstable. It will be part of 3.0.0beta2. Thanks Gregor!
I also included your code from finnlabs' dev_tools to completely disable the settings cache during tests in 637ca24
- Target version set to 3.0.0
- Assignee set to Holger Just
- Status changed from Ready for review to Closed