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

investigate how to speed up update_sources #1009

Closed
redshiftzero opened this issue Mar 25, 2020 · 6 comments
Closed

investigate how to speed up update_sources #1009

redshiftzero opened this issue Mar 25, 2020 · 6 comments
Assignees

Comments

@redshiftzero
Copy link
Contributor

While it's much improved, the logic in our metadata sync doesn't scale to 1000 sources. On first sync, the logic in update_source must run for all sources, which currently can take 10s of seconds up to a minute or so.

One option is to process one source at a time:

  1. Add it to the database
  2. Import its key
  3. Signal the UI to start updating
  4. Start on next source

Another option is for the sources in bulk:

  1. Add in bulk to the db (@rmol is gonna look at how much speedup this gives us)
  2. Import keys async
  3. Return to caller and signal the UI to start updating
@redshiftzero
Copy link
Contributor Author

(assigning @rmol to this one as he's made some progress on this on option 2 (bulk) above)

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Mar 30, 2020

While doing scalability testing on Friday, I was able to measure how long it took on average to get 300 sources (each run with a clean local database):

sources=300, messages=600, replies=600, run_count=3

Unit Mean time (seconds)
on_authenticate_success 0.1
get_sources endpoint 12
get_all_submissions endpoint 4
get_all_replies endpoint 4
update_local_storage 37
total sync time (get_sources + get_all_submissions + get_all_replies + update_local_storage) 57
on_sync_success 0.2

@rmol
Copy link
Contributor

rmol commented Mar 30, 2020

Were those timings taken on master or with the speedsync branch? I've seen update_local_storage take less time than that with 1000 sources on speedsync.

@sssoleileraaa
Copy link
Contributor

I'm sure the speedsync branch from #1024 speeds up update_local_storage - the table above shows how long things are currently taking so we know how much things are improved on your branch and if we improve other areas that are behind the ~57 second delay between login and showing 300 sources (on staging using tor). In the results above, you can see that even if we shorten update_local_storage to 2 seconds for 300 sources + 600 messages + 600 replies, we'll still take an additional ~20 seconds to get the remote data, so it would take 22 seconds before sources are displayed.

@redshiftzero
Copy link
Contributor Author

Ah, so measurements in #648 are without tor, and these measurements were with tor (makes sense since we're talking about reducing the time for the user to get feedback that things are updating), which I think explains the discrepancy - since the comment in #648, we reduced the compute time on the server side to <2s but the response won't be that snappy since the response for get_sources is still large and is going over Tor. I think three things make sense:

  1. Improve feedback indicator on first sync - right now users see the flash indicating that the sync started but we should at least keep flashing it until the sync completes (or something similar to flag to the user that the delay is expected). Tor is never going to be that snappy, and the sync may take a few seconds even when we remove keys from the get_sources endpoint. We can start this immediately.
  2. Continue to further optimize update_local_storage. This is in progress.
  3. Remove keys from GET /sources API responses: Journalist API v2 securedrop#5104 (planned for 1.4.0 on securedrop core side).

@eloquence eloquence added this to the 0.2.0 milestone Apr 1, 2020
@rmol rmol mentioned this issue Apr 2, 2020
6 tasks
@eloquence eloquence removed this from the 0.2.0 milestone Apr 23, 2020
@eloquence
Copy link
Member

Significant progress was made on this through #1034, #1035, #1036, #1037, #1046. Closing as stale for now; we can open new issues with more concrete performance goals in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants