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

[SVCS-899] Remove cute trick to deal with buffer size #359

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

NyanHelsing
Copy link
Contributor

@NyanHelsing NyanHelsing commented Aug 22, 2018

Ticket

https://openscience.atlassian.net/browse/SVCS-899

Purpose

Entire files were being pulled into memory when the client's connection to waterbutler was faster than waterbutler's connection to the provider. This is less than ideal. Wrapping around a socket is cute, but it's not really the right way to do it. Put an actual limit on the buffer size and wait for it to get written. Sending it through a unix socket was a quick and dirty way to get some control into the tornado's data_received handler method, letting us wait until some of the data had been read; The StreamReader has some functionality built in around limits to the buffer size, but if it can't pause the coroutine that's writing things to it, it can't do anything with the data other than save it to it's buffer anyway, otherwise it loses data.

The RequestStreamReader can be tweaked a bit to handle writes to its buffer with a future, which would let it achieve the same thing without needing to wrap a streamreader in another streamreader, and without needing to open up a socketr pair.

The commit that added this trick: bb77d79

Changes

Side effects

QA Notes

Deployment Notes

Wrapping around a socket is cute, but it's not really the right way to
do it. Put an actual limit on the buffer size and wait for it to get
written.
@NyanHelsing NyanHelsing changed the title Remove cute trick to deal with bufffer size *WIP* Remove cute trick to deal with bufffer size Aug 22, 2018
Remove the comment mentioning the commit that created the socket
changes, await the future that feed_data returns
@NyanHelsing NyanHelsing changed the title *WIP* Remove cute trick to deal with bufffer size *WIP* Remove cute trick to deal with buffer size Aug 22, 2018
@NyanHelsing NyanHelsing requested a review from cslzchen August 22, 2018 21:39
@NyanHelsing NyanHelsing changed the title *WIP* Remove cute trick to deal with buffer size [SVCS-899] *WIP* Remove cute trick to deal with buffer size Aug 23, 2018
@NyanHelsing
Copy link
Contributor Author

The test failing shouldn't matter because we're removing v0.

@NyanHelsing NyanHelsing changed the title [SVCS-899] *WIP* Remove cute trick to deal with buffer size [SVCS-899] Remove cute trick to deal with buffer size Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant