mmcgrath@xxxxxxxxxx wrote: > > On Mar 14, 2009, at 8:27 PM, Nigel Jones <nigjones@xxxxxxxxxx> wrote: > >> >> ----- "Toshio Kuratomi" <a.badger@xxxxxxxxx> wrote: >> >>> ricky and I have identified a piece of code in fas2's new >>> safasprovider >>> that's querying the database a lot. It's causing anything that >>> checks >>> identity to issue a query to the database to lookup the visit cookie. >>> This is causing things that lookup information on the identity >>> multiple >>> times to take a lot of time. One of the functions that does this is >>> filter_private(), the function that removes excess information >>> according >>> to privacy settings and who is looking for the data. Our test was >>> the >>> /user/list method which is currently running over 5 minutes for an >>> otherwise non-database loop. Changing this caused the loop to run for >>> 8 >>> seconds. >>> >>> That's the benefit. The risk is that this is a change to the >>> safasprovider, ie the portion of fas2 that authenticates the user. >>> So >>> if there's a reason this shouldn't be cached, we could potentially be >>> breaking a lot of things. >>> >>> Ricky and I have both looked at the code in >>> fas/safasprovider.py::SaFasIdentity and think that it's safe to cache >>> this. The TG-1.0.8 saprovider on which safasprovider is based does >>> not >>> cache this but I've looked at the code and it seems like their >>> provider >>> only uses the variable in question a maximum of two times during a >>> request. The CSRF protection that we've enabled needs to use this >>> variable more often. >>> >>> Here's the code: >>> >>> --- a/fas/safasprovider.py >>> +++ b/fas/safasprovider.py >>> @@ -65,6 +65,7 @@ class SaFasIdentity(object): >>> >>> def __init__(self, visit_key=None, user=None, using_ssl=False): >>> self.visit_key = visit_key >>> + self._visit_link = None >>> if user: >>> self._user = user >>> if visit_key is not None: >>> @@ -201,9 +202,13 @@ class SaFasIdentity(object): >>> ### TG: Same as TG-1.0.8 >>> def _get_visit_link(self): >>> '''Get the visit link to this identity.''' >>> + if self._visit_link: >>> + return self.visit_link >>> if self.visit_key is None: >>> - return None >>> - return >>> visit_class.query.filter_by(visit_key=self.visit_key).first() >>> + self._visit_link = None >>> + else: >>> + self._visit_link = >>> visit_class.query.filter_by(visit_key=self.visi >>> t_key).first() >>> + return self._visit_link >>> visit_link = property(_get_visit_link) >>> >>> If we were outside of freeze, I would apply this as it is causing >>> issues >>> for some of the things that talk to fas (like zodbot and developer >>> instances of pkgdb). >>> >>> -Toshio >> +1 - I live for danger! >> >> That said, anything that speeds it up is a good thing! > > +1. Just the beta freezeand the revert is easy. > Thanks guys. Hotfixed on the server. If anyone notices wierdness with authentication to fas, let me know. -Toshio
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Fedora-infrastructure-list mailing list Fedora-infrastructure-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-infrastructure-list
