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