[checkmk-commits] Fixed wrong cache usage

Module: check_mk Branch: master Commit: edf26498dcc5d7a44415707f683d58d428d3a37f URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=edf26498dcc5d7… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Tue Sep 26 15:42:41 2017 +0200 Fixed wrong cache usage An empty cache does not mean that it has not be populated. An empty population result can also be a regular case in which the calculation should not be done twice. Example: A sites needs to detect the list of hosts to be monitored locally. Even when there are hundret of sites with thousands of hosts monitored on all sites, a empty result has to be cached for a site that does not monitor any host. Change-Id: Id69612525c3e8ffc3b02e578f8a4543aad055cc3 --- cmk_base/caching.py | 15 +++++++++++++++ cmk_base/config.py | 21 ++++++++++++++------- tests/cmk_base/test_caching.py | 18 ++++++++++++++++++ 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/cmk_base/caching.py b/cmk_base/caching.py index e6451fa..28227b2 100644 --- a/cmk_base/caching.py +++ b/cmk_base/caching.py @@ -76,10 +76,25 @@ class CacheManager(object): class Cache(object): + _populated = False + def is_empty(self): + """Whether or not there is something in the collection at the moment""" return not self + def is_populated(self): + """Whether or not the cache has been marked as populated. This is just a flag + to tell the caller the initialization state of the cache. It has to be set + to True manually by using self.set_populated()""" + return self._populated + + + def set_populated(self): + self._populated = True + + + # Just a small wrapper round a dict to get some caching specific functionality # for analysis etc. class DictCache(dict, Cache): diff --git a/cmk_base/config.py b/cmk_base/config.py index b44c070..a8e37fb 100644 --- a/cmk_base/config.py +++ b/cmk_base/config.py @@ -399,8 +399,9 @@ def collect_hosttags(): # Returns a set of all active hosts def all_active_hosts(): cache = cmk_base.config_cache.get_set("all_active_hosts") - if cache.is_empty(): + if not cache.is_populated(): cache.update(all_active_realhosts(), all_active_clusters()) + cache.set_populated() return cache @@ -409,8 +410,9 @@ def all_active_hosts(): def all_active_realhosts(): active_realhosts = cmk_base.config_cache.get_set("active_realhosts") - if active_realhosts.is_empty(): + if not active_realhosts.is_populated(): active_realhosts.update(filter_active_hosts(all_configured_realhosts())) + active_realhosts.set_populated() return active_realhosts @@ -420,8 +422,9 @@ def all_active_realhosts(): def all_active_clusters(): active_clusters = cmk_base.config_cache.get_set("active_clusters") - if active_clusters.is_empty(): + if not active_clusters.is_populated(): active_clusters.update(filter_active_hosts(all_configured_clusters())) + active_clusters.set_populated() return active_clusters @@ -430,8 +433,9 @@ def all_active_clusters(): # disabled or monitored on a remote site. def all_configured_hosts(): cache = cmk_base.config_cache.get_set("all_configured_hosts") - if cache.is_empty(): + if not cache.is_populated(): cache.update(all_configured_realhosts(), all_configured_clusters()) + cache.set_populated() return cache @@ -440,8 +444,9 @@ def all_configured_hosts(): # cluster hosts. def all_configured_realhosts(): cache = cmk_base.config_cache.get_set("all_configured_realhosts") - if cache.is_empty(): + if not cache.is_populated(): cache.update(strip_tags(all_hosts)) + cache.set_populated() return cache @@ -450,8 +455,9 @@ def all_configured_realhosts(): # normal hosts. def all_configured_clusters(): cache = cmk_base.config_cache.get_set("all_configured_clusters") - if cache.is_empty(): + if not cache.is_populated(): cache.update(strip_tags(clusters.keys())) + cache.set_populated() return cache @@ -570,11 +576,12 @@ def parents_of(hostname): # If not, return an empty list. def clusters_of(hostname): cache = cmk_base.config_cache.get_dict("clusters_of") - if cache.is_empty(): + if not cache.is_populated(): for cluster, hosts in clusters.items(): clustername = cluster.split('|', 1)[0] for name in hosts: cache.setdefault(name, []).append(clustername) + cache.set_populated() return cache.get(hostname, []) diff --git a/tests/cmk_base/test_caching.py b/tests/cmk_base/test_caching.py index 33191eb..26deb00 100644 --- a/tests/cmk_base/test_caching.py +++ b/tests/cmk_base/test_caching.py @@ -71,3 +71,21 @@ def test_clear_all(): assert dict_cache.is_empty() assert set_cache.is_empty() + +def test_populated(): + mgr = cmk_base.caching.CacheManager() + + cache = mgr.get_set("test1") + assert not cache.is_populated() + cache.set_populated() + assert cache.is_populated() + + cache = mgr.get_dict("test2") + assert not cache.is_populated() + cache.set_populated() + assert cache.is_populated() + + cache = mgr.get_list("test3") + assert not cache.is_populated() + cache.set_populated() + assert cache.is_populated()