Mercurial > kallithea
changeset 8343:a67bcc6f9118
db: drop SA caching_query and FromCache, and thus sql_cache_short beaker cache
It is not a good idea to have dead ORM objects. If we want caching, we should
do it explicit.
It is unknown how much this cache helps, but we can profile and introduce
better caching of simple data where relevant.
| author | Mads Kiilerich <mads@kiilerich.com> |
|---|---|
| date | Sun, 20 Oct 2019 22:06:26 +0200 |
| parents | dd3171263afd |
| children | aec1b9c9ffe6 |
| files | development.ini kallithea/lib/auth.py kallithea/lib/base.py kallithea/lib/caching_query.py kallithea/lib/helpers.py kallithea/lib/paster_commands/template.ini.mako kallithea/model/db.py kallithea/model/meta.py kallithea/model/repo.py kallithea/model/user.py kallithea/model/user_group.py kallithea/tests/conftest.py |
| diffstat | 12 files changed, 27 insertions(+), 329 deletions(-) [+] |
line wrap: on
line diff
--- a/development.ini Sun Oct 20 23:55:46 2019 +0200 +++ b/development.ini Sun Oct 20 22:06:26 2019 +0200 @@ -275,7 +275,7 @@ beaker.cache.data_dir = %(here)s/data/cache/data beaker.cache.lock_dir = %(here)s/data/cache/lock -beaker.cache.regions = short_term,long_term,sql_cache_short,long_term_file +beaker.cache.regions = short_term,long_term,long_term_file beaker.cache.short_term.type = memory beaker.cache.short_term.expire = 60 @@ -285,10 +285,6 @@ beaker.cache.long_term.expire = 36000 beaker.cache.long_term.key_length = 256 -beaker.cache.sql_cache_short.type = memory -beaker.cache.sql_cache_short.expire = 10 -beaker.cache.sql_cache_short.key_length = 256 - beaker.cache.long_term_file.type = file beaker.cache.long_term_file.expire = 604800 beaker.cache.long_term_file.key_length = 256
--- a/kallithea/lib/auth.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/lib/auth.py Sun Oct 20 22:06:26 2019 +0200 @@ -40,7 +40,6 @@ from webob.exc import HTTPForbidden, HTTPFound from kallithea.config.routing import url -from kallithea.lib.caching_query import FromCache from kallithea.lib.utils import conditional_cache, get_repo_group_slug, get_repo_slug, get_user_group_slug from kallithea.lib.utils2 import ascii_bytes, ascii_str, safe_bytes from kallithea.lib.vcs.utils.lazy import LazyProperty @@ -139,7 +138,7 @@ #====================================================================== # fetch default permissions #====================================================================== - default_user = User.get_by_username('default', cache=True) + default_user = User.get_by_username('default') default_user_id = default_user.user_id default_repo_perms = Permission.get_default_perms(default_user_id) @@ -390,7 +389,7 @@ if not dbuser.active: log.info('Db user %s not active', dbuser.username) return None - allowed_ips = AuthUser.get_allowed_ips(dbuser.user_id, cache=True) + allowed_ips = AuthUser.get_allowed_ips(dbuser.user_id) if not check_ip_access(source_ip=ip_addr, allowed_ips=allowed_ips): log.info('Access for %s from %s forbidden - not in %s', dbuser.username, ip_addr, allowed_ips) return None @@ -550,14 +549,11 @@ ) @classmethod - def get_allowed_ips(cls, user_id, cache=False): + def get_allowed_ips(cls, user_id): _set = set() default_ips = UserIpMap.query().filter(UserIpMap.user_id == - User.get_default_user(cache=True).user_id) - if cache: - default_ips = default_ips.options(FromCache("sql_cache_short", - "get_user_ips_default")) + User.get_default_user().user_id) for ip in default_ips: try: _set.add(ip.ip_addr) @@ -567,9 +563,6 @@ pass user_ips = UserIpMap.query().filter(UserIpMap.user_id == user_id) - if cache: - user_ips = user_ips.options(FromCache("sql_cache_short", - "get_user_ips_%s" % user_id)) for ip in user_ips: try: _set.add(ip.ip_addr)
--- a/kallithea/lib/base.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/lib/base.py Sun Oct 20 22:06:26 2019 +0200 @@ -223,7 +223,7 @@ Returns (None, wsgi_app) to send the wsgi_app response to the client. """ # Use anonymous access if allowed for action on repo. - default_user = User.get_default_user(cache=True) + default_user = User.get_default_user() default_authuser = AuthUser.make(dbuser=default_user, ip_addr=ip_addr) if default_authuser is None: log.debug('No anonymous access at all') # move on to proper user auth @@ -454,7 +454,7 @@ return log_in_user(user, remember=False, is_external_auth=True, ip_addr=ip_addr) # User is default user (if active) or anonymous - default_user = User.get_default_user(cache=True) + default_user = User.get_default_user() authuser = AuthUser.make(dbuser=default_user, ip_addr=ip_addr) if authuser is None: # fall back to anonymous authuser = AuthUser(dbuser=default_user) # TODO: somehow use .make?
--- a/kallithea/lib/caching_query.py Sun Oct 20 23:55:46 2019 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,233 +0,0 @@ -# apparently based on https://github.com/sqlalchemy/sqlalchemy/blob/rel_0_7/examples/beaker_caching/caching_query.py - -"""caching_query.py - -Represent persistence structures which allow the usage of -Beaker caching with SQLAlchemy. - -The three new concepts introduced here are: - - * CachingQuery - a Query subclass that caches and - retrieves results in/from Beaker. - * FromCache - a query option that establishes caching - parameters on a Query - * _params_from_query - extracts value parameters from - a Query. - -The rest of what's here are standard SQLAlchemy and -Beaker constructs. - -""" -import beaker -from beaker.exceptions import BeakerException -from sqlalchemy.orm.interfaces import MapperOption -from sqlalchemy.orm.query import Query -from sqlalchemy.sql import visitors - - -class CachingQuery(Query): - """A Query subclass which optionally loads full results from a Beaker - cache region. - - The CachingQuery stores additional state that allows it to consult - a Beaker cache before accessing the database: - - * A "region", which is a cache region argument passed to a - Beaker CacheManager, specifies a particular cache configuration - (including backend implementation, expiration times, etc.) - * A "namespace", which is a qualifying name that identifies a - group of keys within the cache. A query that filters on a name - might use the name "by_name", a query that filters on a date range - to a joined table might use the name "related_date_range". - - When the above state is present, a Beaker cache is retrieved. - - The "namespace" name is first concatenated with - a string composed of the individual entities and columns the Query - requests, i.e. such as ``Query(User.id, User.name)``. - - The Beaker cache is then loaded from the cache manager based - on the region and composed namespace. The key within the cache - itself is then constructed against the bind parameters specified - by this query, which are usually literals defined in the - WHERE clause. - - The FromCache mapper option below represent - the "public" method of configuring this state upon the CachingQuery. - - """ - - def __init__(self, manager, *args, **kw): - self.cache_manager = manager - Query.__init__(self, *args, **kw) - - def __iter__(self): - """override __iter__ to pull results from Beaker - if particular attributes have been configured. - - Note that this approach does *not* detach the loaded objects from - the current session. If the cache backend is an in-process cache - (like "memory") and lives beyond the scope of the current session's - transaction, those objects may be expired. The method here can be - modified to first expunge() each loaded item from the current - session before returning the list of items, so that the items - in the cache are not the same ones in the current Session. - - """ - if hasattr(self, '_cache_parameters'): - return self.get_value(createfunc=lambda: - list(Query.__iter__(self))) - else: - return Query.__iter__(self) - - def invalidate(self): - """Invalidate the value represented by this Query.""" - - cache, cache_key = _get_cache_parameters(self) - cache.remove(cache_key) - - def get_value(self, merge=True, createfunc=None): - """Return the value from the cache for this query. - - Raise KeyError if no value present and no - createfunc specified. - - """ - cache, cache_key = _get_cache_parameters(self) - ret = cache.get_value(cache_key, createfunc=createfunc) - if merge: - ret = self.merge_result(ret, load=False) - return ret - - -def query_callable(manager, query_cls=CachingQuery): - def query(*arg, **kw): - return query_cls(manager, *arg, **kw) - return query - - -def get_cache_region(name, region): - if region not in beaker.cache.cache_regions: - raise BeakerException('Cache region `%s` not configured ' - 'Check if proper cache settings are in the .ini files' % region) - kw = beaker.cache.cache_regions[region] - return beaker.cache.Cache._get_cache(name, kw) - - -def _get_cache_parameters(query): - """For a query with cache_region and cache_namespace configured, - return the corresponding Cache instance and cache key, based - on this query's current criterion and parameter values. - - """ - if not hasattr(query, '_cache_parameters'): - raise ValueError("This Query does not have caching " - "parameters configured.") - - region, namespace, cache_key = query._cache_parameters - - namespace = _namespace_from_query(namespace, query) - - if cache_key is None: - # cache key - the value arguments from this query's parameters. - args = _params_from_query(query) - args.append(query._limit) - args.append(query._offset) - cache_key = " ".join(str(x) for x in args) - - if cache_key is None: - raise Exception('Cache key cannot be None') - - # get cache - #cache = query.cache_manager.get_cache_region(namespace, region) - cache = get_cache_region(namespace, region) - # optional - hash the cache_key too for consistent length - # import uuid - # cache_key= str(uuid.uuid5(uuid.NAMESPACE_DNS, cache_key)) - - return cache, cache_key - - -def _namespace_from_query(namespace, query): - # cache namespace - the token handed in by the - # option + class we're querying against - namespace = " ".join([namespace] + [str(x) for x in query._entities]) - - # memcached wants this - namespace = namespace.replace(' ', '_') - - return namespace - - -def _set_cache_parameters(query, region, namespace, cache_key): - - if hasattr(query, '_cache_parameters'): - region, namespace, cache_key = query._cache_parameters - raise ValueError("This query is already configured " - "for region %r namespace %r" % - (region, namespace) - ) - query._cache_parameters = region, namespace, cache_key - - -class FromCache(MapperOption): - """Specifies that a Query should load results from a cache.""" - - propagate_to_loaders = False - - def __init__(self, region, namespace, cache_key=None): - """Construct a new FromCache. - - :param region: the cache region. Should be a - region configured in the Beaker CacheManager. - - :param namespace: the cache namespace. Should - be a name uniquely describing the target Query's - lexical structure. - - :param cache_key: optional. A string cache key - that will serve as the key to the query. Use this - if your query has a huge amount of parameters (such - as when using in_()) which correspond more simply to - some other identifier. - - """ - self.region = region - self.namespace = namespace - self.cache_key = cache_key - - def process_query(self, query): - """Process a Query during normal loading operation.""" - - _set_cache_parameters(query, self.region, self.namespace, - self.cache_key) - - -def _params_from_query(query): - """Pull the bind parameter values from a query. - - This takes into account any scalar attribute bindparam set up. - - E.g. params_from_query(query.filter(Cls.foo==5).filter(Cls.bar==7))) - would return [5, 7]. - - """ - v = [] - - def visit_bindparam(bind): - if bind.key in query._params: - value = query._params[bind.key] - elif bind.callable: - # lazyloader may dig a callable in here, intended - # to late-evaluate params after autoflush is called. - # convert to a scalar value. - value = bind.callable() - else: - value = bind.value - - v.append(value) - if query._criterion is not None: - visitors.traverse(query._criterion, {}, {'bindparam': visit_bindparam}) - for f in query._from_obj: - visitors.traverse(f, {}, {'bindparam': visit_bindparam}) - return v
--- a/kallithea/lib/helpers.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/lib/helpers.py Sun Oct 20 22:06:26 2019 +0200 @@ -568,7 +568,7 @@ email = author_email(author) if email: from kallithea.model.db import User - user = User.get_by_email(email, cache=True) # cache will only use sql_cache_short + user = User.get_by_email(email) if user is not None: return getattr(user, show_attr) return None
--- a/kallithea/lib/paster_commands/template.ini.mako Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/lib/paster_commands/template.ini.mako Sun Oct 20 22:06:26 2019 +0200 @@ -381,7 +381,7 @@ beaker.cache.data_dir = %(here)s/data/cache/data beaker.cache.lock_dir = %(here)s/data/cache/lock -beaker.cache.regions = short_term,long_term,sql_cache_short,long_term_file +beaker.cache.regions = short_term,long_term,long_term_file beaker.cache.short_term.type = memory beaker.cache.short_term.expire = 60 @@ -391,10 +391,6 @@ beaker.cache.long_term.expire = 36000 beaker.cache.long_term.key_length = 256 -beaker.cache.sql_cache_short.type = memory -beaker.cache.sql_cache_short.expire = 10 -beaker.cache.sql_cache_short.key_length = 256 - beaker.cache.long_term_file.type = file beaker.cache.long_term_file.expire = 604800 beaker.cache.long_term_file.key_length = 256
--- a/kallithea/model/db.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/model/db.py Sun Oct 20 22:06:26 2019 +0200 @@ -46,7 +46,6 @@ import kallithea from kallithea.lib import ext_json -from kallithea.lib.caching_query import FromCache from kallithea.lib.exceptions import DefaultUserException from kallithea.lib.utils2 import (Optional, ascii_bytes, aslist, get_changeset_safe, get_clone_url, remove_prefix, safe_bytes, safe_int, safe_str, str2bool, urlreadable) @@ -278,13 +277,9 @@ return res @classmethod - def get_app_settings(cls, cache=False): + def get_app_settings(cls): ret = cls.query() - - if cache: - ret = ret.options(FromCache("sql_cache_short", "get_hg_settings")) - if ret is None: raise Exception('Could not get application settings !') settings = {} @@ -295,7 +290,7 @@ return settings @classmethod - def get_auth_settings(cls, cache=False): + def get_auth_settings(cls): ret = cls.query() \ .filter(cls.app_settings_name.startswith('auth_')).all() fd = {} @@ -304,7 +299,7 @@ return fd @classmethod - def get_default_repo_settings(cls, cache=False, strip_prefix=False): + def get_default_repo_settings(cls, strip_prefix=False): ret = cls.query() \ .filter(cls.app_settings_name.startswith('default_')).all() fd = {} @@ -548,7 +543,7 @@ return user @classmethod - def get_by_username_or_email(cls, username_or_email, case_insensitive=True, cache=False): + def get_by_username_or_email(cls, username_or_email, case_insensitive=True): """ For anything that looks like an email address, look up by the email address (matching case insensitively). @@ -557,35 +552,24 @@ This assumes no normal username can have '@' symbol. """ if '@' in username_or_email: - return User.get_by_email(username_or_email, cache=cache) + return User.get_by_email(username_or_email) else: - return User.get_by_username(username_or_email, case_insensitive=case_insensitive, cache=cache) + return User.get_by_username(username_or_email, case_insensitive=case_insensitive) @classmethod - def get_by_username(cls, username, case_insensitive=False, cache=False): + def get_by_username(cls, username, case_insensitive=False): if case_insensitive: q = cls.query().filter(sqlalchemy.func.lower(cls.username) == sqlalchemy.func.lower(username)) else: q = cls.query().filter(cls.username == username) - - if cache: - q = q.options(FromCache( - "sql_cache_short", - "get_user_%s" % _hash_key(username) - ) - ) return q.scalar() @classmethod - def get_by_api_key(cls, api_key, cache=False, fallback=True): + def get_by_api_key(cls, api_key, fallback=True): if len(api_key) != 40 or not api_key.isalnum(): return None q = cls.query().filter(cls.api_key == api_key) - - if cache: - q = q.options(FromCache("sql_cache_short", - "get_api_key_%s" % api_key)) res = q.scalar() if fallback and not res: @@ -600,20 +584,12 @@ @classmethod def get_by_email(cls, email, cache=False): q = cls.query().filter(sqlalchemy.func.lower(cls.email) == sqlalchemy.func.lower(email)) - - if cache: - q = q.options(FromCache("sql_cache_short", - "get_email_key_%s" % email)) - ret = q.scalar() if ret is None: q = UserEmailMap.query() # try fetching in alternate email map q = q.filter(sqlalchemy.func.lower(UserEmailMap.email) == sqlalchemy.func.lower(email)) q = q.options(joinedload(UserEmailMap.user)) - if cache: - q = q.options(FromCache("sql_cache_short", - "get_email_map_key_%s" % email)) ret = getattr(q.scalar(), 'user', None) return ret @@ -651,8 +627,8 @@ return user @classmethod - def get_default_user(cls, cache=False): - user = User.get_by_username(User.DEFAULT_USER, cache=cache) + def get_default_user(cls): + user = User.get_by_username(User.DEFAULT_USER) if user is None: raise Exception('Missing default account!') return user @@ -851,26 +827,16 @@ return super(UserGroup, cls).guess_instance(value, UserGroup.get_by_group_name) @classmethod - def get_by_group_name(cls, group_name, cache=False, - case_insensitive=False): + def get_by_group_name(cls, group_name, case_insensitive=False): if case_insensitive: q = cls.query().filter(sqlalchemy.func.lower(cls.users_group_name) == sqlalchemy.func.lower(group_name)) else: q = cls.query().filter(cls.users_group_name == group_name) - if cache: - q = q.options(FromCache( - "sql_cache_short", - "get_group_%s" % _hash_key(group_name) - ) - ) return q.scalar() @classmethod - def get(cls, user_group_id, cache=False): + def get(cls, user_group_id): user_group = cls.query() - if cache: - user_group = user_group.options(FromCache("sql_cache_short", - "get_users_group_%s" % user_group_id)) return user_group.get(user_group_id) def get_api_data(self, with_members=True): @@ -1480,7 +1446,7 @@ return super(RepoGroup, cls).guess_instance(value, RepoGroup.get_by_group_name) @classmethod - def get_by_group_name(cls, group_name, cache=False, case_insensitive=False): + def get_by_group_name(cls, group_name, case_insensitive=False): group_name = group_name.rstrip('/') if case_insensitive: gr = cls.query() \ @@ -1488,12 +1454,6 @@ else: gr = cls.query() \ .filter(cls.group_name == group_name) - if cache: - gr = gr.options(FromCache( - "sql_cache_short", - "get_group_%s" % _hash_key(group_name) - ) - ) return gr.scalar() @property
--- a/kallithea/model/meta.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/model/meta.py Sun Oct 20 22:06:26 2019 +0200 @@ -18,8 +18,6 @@ from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import scoped_session, sessionmaker -from kallithea.lib import caching_query - # Beaker CacheManager. A home base for cache configurations. cache_manager = cache.CacheManager() @@ -29,18 +27,13 @@ # # SQLAlchemy session manager. # -session_factory = sessionmaker( - query_cls=caching_query.query_callable(cache_manager), - expire_on_commit=True) +session_factory = sessionmaker(expire_on_commit=True) Session = scoped_session(session_factory) # The base class for declarative schemas in db.py # Engine is injected when model.__init__.init_model() sets meta.Base.metadata.bind Base = declarative_base() -# to use cache use this in query: -# .options(FromCache("sqlalchemy_cache_type", "cachekey")) - # Define naming conventions for foreign keys, primary keys, indexes, # check constraints, and unique constraints, respectively.
--- a/kallithea/model/repo.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/model/repo.py Sun Oct 20 22:06:26 2019 +0200 @@ -83,7 +83,6 @@ def get(self, repo_id): repo = Repository.query() \ .filter(Repository.repo_id == repo_id) - return repo.scalar() def get_repo(self, repository): @@ -92,7 +91,6 @@ def get_by_repo_name(self, repo_name): repo = Repository.query() \ .filter(Repository.repo_name == repo_name) - return repo.scalar() def get_all_user_repos(self, user):
--- a/kallithea/model/user.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/model/user.py Sun Oct 20 22:06:26 2019 +0200 @@ -36,7 +36,6 @@ from tg import config from tg.i18n import ugettext as _ -from kallithea.lib.caching_query import FromCache from kallithea.lib.exceptions import DefaultUserException, UserOwnsReposException from kallithea.lib.utils2 import generate_api_key, get_current_authuser from kallithea.model.db import Permission, User, UserEmailMap, UserIpMap, UserToPerm @@ -49,11 +48,8 @@ class UserModel(object): password_reset_token_lifetime = 86400 # 24 hours - def get(self, user_id, cache=False): + def get(self, user_id): user = User.query() - if cache: - user = user.options(FromCache("sql_cache_short", - "get_user_%s" % user_id)) return user.get(user_id) def get_user(self, user): @@ -203,7 +199,7 @@ def update(self, user_id, form_data, skip_attrs=None): from kallithea.lib.auth import get_crypt_password skip_attrs = skip_attrs or [] - user = self.get(user_id, cache=False) + user = self.get(user_id) if user.is_default_user: raise DefaultUserException( _("You can't edit this user since it's "
--- a/kallithea/model/user_group.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/model/user_group.py Sun Oct 20 22:06:26 2019 +0200 @@ -94,8 +94,8 @@ def get_group(self, user_group): return UserGroup.guess_instance(user_group) - def get_by_name(self, name, cache=False, case_insensitive=False): - return UserGroup.get_by_group_name(name, cache=cache, case_insensitive=case_insensitive) + def get_by_name(self, name, case_insensitive=False): + return UserGroup.get_by_group_name(name, case_insensitive=case_insensitive) def create(self, name, description, owner, active=True, group_data=None): try:
--- a/kallithea/tests/conftest.py Sun Oct 20 23:55:46 2019 +0200 +++ b/kallithea/tests/conftest.py Sun Oct 20 22:06:26 2019 +0200 @@ -45,7 +45,6 @@ #'ssh_locale': 'C', 'app_instance_uuid': 'test', 'show_revision_number': 'true', - 'beaker.cache.sql_cache_short.expire': '1', 'session.secret': '{74e0cd75-b339-478b-b129-07dd221def1f}', #'i18n.lang': '', },