Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

further reduce api response time by indexing DB tables that can be filtered via in /explorer #4555

Closed
wants to merge 46 commits into from

Conversation

6 participants
@owocki
Copy link
Member

commented May 31, 2019

Description

further reduce api speed by indexing DB tables that can be filtered via in /explorer

takes the API reponse time down from 0.45s to 0.2s in some situations

Refers/Fixes

#4550

Testing

tested locally

@owocki owocki requested a review from danlipert May 31, 2019

@owocki

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

@danlipert i suppose we could index_together a lot of these fields, but there are many many many permutations here.. not sure it's worth it.

@codecov

This comment has been minimized.

Copy link

commented Jun 1, 2019

Codecov Report

Merging #4555 into stable will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           stable    #4555   +/-   ##
=======================================
  Coverage   30.14%   30.14%           
=======================================
  Files         210      210           
  Lines       16952    16952           
  Branches     2289     2289           
=======================================
  Hits         5111     5111           
  Misses      11643    11643           
  Partials      198      198
Impacted Files Coverage Δ
app/dashboard/models.py 56.47% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a936235...f789408. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

commented Jun 1, 2019

Codecov Report

Merging #4555 into stable will decrease coverage by 0.53%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           stable    #4555      +/-   ##
==========================================
- Coverage   30.41%   29.88%   -0.54%     
==========================================
  Files         216      210       -6     
  Lines       17223    17370     +147     
  Branches     2332     2419      +87     
==========================================
- Hits         5239     5191      -48     
- Misses      11776    11970     +194     
- Partials      208      209       +1
Impacted Files Coverage Δ
app/dashboard/models.py 56.69% <100%> (+1.01%) ⬆️
app/marketing/management/commands/sync_mail.py 0% <0%> (-29.17%) ⬇️
app/grants/templatetags/grants_extra.py 71.42% <0%> (-28.58%) ⬇️
app/grants/models.py 50.17% <0%> (-8.19%) ⬇️
app/marketing/mails.py 11.52% <0%> (-1.33%) ⬇️
app/retail/emails.py 21.52% <0%> (-0.81%) ⬇️
app/grants/admin.py 41.97% <0%> (-0.71%) ⬇️
app/economy/models.py 58% <0%> (-0.42%) ⬇️
app/retail/views.py 28.84% <0%> (-0.42%) ⬇️
app/app/settings.py 78.65% <0%> (-0.16%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e5b2e7...228ad93. Read the comment docs.

@owocki

This comment has been minimized.

Copy link
Member Author

commented Jun 1, 2019

f789408 adds some index_togethers!

@kuhnchris

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

you mean "increase API speed" or maybe "decrease response time" or "increase responsiveness"?

@danlipert danlipert changed the title further reduce api speed by indexing DB tables that can be filtered via in /explorer further reduce api response time by indexing DB tables that can be filtered via in /explorer Jun 12, 2019

repo_type = models.CharField(max_length=50, choices=REPO_TYPES, default='public')
snooze_warnings_for_days = models.IntegerField(default=0)
is_featured = models.BooleanField(
default=False, help_text=_('Whether this bounty is featured'))
featuring_date = models.DateTimeField(blank=True, null=True)
featuring_date = models.DateTimeField(blank=True, null=True, db_index=True)

This comment has been minimized.

Copy link
@thelostone-mc

thelostone-mc Jun 12, 2019

Member

@owocki / @danlipert do we need to index featuring_date?
indexing == yay but only if it's legit needed right ?

This comment has been minimized.

Copy link
@owocki

owocki Jun 12, 2019

Author Member

what do we need to fix about it?

indexing == yay but only if it's legit needed right ?

IMHO the trade off here is pretty positive ROI. indexing on a table the size of dashboard_bounty is cheap (in terms of storage space + write speed) + it improves lookup times quite significantly

@owocki

This comment has been minimized.

Copy link
Member Author

commented Jun 12, 2019

do i press the big scary merge button? or do u guys do that?

@thelostone-mc

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

@owocki we'll get it in :P

@thelostone-mc thelostone-mc added this to Review-Pending in PR Review Board Jun 23, 2019

@danlipert danlipert requested a review from octavioamu Jun 28, 2019

@danlipert

This comment has been minimized.

Copy link
Collaborator

commented Jun 28, 2019

@octavioamu you want to take a look at this? Also @thelostone-mc we should rebase and merge the migrations

@danlipert danlipert moved this from Review-Pending to Revisit + Fix in PR Review Board Jun 28, 2019

octavioamu and others added some commits Jun 28, 2019

octavioamu and others added some commits Jul 11, 2019

Merge pull request #4760 from gitcoinco/kevin/profile_speed_2
improves profile speed by moving the activity tab into lazy load
Merge pull request #4761 from gitcoinco/kevin/prfile_speed_3
increase profile speed by 0.5s by adding an index
Merge pull request #4763 from gitcoinco/kevin/profile_performance_5
profile performance - saves 0.25 seconds by hitting the DB once total for activity tabs per profile load instead of once per activity type
Check-In Tool (ambassador) API extension (#4750)
* Added new set for checkin tool

* Update router.py
Check funds before popup metamask (#4742)
* add balance checker

* add featured bounties

* fix msg when featured with token
Merge pull request #4781 from gitcoinco/fix/invite-bounty
Fix invite to work on a bounty detail and user directory
@thelostone-mc

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Closing this out and getting #4812 in instead

@thelostone-mc thelostone-mc moved this from Revisit + Fix to Done in PR Review Board Jul 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.