Vedovani68 / bungeni-portal

Automatically exported from code.google.com/p/bungeni-portal
0 stars 0 forks source link

TeamSpace slowness .. #112

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
At the moment, accessing content under '.../debate-record-folder/...' is
very slow. manage_debug_threads shows that we're constantly in::

  File ".../TeamSpace/security.py", line 47, in _getTeamLocalRoles
    for m in t.getActiveMemberships():
  File ".../TeamSpace/team.py", line 317, in getActiveMemberships
    return self.getMembershipsByStates(self._active_states)
  File ".../TeamSpace/team.py", line 344, in getMembershipsByStates
    return [b.getObject() for b in brains

Check whether something's wrong, or whether TeamSpace needs some
caching.

Original issue reported on code.google.com by jean.jor...@gmail.com on 3 Oct 2007 at 3:42

GoogleCodeExporter commented 9 years ago
When browsing 
http://reporter:8080/plone/parliaments/parliament-ix/session-56-of-parliament-9/
sitting-3858-of-session-56
`Team.getMembershipsByStates` gets called 420 times. 409 of these calls are
on the parent team ('parliament-ix'). It has 228 members, and they're
all active, so they're all woken up. This counts for 93252 iterations. 

In addition, the reporter is member of two teams ('debate-record-office'
and 'dro') that have 1 and 22 members respectively. This counts for
another 792 iterations and 28 objects woken up.

I'm going to see if I can memoize this method. 

Original comment by jean.jor...@gmail.com on 4 Oct 2007 at 9:05

GoogleCodeExporter commented 9 years ago
The attached patch seems to do the trick. I just want to cache per request, as 
teams
and membership states may change at any time. CleanupDict will see that the 
cache is
 purged from time to time, though there's probably a better way to accomplish this.

Does it look sane to you?

In Plone 3, memoize is available. For 2.5 without memoize, we could do 
something like:

try:
  from plone.memoize import volatile
except ImportError:
  class v:
    def cache(self, key):
      pass
  volatile = v()

Original comment by jean.jor...@gmail.com on 4 Oct 2007 at 3:04

Attachments:

GoogleCodeExporter commented 9 years ago
the analysis and the need for a cache fix look good. roles and security are 
computed
many times in a request. the plone.memoize package is a good place, the volatile
cache is a little suspect for per request volatiles, since its not request 
based but
time based. i would instead look at something analagous in lifetime value scope 
to
the plone.memoize.view module, which stores and annotates cache values on the
request, and is thus bound to its lifecycle, and creating a simple teamspace 
cache
decorator that based on that which pulls.

waking the object isn't avoidable without changing the logic, but the query and
results resolution, and dereferencing objects will be problematic.

i had a talk with rob whose had much more real world deployment experience with
teamspace.. he's suggest using borg.localrole and the corresponding branch of
teamspace. my conversation with him is below, for more details.

[12:25]     RaFromBRC   sure, what's up?
[12:25]     hazmat  i wanted to ask some questions on teamspace
[12:25]     RaFromBRC   word
[12:26]     hazmat  i'm advising on a un project and their running into some 
scaling issues
[12:26]     RaFromBRC   yeah, i bet
[12:26]     RaFromBRC   what issues, specifically?
[12:26]     hazmat  i was curious, what the largest team sizes that you've been 
able to
useable use teamspace at
[12:26]     hazmat  specifically it needs caching on the security local roles check 
per
request
[12:26]     RaFromBRC   have you looked at the borg.localroles branch?
[12:27]     hazmat  local roles get computed alot
[12:27]     RaFromBRC   yeah
[12:27]     RaFromBRC   but the borg.localroles branch helps a lot
[12:27]     hazmat  is it simple?
[12:27]     RaFromBRC   very
[12:27]     hazmat  aka easy to comprehend risk, or is it just easier to use a 
variation
on plone.memoize and fix the issue
[12:27]     hazmat  directly
[12:28]     RaFromBRC   switches to using a PAS plug-in for computation, instead of 
the
computed attribute
[12:28]     RaFromBRC   the benefit is that when it's looking up the local roles for 
a
single user, it only needs to compute the roles for that user
[12:28]     RaFromBRC   currently it always has to compute all of them
[12:28]     hazmat  ahh
[12:28]     RaFromBRC   i did it as a side project for someone who had big teams... 
pages
were taking 60 seconds, this dropped them down to regular response times
[12:28]     hazmat  right.. it uses the pas plugin local role hook on a user class 
to
compute roles in context based on membership
[12:29]     hazmat  much better
[12:29]     RaFromBRC   no caching, but big improvement
[12:29]     hazmat  i like it
[12:29]     hazmat  nice work :-)
[12:29]     RaFromBRC   a drop in replacement, too, assuming you install 
borg.localroles
[12:29]     RaFromBRC   it's in production on openplans.org, i'm confident in it
[12:29]     RaFromBRC   no migration necessary
[12:29]     hazmat  cool, how big are the the largest teams you've seen with it
[12:30]     hazmat  rouhgly
[12:30]     RaFromBRC   only catch is if you have any code explicitly relying on
__ac_local_roles__ it'll break, you have to use the API's
[12:30]     RaFromBRC   i haven't seen very big ones myself... under 100
[12:30]     RaFromBRC   how big do you need to get?
[12:31]     hazmat  we're at 230s on test data, i'd like to know it works up to 1-2k
[12:31]     hazmat  but considering you don't have to load all brains
[12:31]     RaFromBRC   you'll definitely want to load test
[12:31]     hazmat  its just a much better solution than whats in ts by default
[12:31]     hazmat  since you only have to do current user lookups 
[12:31]     RaFromBRC   yeah,  i would have put it on the trunk, but i needed to 
stay with
the stable version, except for that change
[12:31]     hazmat  instead of computing an entire local role set for a space
[12:32]     RaFromBRC   might need another pre-2.0 branch or something
[12:32]     RaFromBRC   right
[12:32]     RaFromBRC   there are still times when that may be necessary, but not 
nearly
as many
[12:32]     hazmat  i can't think of any off hand
[12:32]     RaFromBRC   and caching can happen at the PAS layer, if necessary
[12:32]     hazmat  the whole existence for it is the security machinery
[12:32]     RaFromBRC   right
[12:32]     hazmat  and thats happening via a different mechanism with this
[12:32]     hazmat  and its all dyanmically computed, so few migration risks
[12:33]     RaFromBRC   i've been seeing slowdowns with spaces that have a lot of
content... security reindexing of the spaces takes a long time
[12:33]     RaFromBRC   playing w/ queuecatalog for that
[12:33]     RaFromBRC   right
[12:33]     hazmat  thats just the nature of the beast re index ;-) get a new beast
[12:33]     hazmat  my kingdom for a horse
[12:33]     RaFromBRC   i've had no migration issues... just drop it in, get it 
working,
all works exactly as before
[12:33]     RaFromBRC   yup
[12:33]     hazmat  perfect, thanks a bunch, thats very helpful
[12:33]     RaFromBRC   n/p... let me know if you have problems

