dkupka's pull request #47: "schema cache: Store and check info for pre-schema servers" was opened
PR body: """ Cache CommandError answer to schema command to avoid sending the command to pre-schema servers every time. This information expires after some time (1 hour) in order to start using schema as soon as the server is upgraded. https://fedorahosted.org/freeipa/ticket/6095 """ See the full pull-request at https://github.com/freeipa/freeipa/pull/47 ... or pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/47/head:pr47 git checkout pr47
From 815bb2a5bc837f88275e39b3854fc6ae408c2b44 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Mon, 22 Aug 2016 13:34:30 +0200 Subject: [PATCH] schema cache: Store and check info for pre-schema servers Cache CommandError answer to schema command to avoid sending the command to pre-schema servers every time. This information expires after some time (1 hour) in order to start using schema as soon as the server is upgraded. https://fedorahosted.org/freeipa/ticket/6095 --- ipaclient/remote_plugins/__init__.py | 77 +++++++++++++------ ipaclient/remote_plugins/compat.py | 9 ++- ipaclient/remote_plugins/schema.py | 145 ++++++++++++++++++----------------- 3 files changed, 138 insertions(+), 93 deletions(-) diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py index 2be9222..3d6ba8f 100644 --- a/ipaclient/remote_plugins/__init__.py +++ b/ipaclient/remote_plugins/__init__.py @@ -5,7 +5,9 @@ import collections import errno import json +import locale import os +import time from . import compat from . import schema @@ -23,8 +25,16 @@ class ServerInfo(collections.MutableMapping): def __init__(self, api): hostname = DNSName(api.env.server).ToASCII() self._path = os.path.join(self._DIR, hostname) + self._force_check = api.env.force_schema_check self._dict = {} - self._dirty = False + + # copy-paste from ipalib/rpc.py + try: + self._language = ( + locale.setlocale(locale.LC_ALL, '').split('.')[0].lower() + ) + except locale.Error: + self._language = 'en_us' self._read() @@ -32,11 +42,7 @@ def __enter__(self): return self def __exit__(self, *_exc_info): - self.flush() - - def flush(self): - if self._dirty: - self._write() + self._write() def _read(self): try: @@ -62,13 +68,10 @@ def __getitem__(self, key): return self._dict[key] def __setitem__(self, key, value): - if key not in self._dict or self._dict[key] != value: - self._dirty = True self._dict[key] = value def __delitem__(self, key): del self._dict[key] - self._dirty = True def __iter__(self): return iter(self._dict) @@ -76,26 +79,54 @@ def __iter__(self): def __len__(self): return len(self._dict) + def update_validity(self, ttl=None): + if ttl is None: + ttl = 3600 + self['expiration'] = time.time() + ttl + self['language'] = self._language + + def is_valid(self): + if self._force_check: + return False + + try: + expiration = self._dict['expiration'] + language = self._dict['language'] + self._dict['fingerprint'] + except KeyError: + # if any of these is missing consider the entry expired + return False + + if expiration < time.time(): + # validity passed + return False + + if language != self._language: + # language changed since last check + return False + + return True + def get_package(api): if api.env.in_tree: from ipaserver import plugins else: - client = rpcclient(api) - client.finalize() - try: - server_info = api._server_info + plugins = api._remote_plugins except AttributeError: - server_info = api._server_info = ServerInfo(api) - - try: - plugins = schema.get_package(api, server_info, client) - except schema.NotAvailable: - plugins = compat.get_package(api, server_info, client) - finally: - server_info.flush() - if client.isconnected(): - client.disconnect() + with ServerInfo(api) as server_info: + client = rpcclient(api) + client.finalize() + + try: + plugins = schema.get_package(server_info, client) + except schema.NotAvailable: + plugins = compat.get_package(server_info, client) + finally: + if client.isconnected(): + client.disconnect() + + object.__setattr__(api, '_remote_plugins', plugins) return plugins diff --git a/ipaclient/remote_plugins/compat.py b/ipaclient/remote_plugins/compat.py index 5e08cb0..984eecd 100644 --- a/ipaclient/remote_plugins/compat.py +++ b/ipaclient/remote_plugins/compat.py @@ -31,10 +31,15 @@ class CompatObject(Object): pass -def get_package(api, server_info, client): +def get_package(server_info, client): try: server_version = server_info['version'] except KeyError: + is_valid = False + else: + is_valid = server_info.is_valid() + + if not is_valid: if not client.isconnected(): client.connect(verbose=False) env = client.forward(u'env', u'api_version', version=u'2.0') @@ -51,6 +56,8 @@ def get_package(api, server_info, client): else: server_version = u'2.0' server_info['version'] = server_version + server_info.update_validity() + server_version = LooseVersion(server_version) package_names = {} diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py index 553da35..9c10807 100644 --- a/ipaclient/remote_plugins/schema.py +++ b/ipaclient/remote_plugins/schema.py @@ -7,10 +7,8 @@ import errno import fcntl import json -import locale import os import sys -import time import types import zipfile @@ -220,7 +218,7 @@ def _create_class(self, api, schema): def __call__(self, api): if self._class is None: - schema = api._schema[self.schema_key][self.full_name] + schema = self._schema[self.schema_key][self.full_name] name, bases, class_dict = self._create_class(api, schema) self._class = type(name, bases, class_dict) @@ -361,7 +359,8 @@ class Schema(object): namespaces = {'classes', 'commands', 'topics'} _DIR = os.path.join(paths.USER_CACHE_PATH, 'ipa', 'schema', FORMAT) - def __init__(self, api, server_info, client): + def __init__(self, client): + self._client = client self._dict = {} self._namespaces = {} self._help = None @@ -371,49 +370,6 @@ def __init__(self, api, server_info, client): self._dict[ns] = {} self._namespaces[ns] = _SchemaNameSpace(self, ns) - # copy-paste from ipalib/rpc.py - try: - self._language = ( - locale.setlocale(locale.LC_ALL, '').split('.')[0].lower() - ) - except locale.Error: - # fallback to default locale - self._language = 'en_us' - - try: - self._fingerprint = server_info['fingerprint'] - self._expiration = server_info['expiration'] - language = server_info['language'] - except KeyError: - is_known = False - else: - is_known = (not api.env.force_schema_check and - self._expiration > time.time() and - self._language == language) - - if is_known: - try: - self._read_schema() - except Exception: - pass - else: - return - - try: - self._fetch(client) - except NotAvailable: - raise - except SchemaUpToDate as e: - self._fingerprint = e.fingerprint - self._expiration = time.time() + e.ttl - self._read_schema() - else: - self._write_schema() - - server_info['fingerprint'] = self._fingerprint - server_info['expiration'] = self._expiration - server_info['language'] = self._language - @contextlib.contextmanager def _open(self, filename, mode): path = os.path.join(self._DIR, filename) @@ -429,20 +385,22 @@ def _open(self, filename, mode): finally: fcntl.flock(f, fcntl.LOCK_UN) - def _fetch(self, client): - if not client.isconnected(): - client.connect(verbose=False) + def fetch_schema(self, ignore_cache=False): + if not self._client.isconnected(): + self._client.connect(verbose=False) - try: - fps = [fsdecode(f) for f in os.listdir(self._DIR)] - except EnvironmentError: - fps = [] + fps = [] + if not ignore_cache: + try: + fps = [fsdecode(f) for f in os.listdir(self._DIR)] + except EnvironmentError: + pass kwargs = {u'version': u'2.170'} if fps: kwargs[u'known_fingerprints'] = fps try: - schema = client.forward(u'schema', **kwargs)['result'] + schema = self._client.forward(u'schema', **kwargs)['result'] except errors.CommandError: raise NotAvailable() @@ -459,13 +417,16 @@ def _fetch(self, client): logger.warning("Failed to fetch schema: %s", e) raise NotAvailable() - self._fingerprint = fp - self._expiration = time.time() + ttl + return (fp, ttl,) - def _read_schema(self): + def read_schema(self, fingerprint): self._file.truncate(0) - with self._open(self._fingerprint, 'r') as f: - self._file.write(f.read()) + try: + with self._open(fingerprint, 'r') as f: + self._file.write(f.read()) + except EnvironmentError as e: + logger.warning("Failed to read schema: {}".format(e)) + raise with zipfile.ZipFile(self._file, 'r') as schema: for name in schema.namelist(): @@ -500,7 +461,7 @@ def _generate_help(self, schema): return halp - def _write_schema(self): + def write_schema(self, fingerprint): try: os.makedirs(self._DIR) except EnvironmentError as e: @@ -523,9 +484,12 @@ def _write_schema(self): json.dumps(self._generate_help(self._dict))) self._file.seek(0) - with self._open(self._fingerprint, 'w') as f: - f.truncate(0) - f.write(self._file.read()) + try: + with self._open(fingerprint, 'w') as f: + f.truncate(0) + f.write(self._file.read()) + except EnvironmentError as e: + logger.warning("Failed to write schema: {}".format(e)) def _read(self, path): with zipfile.ZipFile(self._file, 'r') as zf: @@ -550,12 +514,55 @@ def get_help(self, namespace, member): return self._help[namespace][member] -def get_package(api, server_info, client): +def get_package(server_info, client): try: - schema = api._schema - except AttributeError: - schema = Schema(api, server_info, client) - object.__setattr__(api, '_schema', schema) + fingerprint = server_info['fingerprint'] + except KeyError: + is_valid = False + else: + if fingerprint is None: + raise NotAvailable() + is_valid = server_info.is_valid() + + schema = Schema(client) + + success = False + read_failed = False + while not success: + if is_valid: + try: + schema.read_schema(fingerprint) + except Exception: + # Failed to read the schema from cache. There may be a lot of + # causes and not much we can do about it. Just ensure we will + # ignore the cache and fetch the schema from server. + is_valid = False + read_failed = True + else: + success = True + else: + update = False + try: + fingerprint, ttl = schema.fetch_schema( + ignore_cache=read_failed) + except NotAvailable: + fingerprint = None + ttl = None + update = True + raise + except SchemaUpToDate as e: + fingerprint = e.fingerprint + ttl = e.ttl + update = True + is_valid = True # read from cache + else: + schema.write_schema(fingerprint) + update = True + success = True + finally: + if update: + server_info['fingerprint'] = fingerprint + server_info.update_validity(ttl) fingerprint = str(server_info['fingerprint']) package_name = '{}${}'.format(__name__, fingerprint)
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code