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

Async gist upload #8

Open
baines opened this issue Apr 22, 2017 · 1 comment
Open

Async gist upload #8

baines opened this issue Apr 22, 2017 · 1 comment

Comments

@baines
Copy link
Owner

baines commented Apr 22, 2017

Currently, inso_gist.h is totally synchronous — when modules using it like mod_quotes do an upload, their response can feel laggy since they do the upload immediately before responding.

It is actually a bit worse than that, since they will first do a GET with If-Modified-Since to get any changes to the gist that may have occurred since the last check. This stops any manual gist changes from being overwritten, but again adds some lag.

If GitHub correctly supported the If-Match header on their gist patch API, the GET request would be unnecessary most of the time, but unfortunately their API is broken and responds with a 412 but still modifies the contents in the presence of a non-matching If-Match. It also seems to ignore If-Unmodified-Since.

A decent way to improve the responsiveness would be to assume the gist hasn't changed, update based on our local data, start the GET + upload in a background thread (or via curl's async multi interface), then we can respond immediately.

The problem is we would need to have a way of handling merges if the async GET got an updated gist, and potentially correct our response if it included wrong info (like a stale quote ID). Having multiple async uploads in flight could also be problematic, especially if an early one needs merging... needs some further thought.

@baines
Copy link
Owner Author

baines commented Aug 6, 2017

This may no longer be appropriate now that the quotes aren't using gist, and are much faster as a result. (e23782f)

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

No branches or pull requests

1 participant