use cache on dynamic group #2058
Conversation
| static function getTargetsByGroup(PluginFusioninventoryDeployGroup $group, $use_cache = false) { | ||
| $ids = array(); | ||
|
|
||
| if (!$use_cache || !$ids = self::retrieveCache($group)) { |
There was a problem hiding this comment.
Not really agree with that, if you call the function with $use_cache = 'false' and you have ids in cache, it use cache.
So 2 options:
- if we not use cache, really not use it
- keep like it but remove the param
$use_cachein the function
There was a problem hiding this comment.
maybe the notation is a little too compact but here is the different cases:
- $use_cache = false, the first part of the 'if' will trigger (!$use_cache) and the second part will not (due to the '|' usage)
- $use_cache = true, the first part return false, so we try with the second part to load the cache:
- it succeeds: we skip the block
- it fails: we execute the block and load data from Search engine.
For resume, as the mentionned 'if' have an OR condition, if the first part is evaluated as true, the second part will not be evaluated.
There was a problem hiding this comment.
I agree, the second part will not be called if $use_cache is set to false.
| @@ -915,6 +915,7 @@ CREATE TABLE `glpi_plugin_fusioninventory_deploygroups_dynamicdatas` ( | |||
| `plugin_fusioninventory_deploygroups_id` int(11) NOT NULL DEFAULT '0', | |||
| `fields_array` text DEFAULT NULL, | |||
| `can_update_group` tinyint(1) NOT NULL DEFAULT '0', | |||
| `computers_id_cache` text DEFAULT NULL, | |||
There was a problem hiding this comment.
On big computers database, perhaps use longtext or blob
There was a problem hiding this comment.
OK, but, should not blob cause issues (I'm thinking about future multi-database support mainly)?
There was a problem hiding this comment.
I think about that too, but postgresql use bytea instead blob and the text is unlimited in size instead MySQL, so in all cases, will need modify it :p
I preferer longtext now
| static function getTargetsByGroup(PluginFusioninventoryDeployGroup $group, $use_cache = false) { | ||
| $ids = array(); | ||
|
|
||
| if (!$use_cache || !$ids = self::retrieveCache($group)) { |
There was a problem hiding this comment.
I agree, the second part will not be called if $use_cache is set to false.
| return $ids; | ||
| } | ||
|
|
||
| static function storeCache(PluginFusioninventoryDeployGroup $group, $ids) { |
| return $DB->query($query); | ||
| } | ||
|
|
||
| static function retrieveCache(PluginFusioninventoryDeployGroup $group) { |
|
Any news on this PR ? |
|
Ok, I'm ok with that, you can merge ;) |
|
Thank you ! |
* use cache on dynamic group * use longtext for cache * add dockblock
When demanding a b/collect/index.php?action=getJobs ,
All agent are computed again and so a dynamic group listing launch.
Depending on the criteria saved in this group, query could be very long and executed again for each agent of this group (imagine a deployment for 1000 computers querying the dynamic group each).
If i understand well, the code check if an agent always belongs to a prepared job and cancel if not.
For mass deployment, it's pretty aggressive on the server.
I suggest, in this pr, to cache computers_id of dynamic groups at the preparation of the task (use_cache = false, the default), and at the moment of execution, use this cache (use_cache = true).
Maybe, we can do better for this problem, but i don't see atm.
Also cleaned some incoherence in update.php