Original comment by kapilt@gmail.com on 4 Oct 2007 at 4:40

GoogleCodeExporter commented 9 years ago
sorry didn't edit what i previously wrote prior to talking with rob, definitely 
the
best solution is using the borg.localrole package and corresponding branch of
teamspace. its just a much better way of getting the security local roles 
working for
the current user, without forcing computation of the whole set. caching by 
itself is
a bandage, this fixes the underlying issue, if you still need to add caching on 
top
of that it also can be done, but i think the algorithmic fix should suffice for
useability.

Original comment by kapilt@gmail.com on 4 Oct 2007 at 4:43

GoogleCodeExporter commented 9 years ago
Beautiful, thanks a lot for following up. I agree that caching is a bandage, 
will
check out the branch & if there are no issues, update the buildout to use that
instead of the current release. 

Original comment by jean.jor...@gmail.com on 5 Oct 2007 at 10:23

GoogleCodeExporter commented 9 years ago
Moving to 'ra-borg.localrole-branch':
- Move from TeamSpace 1.5 release to the branch.
- Add 'borg.localrole' to 'buildout.cfg'
- Add 'zope.deferredimport' to 'buildout.cfg' ('borg.localrole'
  dependency).

Breakage: zope.deferredimport pulls in
zope.interface-3.4.1-py2.4-linux-i686.egg and
zope.proxy-3.4.0-py2.4-linux-i686.egg .. now Zope startup fails. 

Is this a Plone 2.5 issue?

Traceback attached. 

Original comment by jean.jor...@gmail.com on 25 Oct 2007 at 12:39

Attachments:

GoogleCodeExporter commented 9 years ago
This is on
https://bungeni-portal.googlecode.com/svn/bungeni.buildout/branches/teamspace-bo
rg.localrole-branch

Original comment by jean.jor...@gmail.com on 25 Oct 2007 at 12:39

GoogleCodeExporter commented 9 years ago
this is just a general issue with using buildout in zope2.

you cannot and should not try to use any zope.* egg dependencies, the zope.*
dependencies are already present in the system in non egg form as part of a zope
distribution, trying to pull in any of these eggs results in pulling in the 
latest
versions of them off pypi, which will conflict with the packages in the zope
distribution.

also you shouldn't try to specify dependencies by hand for eggs you want, just
specify the egg you want, buildout will automatically pull in the dependency 
for you.

so basically remove the buildout.cfg spec for zope.deferredimport and also i
discovered you need to change the download url for appy as the file download 
url has
changed upstream. 

to fix an existing buildout just rm -Rf eggs/zope.*

hth

-k

Original comment by kapilt@gmail.com on 25 Oct 2007 at 9:16

GoogleCodeExporter commented 9 years ago

Original comment by ashok.ha...@gmail.com on 8 May 2008 at 7:42

GoogleCodeExporter commented 9 years ago

Original comment by ashok.ha...@gmail.com on 30 Jul 2009 at 9:26