Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

use cache on dynamic group #2058

Merged
merged 3 commits into from
May 10, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions inc/deploygroup.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,10 @@ static function showCriteria(PluginFusioninventoryDeployGroup $item, $p) {
* Get targets for the group
*
* @param integer $groups_id id of the group
* @param bool $use_cache retrieve agents from cache or not (only for dynamic groups)
* @return array list of computers
*/
static function getTargetsForGroup($groups_id) {
static function getTargetsForGroup($groups_id, $use_cache = false) {
$group = new self();
$group->getFromDB($groups_id);

Expand All @@ -426,7 +427,8 @@ static function getTargetsForGroup($groups_id) {
$results[$tmpgroup['items_id']] = $tmpgroup['items_id'];
}
} else {
$results = PluginFusioninventoryDeployGroup_Dynamicdata::getTargetsByGroup($group);
$results = PluginFusioninventoryDeployGroup_Dynamicdata::getTargetsByGroup($group,
$use_cache);
}
return $results;
}
Expand Down
71 changes: 55 additions & 16 deletions inc/deploygroup_dynamicdata.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,27 +190,66 @@ static function getDatas($itemtype, $params, array $forcedisplay=array()) {
*
* @since 0.85+1.0
*
* @param object $group PluginFusioninventoryDeployGroup instance
* @return array
* @param group the group object
* @param use_cache retrieve computers_id from cache (computers_id_cache field)
* @return an array of computer ids
*/
static function getTargetsByGroup(PluginFusioninventoryDeployGroup $group) {
$search_params = PluginFusioninventoryDeployGroup::getSearchParamsAsAnArray($group, false,true);
if (isset($search_params['metacriteria']) && empty($search_params['metacriteria'])) {
unset($search_params['metacriteria']);
}
$search_params['sort'] = '';
static function getTargetsByGroup(PluginFusioninventoryDeployGroup $group, $use_cache = false) {
$ids = array();

if (!$use_cache || !$ids = self::retrieveCache($group)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_cache in the function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the second part will not be called if $use_cache is set to false.

$search_params = PluginFusioninventoryDeployGroup::getSearchParamsAsAnArray($group, false,true);
if (isset($search_params['metacriteria']) && empty($search_params['metacriteria'])) {
unset($search_params['metacriteria']);
}

//force no sort (Search engine will sort by id) for better performance
$search_params['sort'] = '';

//Only retrieve computers IDs
$results = Search::prepareDatasForSearch('PluginFusioninventoryComputer', $search_params, array('2'));
Search::constructSQL($results);
$results['sql']['search'] = str_replace("`mainitemtype` = 'PluginFusioninventoryComputer'",
//Only retrieve computers IDs
$results = self::getDatas(
'PluginFusioninventoryComputer',
$search_params,
array('2')
);

$results = Search::prepareDatasForSearch('PluginFusioninventoryComputer', $search_params, array('2'));
Search::constructSQL($results);
$results['sql']['search'] = str_replace("`mainitemtype` = 'PluginFusioninventoryComputer'",
"`mainitemtype` = 'Computer'", $results['sql']['search']);
Search::constructDatas($results);
Search::constructDatas($results);

$ids = array();
foreach ($results['data']['rows'] as $row) {
$ids[$row['id']] = $row['id'];
foreach ($results['data']['rows'] as $id => $row) {
$ids[$row['id']] = $row['id'];
}

//store results in cache (for reusing on agent communication)
self::storeCache($group, $ids);
}

return $ids;
}

static function storeCache(PluginFusioninventoryDeployGroup $group, $ids) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add phpdoc

global $DB;

$query = "UPDATE ".self::getTable()."
SET `computers_id_cache` = '".$DB->escape(json_encode($ids))."'
WHERE `plugin_fusioninventory_deploygroups_id` = '".$group->getID()."'";
return $DB->query($query);
}

static function retrieveCache(PluginFusioninventoryDeployGroup $group) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add phpdoc

global $DB;

$ids = false;
$data = getAllDatasFromTable(self::getTable(),
"`plugin_fusioninventory_deploygroups_id` = '".$group->getID()."'");
if (count($data)) {
$first = array_shift($data);
$ids = json_decode($first['computers_id_cache'], true);
}

return $ids;
}
}
Expand Down
7 changes: 4 additions & 3 deletions inc/task.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ function getTaskjobstatesForAgent($agent_id, $methods = array(), $options=array(
$actor_key = "".key($actor)."_".$actor[key($actor)];
if (!isset($actors[$actor_key])) {
$actors[$actor_key] = array();
foreach ($this->getAgentsFromActors(array($actor)) as $agent) {
foreach ($this->getAgentsFromActors(array($actor), true) as $agent) {
$actors[$actor_key][$agent] = true;
}
}
Expand Down Expand Up @@ -662,9 +662,10 @@ function prepareTaskjobs($methods = array()) {
* corresponding itemtype classes.
*
* @param array $actors
* @param bool $use_cache retrieve agents from cache or not
* @return array list of agents
*/
public function getAgentsFromActors($actors = array()) {
public function getAgentsFromActors($actors = array(), $use_cache = false) {
$agents = array();
$computers = array();
$computer = new Computer();
Expand All @@ -688,7 +689,7 @@ public function getAgentsFromActors($actors = array()) {
case 'PluginFusioninventoryDeployGroup':
$group_targets = $pfToolbox->executeAsFusioninventoryUser(
'PluginFusioninventoryDeployGroup::getTargetsForGroup',
array($itemid)
array($itemid, $use_cache)
);
foreach ($group_targets as $computerid) {
$computers[$computerid] = 1;
Expand Down
1 change: 1 addition & 0 deletions install/mysql/plugin_fusioninventory-empty.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On big computers database, perhaps use longtext or blob

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but, should not blob cause issues (I'm thinking about future multi-database support mainly)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for longtext

PRIMARY KEY (`id`),
KEY `plugin_fusioninventory_deploygroups_id` (`plugin_fusioninventory_deploygroups_id`),
KEY `can_update_group` (`can_update_group`)
Expand Down
42 changes: 19 additions & 23 deletions install/update.php
Original file line number Diff line number Diff line change
Expand Up @@ -894,25 +894,6 @@ function pluginFusioninventoryUpdate($current_version, $migrationname='Migration
'comment'=>'Wake agents ups'));
}

/**
* Add field to manage which group can be refreshed by updatedynamictasks crontask
*/
if (!FieldExists('glpi_plugin_fusioninventory_deploygroups_dynamicdatas', 'can_update_group')) {
$migration->addField('glpi_plugin_fusioninventory_deploygroups_dynamicdatas', 'can_update_group', 'bool');
$migration->addKey('glpi_plugin_fusioninventory_deploygroups_dynamicdatas', 'can_update_group');
$migration->migrationOneTable('glpi_plugin_fusioninventory_deploygroups_dynamicdatas');
}


//Change static & dynamic structure to fit the GLPI framework
$migration->changeField('glpi_plugin_fusioninventory_deploygroups_dynamicdatas',
'groups_id',
'plugin_fusioninventory_deploygroups_id', 'integer');
$migration->migrationOneTable('glpi_plugin_fusioninventory_deploygroups_dynamicdatas');
$migration->changeField('glpi_plugin_fusioninventory_deploygroups_staticdatas',
'groups_id', 'plugin_fusioninventory_deploygroups_id', 'integer');
$migration->migrationOneTable('glpi_plugin_fusioninventory_deploygroups_staticdatas');



// Fix software version in computers. see https://github.com/fusioninventory/fusioninventory-for-glpi/issues/1810
Expand Down Expand Up @@ -5534,7 +5515,7 @@ function do_deploygroup_migration($migration) {
'type' => 'autoincrement',
'value' => NULL
),
'groups_id' => array(
'plugin_fusioninventory_deploygroups_id' => array(
'type' => 'integer',
'value' => NULL
),
Expand All @@ -5552,11 +5533,12 @@ function do_deploygroup_migration($migration) {
);

$a_table['renamefields'] = array(
'groups_id' => 'plugin_fusioninventory_deploygroups_id',
);

$a_table['keys'] = array(
array(
'field' => 'groups_id',
'field' => 'plugin_fusioninventory_deploygroups_id',
'name' => '',
'type' => 'KEY'
),
Expand Down Expand Up @@ -5589,25 +5571,39 @@ function do_deploygroup_migration($migration) {
'type' => 'autoincrement',
'value' => NULL
),
'groups_id' => array(
'plugin_fusioninventory_deploygroups_id' => array(
'type' => 'integer',
'value' => NULL
),
'fields_array' => array(
'type' => 'text',
'value' => NULL
),
'can_update_group' => array(
'type' => 'bool',
'value' => 0
),
'computers_id_cache' => array(
'type' => 'text',
'value' => NULL
),
);

$a_table['oldfields'] = array(
);

$a_table['renamefields'] = array(
'groups_id' => 'plugin_fusioninventory_deploygroups_id',
);

$a_table['keys'] = array(
array(
'field' => 'groups_id',
'field' => 'plugin_fusioninventory_deploygroups_id',
'name' => '',
'type' => 'KEY'
),
array(
'field' => 'can_update_group',
'name' => '',
'type' => 'KEY'
),
Expand Down