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

Do not fetch all users on application start, fetch only updates #1804

Closed
kunall17 opened this issue Jan 26, 2018 · 5 comments
Closed

Do not fetch all users on application start, fetch only updates #1804

kunall17 opened this issue Jan 26, 2018 · 5 comments

Comments

@kunall17
Copy link
Contributor

Currently, we are fetching all the users from the server on the realm init, which is called on every application cold start.

I tested this out (in a local server) and the user addition works fine if the app is in the background or in the running but if the app is killed and a user registers on the server then this user info is not fetched.

Ideally, we could only pull the updates and can save a lot of network usage and the performance at startup can be increased as well!

Same goes for streams, realm_emoji but they can be ignored as they are really small as compared to users!

@borisyankov
Copy link
Contributor

How do you propose we fetch only the updates? There is no API call for that.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Feb 1, 2020

Hmm, there are realms like chat.zulip.org that have very large numbers of users. (GET chat.zulip.org/api/v1/users gives a response of 4.56 MB.)

@gnprice, @timabbott, what do you think? The currently open image caching and thumbnailing issues eat even more of people’s data (like #3853, and Greg, remember the 13 MB image and its repeatedly failed requests on Android), and Boris makes a good point about the current API, but I wondered if there'd been any activity on this that we could track here.

@chrisbobbe chrisbobbe added a-data-sync Zulip's event system, event queues, staleness/liveness and removed a-data-sync Zulip's event system, event queues, staleness/liveness labels Feb 1, 2020
@timabbott
Copy link
Sponsor Member

Just a note: Compressed size is mostly what we care about, not uncompressed size.

We have a plan involving long-lived event queues that covers this issue in a way we expect to be successful long-term. I'm not sure where the issue is for that, but probably this should be closed and just folded in as a bullet point on that issue/design doc.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Feb 4, 2020

I'm not sure where the issue is for that

I did some searching in zulip/zulip and didn't find it, but it sounds like it's being tracked. In Greg's description of the proposal, also, this issue is naturally covered, so it should be fine not to call it out as a special case.

Just a note: Compressed size is mostly what we care about, not uncompressed size.

Mm, that makes sense. (I just realized that we're getting the users with POST /register, not GET /users, but the response sizes are comparable, with our params.) A content-length header isn't received with the response, but, indeed, curl shows that a POST /register to czo with our params is compressed with content-encoding: br to 902 KB, which is better than the 5.08 MB, uncompressed (sizes from using -w '%{size_download}').

The 13 MB image was image/jpeg; the server doesn't do additional compression, which makes sense because it wouldn't reduce the size.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Feb 19, 2020

Tim mentions that the PR thread for the ability to downgrade to a long-lived event queue is zulip/zulip#12926.

but probably this should be closed and just folded in as a bullet point on that issue/design doc.

So, closing this, and opening #3916, which will encompass this and other benefits and implementation details.

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

No branches or pull requests

4 participants