support intermediate flushes when encoding #155
Conversation
due to the use of ready!(), whenever the underlying reader returns Poll::Pending, it is transmitted directly to the caller, so there is no flush until the entire data has been read. This commit flushes compressed data when we get Poll::Pending, and stays in the flushing state in case there is more data to send.
Fix #1572 async-compression is used in tower-http's CompressionLayer. Inside the AsyncRead based compression code, if the underlying stream returns Poll::Pending, it will be returned directly, while in the case of deferred responses, the next one might come way later, so we want to send whatever data we have available right now. We will have to switch to an AsyncWrite interface instead, which will be more flexible, but considering the timing of the release, this patch will hold for now. This uses the code from Nullus157/async-compression#155
|
I think this effort has been abandoned (because the apollo graphql router just removed compression for multipart responses), but we're facing a similar issue with "html streaming" where the content-type is text/html. It's a slow-drip stream too. Reproduction: Edit: Does not reproduce anymore, using this PR with this small change fixes it: prīmum...jeromegn:async-compression:fly-encoder-flush#diff-db895ba77ea00b18eddff756a977174f242792a8797afe28999357484774946dR100 Trying this branch (thankfully) broke our tests. It looks like the read loop is stuck like: Since it has already read some bytes, |
|
It's not abandoned, I'm just under tight constraints, due to moving out, and the router release coming up soon 😅 I'd like to look a bit at using a writer oriented approach from inside tower-http, maybe that will be a cleaner way |
We replace tower-http's `CompressionLayer` with a custom stream transformation. This is necessary because tower-http uses async-compression, which buffers data until the end of the stream to then write it, ensuring a better compression. This is incompatible with the multipart protocol for `@defer`, which requires chunks to be sent as soon as possible. So we need to compress them independently. This extracts parts of the codec module of async-compression, which so far is not public, and makes a streaming wrapper above it that flushes the compressed data on every response in the stream. This is expected to be temporary, as we have in flight PRs for async-compression: - Nullus157/async-compression#155 - Nullus157/async-compression#178 With Nullus157/async-compression#150 we might be able to at least remove the vendored code
robjtede
changed the title
disclaimer: this is an attempt at fixing apollographql/router#1572, for which I'm under tight timing constraints, so right now this PR's goal is discussing and finding a quick fix that we can use directly in the router, and then I'll help find a proper solution.
tentative fix for #154
Here I make sure to call the
flushmethod on pending, but what happens here is that one 64 bytes chunk of compressed data is returned immediately (not enough for the first part of the encoded data), and then it waits for 5s for the rest. Any advice on making it work? Does it need to flush multiple times?