----- "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! > > > _______________________________________________ > Fedora-infrastructure-list mailing list > Fedora-infrastructure-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/fedora-infrastructure-list _______________________________________________ Fedora-infrastructure-list mailing list Fedora-infrastructure-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-infrastructure-list
