search

ubc / iPeer

Peer Evaluation System
http://ipeer.ctlt.ubc.ca
Other
17 stars 18 forks source link

Clear cache on upgrade #578

Closed wynnset closed 6 years ago

wynnset commented 6 years ago

Add missing line to clear default cache in addition to the custom "Configuration" cache

Fixes #577

wynnset commented 6 years ago

I discovered that we have 2 cache set up, according to config/core.php. One is the default one, and one is the 'configuration' one (a custom cache we had added "for configuration/sys parameters"). Only the "configuration" cache was being cleared, leaving the "default" cache uncleared.

I've now added a line to clear the default cache as well when upgrading, so I think this should do the trick.I have not tested it on verf. We will need to test this somehow on verf and make sure it will work.

Let me know if you have any suggestions for testing. I think perhaps we can add a bogus column to one of the existing tables in one upgrade. We can do this on a branch perhaps and push that branch to verf, then test to see if it works (i.e. if, when querying the model, the bogus column is returned)

kitsook commented 6 years ago

The caching issue we encountered was on the DB models stored under app/tmp/cache/models. I believe it is different from the caching provided by Cache. Tried to invoke Cache::clear explicitly and the model files are not cleared / updated.

The model cache update is handled by CakePHP. It can be simulated in dev by turning on/off debug mode (set IPEER_DEBUG equal to 0/1/2 in docker-compose.yml). If debug mode is on, CakePHP checks for DB changes frequently (e.g. during page load) and the model files under app/tmp/cache/models will be updated.

The Cache library is more on the application caching. Grepping the code, seems that only the system parameter module used the configuration cache. The default cache isn't used at all.

kitsook commented 6 years ago

I think what we need is to call clearCache for models.

wynnset commented 6 years ago

Good find. I've added clearCache() to be run after the upgrade. Seems like we have to explicitly set the directory to clear, so I've set it to clear the following directories under app/tmp/cache:

I think it's OK to clear all of the above when doing an upgrade.

I've also tested this works on dev by turning off caching, checking that the cache files exist when doing a page load, then running the upgrade, and seeing all the files are deleted.

Minor note: Changing IPEER_DEBUG did not do anything for me, even after restarting the container. I also tried turning debug off in config/core.php and that also did not work, It kept refreshing the cache on reload. Hence I had to just turn off caching altogether, which worked.

wynnset commented 6 years ago

@xcompass maybe we could include this in the release and see if the cache gets cleared (we can manually clear if it doesn't, but our tests showed that it'll work).