Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

It takes a really long time to get a list of members for the gittip team #1417

Closed
zbynekwinkler opened this issue Sep 9, 2013 · 12 comments
Closed

Comments

@zbynekwinkler
Copy link
Contributor

$ time curl https://www.gittip.com/Gittip/members/index.json > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 21613    0 21613    0     0   5303      0 --:--:--  0:00:04 --:--:--  6054

real  0m4.321s
user  0m0.012s
sys   0m0.016s

Compared to:

$ time curl https://www.gittip.com/Gittip/members/ > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  8073    0  8073    0     0   7098      0 --:--:--  0:00:01 --:--:-- 10231

real  0m1.147s
user  0m0.008s
sys   0m0.008s

That is more that 3 seconds spent somewhere in the backend. Both measurements where taken from the same network within a couple of seconds apart. I believe it is not a network problem because:

$ time curl https://assets-gittipllc.netdna-ssl.com/10.1.7/gittip.css > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 36569    0 36569    0     0  33202      0 --:--:--  0:00:01 --:--:-- 47430

real  0m1.302s
user  0m0.004s
sys   0m0.020s
@seanlinsley
Copy link
Contributor

Yep, members/index.json� is taking ~4 seconds for me as well

@zbynekwinkler
Copy link
Contributor Author

Addressing scaling problems with the Infrastructure milestone as per IRC.

@chadwhitacre
Copy link
Contributor

I've decided to remove performance issues from the Infrastructure milestone (#1491, #1495, cf. #1585). Of course we need to address these, but we need to start getting Infrastructure over the hump, and we've got to cut weight from somewhere to do it. After Infrastructure is done, we can include related performance improvements in any focus on a specific part of the app (this one would fit under a focus on teams, for example), or in a specific focus on performance.

@chadwhitacre
Copy link
Contributor

Haven't benchmarked it but I suspect gittip._mixin_team.MixinTeam.get_take_last_week_for. That's called for every member of the team and it looks like it does a scan of the transfers table.

@chadwhitacre
Copy link
Contributor

[gittip] $ time heroku config -s -a gittip | honcho run -e /dev/stdin ./env/bin/python benchmark-membership-listing.py 
82

real    0m10.414s
user    0m1.176s
sys     0m0.202s
[gittip] $
#!/usr/bin/env python
from gittip import wireup
from gittip.models.participant import Participant

wireup.db(wireup.env())

member = Participant.from_username('whit537')
team = Participant.from_username('Gittip')

print len(team.get_memberships(member))

@chadwhitacre
Copy link
Contributor

Yeah. If I stub out get_take_last_week_for the real time drops to 2.223 s.

@chadwhitacre
Copy link
Contributor

Sorry, those were counting heroku config time. New numbers:

  • 7,846 ms w/ get_take_last_week_for
  • 375 ms w/o get_take_last_week_for

New script:

#!/usr/bin/env python
import time
from gittip import wireup
from gittip.models.participant import Participant


env = wireup.env()
wireup.db(env)

member = Participant.from_username('whit537')
team = Participant.from_username('Gittip')

start = time.time()
print "{} members".format(len(team.get_memberships(member)))
print "{} ms".format(int((time.time() - start) * 1000))

@chadwhitacre
Copy link
Contributor

So that would suggest a view materialization over transfers instead of memberships (#2282).

@rummik
Copy link
Contributor

rummik commented Apr 15, 2014

@whit537 I'm reasonably sure the bottleneck is that we're not indexing the table properly. I can't verify it though, since fake data doesn't generate quality fake data :P

In my tests, this reduces the cost for _mixin_team.py#L67-L72 from cost=0.00..2263.42 to cost=0.42..8.44:

CREATE INDEX transfers_tipper_tippee_timestamp_idx
  ON transfers
  USING btree
  (tipper, tippee, timestamp DESC);

Should I submit a PR for someone to play with? I don't really know how changes to the database are handled.

@chadwhitacre
Copy link
Contributor

@rummik Sure, give us a PR. Make a branch.sql file on your branch with the index as above.

chadwhitacre added a commit that referenced this issue Apr 16, 2014
@chadwhitacre
Copy link
Contributor

@rummik That did it. :-)

I applied the index in production and the page is much faster now:

https://www.gittip.com/Gittip/members/

@rummik
Copy link
Contributor

rummik commented Apr 16, 2014

@whit537 Ah. Yeah, I wasn't sure where to put it. I probably have a couple more to add to the list, so I'll make a note of what file that all goes in when I'm working on that :)

citruspi added a commit to citruspi/www.gittip.com that referenced this issue Apr 18, 2014
…o dockerfile

* 'master' of https://github.com/gittip/www.gittip.com:
  Modify Vagrantfile to use a custom gittip image.
  Keep manual exchanges out of payday self-check
  Append index to schema.sql; gratipay#1417
  Fix incorrect  cd added to .profile in Vagrant provisioning.
  Give devs a little pointer re: branch.sql
  Remove os.environ from error handling test page

Conflicts:
	README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants