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

Jump to bottom

Cache list of Tracker plugins in tracker than config file #6065

Closed
mattab opened this issue Aug 25, 2014 · 5 comments
Closed

Cache list of Tracker plugins in tracker than config file #6065

mattab opened this issue Aug 25, 2014 · 5 comments
Assignees
@tsteur
Labels
Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone
2.7.0

Comments

@mattab
Copy link
Member

mattab commented Aug 25, 2014

We hit a bug caused by the fact that Tracker Plugins are stored in the config file.

It makes sense to dynamically cache the list of tracker plugins in the tracker cache instead.

As a result, Tracking servers will pull the list of plugins dynamically rather than rely on content of config file which may be wrong (if config file was not synchronised properly). Note: loading all plugins take 50ms which is too slow, so we must only load Tracker plugins.

Tasks

  • cache the list of plugins in tracker cache
  • remove Plugins_Tracker from config file on upgrade
  • remove references to Plugins_Tracker in codebase
  • when a plugin is activated / deactivated / updated, and when core is updated , check that tracker cache is cleared.

see also #6063
@mattab mattab added this to the Piwik 2.6.0 milestone Aug 25, 2014
@tsteur
Copy link
Member

tsteur commented Aug 26, 2014

FYI I'll most likely not use tracker cache but another cache which is already loaded during tracking anyway (PersistentCache). This also brings the advantage that it is invalidated only every 12hours and not every 4 minutes as Cache should be invalidated on activate/deactivate/uninstall anyway.

The actual problem with Tracker cache is the following: Imagine we are in PluginManager and we are trying to load the tracker plugins. Therefore we are asking the tracker cache to get the general cache and if PluginsTracker is not there yet, we are adding the it to this cache. Problem: We are calling getCacheGeneral before plugins are loaded. As this method triggers an event Tracker.setTrackerCacheGeneral other 3rd party plugins won't be able to add values to the tracker.

The PersistentCache should be perfect for this and even faster as the loadTrackerPlugins is called multiple times if bulk tracking is used and the PersistentCache will not read the file from disk over and over again as a static cache (a static property) is built in. Also the interface is nicer to use

tsteur added a commit that referenced this issue Aug 26, 2014
@tsteur
refs #6065 remove tracker plugins from config and cache them instead …
d5f62db 
…in a file. Wondering which test will fail...
@mattab
Copy link
Member Author

mattab commented Aug 26, 2014

PersistentCache just makes sense, nice one

tsteur added a commit that referenced this issue Aug 26, 2014
@tsteur
refs #6065 forgot to mark method as static, did not trigger a notice …
4187a5a 
…on my PHP
@tsteur tsteur changed the title Cache list of Tracker plugins in tracker cache rather than config file Cache list of Tracker plugins in tracker than config file Aug 26, 2014
@tsteur
Copy link
Member

tsteur commented Aug 26, 2014

FYI: Each UI and API request is on my local instance about 40-80ms faster (2-10% - depends on which page/API is loaded) as we no longer have to check for tracker plugins on each request.

@tsteur tsteur mentioned this issue Aug 26, 2014
Merged
tsteur added a commit that referenced this issue Aug 26, 2014
@tsteur
refs #6065 maybe this makes sure to load plugins always in same order…
0be511f 
… in tests? As glob will return files in any order it pleases. It is only a wrapper for libc glob() function
tsteur added a commit that referenced this issue Aug 26, 2014
@tsteur
refs #6065 added changelog and fixed some tests
b8024bb
@tsteur tsteur closed this as completed Aug 26, 2014
@tsteur tsteur self-assigned this Aug 26, 2014
@tsteur
Copy link
Member

tsteur commented Aug 26, 2014

Should work. See comment in pull request re update file

@mattab
Copy link
Member Author

mattab commented Aug 26, 2014

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsteur tsteur

Labels
Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Milestone
2.7.0
Development

No branches or pull requests
2 participants
@tsteur @mattab