[checkmk-commits] 3347 FIX Improved performance when having a lot of users

Module: check_mk Branch: master Commit: ebd9312b17b1e24b18d27f71c4d7bd68c3f66005 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=ebd9312b17b1e2… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Tue Mar 29 13:08:08 2016 +0200 3347 FIX Improved performance when having a lot of users When having a lot of users (>1000) configured via WATO, e.g. created by the LDAP sync, the GUI was getting too slow since some versions. This has now been improved. --- .werks/3347 | 11 +++++++++ ChangeLog | 1 + web/htdocs/pagetypes.py | 6 ++--- web/htdocs/userdb.py | 61 ++++++++++++++++++++++++++++++++--------------- web/htdocs/visuals.py | 8 +++---- 5 files changed, 61 insertions(+), 26 deletions(-) diff --git a/.werks/3347 b/.werks/3347 new file mode 100644 index 0000000..288940c --- /dev/null +++ b/.werks/3347 @@ -0,0 +1,11 @@ +Title: Improved performance when having a lot of users +Level: 1 +Component: multisite +Class: fix +Compatible: compat +State: unknown +Version: 1.2.9i1 +Date: 1459249594 + +When having a lot of users (>1000) configured via WATO, e.g. created by the LDAP sync, +the GUI was getting too slow since some versions. This has now been improved. diff --git a/ChangeLog b/ChangeLog index 072a9c3..457cd8b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -140,6 +140,7 @@ * 3339 FIX: Fixed exception on "Aggregations Affected by Service" page when no host/service given * 3344 FIX: Removed host inventory painters and sorters from non host based views * 3345 FIX: Fixed exception when inventory painter was used as group painter + * 3347 FIX: Improved performance when having a lot of users... WATO: * 3244 WATO BI Module: swap order of aggregation function and child node selection... diff --git a/web/htdocs/pagetypes.py b/web/htdocs/pagetypes.py index 3045a53..9583d99 100644 --- a/web/htdocs/pagetypes.py +++ b/web/htdocs/pagetypes.py @@ -634,14 +634,14 @@ class Overridable: # Now scan users subdirs for files "user_$type_name.mk" subdirs = os.listdir(config.config_dir) for user in subdirs: - if not userdb.user_exists(user): - continue - try: path = "%s/%s/user_%ss.mk" % (config.config_dir, user, self.type_name()) if not os.path.exists(path): continue + if not userdb.user_exists(user): + continue + user_pages = eval(file(path).read()) for name, page_dict in user_pages.items(): page_dict["owner"] = user diff --git a/web/htdocs/userdb.py b/web/htdocs/userdb.py index d17678a..61123ba 100644 --- a/web/htdocs/userdb.py +++ b/web/htdocs/userdb.py @@ -170,11 +170,32 @@ def create_non_existing_user(connection_id, username): # Call the sync function for this new user hook_sync(connection_id = connection_id, only_username = username) -# FIXME: Can we improve this easily? Would be nice not to have to call "load_users". -# Maybe a directory listing of profiles or a list of a small file would perform better -# than having to load the users, contacts etc. during each http request to multisite + +# This function is called very often during regular page loads so it has to be efficient +# even when having a lot of users. +# +# When using the multisite authentication with just by WATO created users it would be +# easy, but we also need to deal with users which are only existant in the htpasswd +# file and don't have a profile directory yet. def user_exists(username): - return username in load_users().keys() + if _user_exists_according_to_profile(username): + return True + + return _user_exists_htpasswd(username) + + +def _user_exists_according_to_profile(username): + base_path = config.config_dir + "/" + username + "/" + return os.path.exists(base_path + "transids.mk") \ + or os.path.exists(base_path + "serial.mk") + + +def _user_exists_htpasswd(username): + for line in open(defaults.htpasswd_file): + if line.startswith("%s:" % username): + return True + return False + def user_locked(username): users = load_users() @@ -469,7 +490,6 @@ def split_dict(d, keylist, positive): return dict([(k,v) for (k,v) in d.items() if (k in keylist) == positive]) def save_users(profiles): - # Add custom macros core_custom_macros = [ k for k,o in user_attributes.items() if o.get('add_custom_macro') ] for user in profiles.keys(): @@ -578,24 +598,27 @@ def save_users(profiles): if 'last_seen' in user: save_custom_attr(user_id, 'last_seen', repr(user['last_seen'])) - # Remove settings directories of non-existant users. - # Beware: we removed this since it leads to violent destructions - # if the user database is out of the scope of Check_MK. This is - # e.g. the case, if mod_ldap is used for user authentication. - # dir = defaults.var_dir + "/web" - # for e in os.listdir(dir): - # if e not in ['.', '..'] and e not in profiles: - # entry = dir + "/" + e - # if os.path.isdir(entry): - # shutil.rmtree(entry) - # But for the automation.secret this is ok, since automation users are not - # created by other sources in common cases + # During deletion of users we don't delete files which might contain user settings + # and e.g. customized views which are not easy to reproduce. We want to keep the + # files which are the result of a lot of work even when e.g. the LDAP sync deletes + # a user by accident. But for some internal files it is ok to delete them. + # + # Be aware: The user_exists() function relies on these files to be deleted. + profile_files_to_delete = [ + "automation.secret", + "transids.mk", + "serial.mk", + ] dir = defaults.var_dir + "/web" for user_dir in os.listdir(defaults.var_dir + "/web"): if user_dir not in ['.', '..'] and user_dir.decode("utf-8") not in profiles: entry = dir + "/" + user_dir - if os.path.isdir(entry) and os.path.exists(entry + '/automation.secret'): - os.unlink(entry + '/automation.secret') + if not os.path.isdir(entry): + continue + + for to_delete in profile_files_to_delete: + if os.path.exists(entry + '/' + to_delete): + os.unlink(entry + '/' + to_delete) # Release the lock to make other threads access possible again asap # This lock is set by load_users() only in the case something is expected diff --git a/web/htdocs/visuals.py b/web/htdocs/visuals.py index 4a85a30..c7c1cf0 100644 --- a/web/htdocs/visuals.py +++ b/web/htdocs/visuals.py @@ -124,12 +124,9 @@ def load(what, builtin_visuals, skip_func = None, lock=False): visuals[('', name)] = visual - # Now scan users subdirs for files "visuals.mk" + # Now scan users subdirs for files "user_*.mk" subdirs = os.listdir(config.config_dir) for user in subdirs: - if not userdb.user_exists(user): - continue - try: dirpath = config.config_dir + "/" + user if not os.path.isdir(dirpath): @@ -145,6 +142,9 @@ def load(what, builtin_visuals, skip_func = None, lock=False): if not os.path.exists(path): continue + if not userdb.user_exists(user): + continue + if lock: aquire_lock(path)