async-compression icon indicating copy to clipboard operation
async-compression copied to clipboard

support intermediate flushes when encoding

Open Geal opened this issue 3 years ago • 2 comments

disclaimer: this is an attempt at fixing https://github.com/apollographql/router/issues/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 flush method 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?

Geal avatar Aug 24 '22 13:08 Geal

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:

curl -N https://frosty-frog-2648.fly.dev/ --compressed

Edit: Does not reproduce anymore, using this PR with this small change fixes it: https://github.com/Nemo157/async-compression/compare/pr%C4%ABmum...jeromegn:async-compression:fly-encoder-flush#diff-db895ba77ea00b18eddff756a977174f242792a8797afe28999357484774946dR100

Trying this branch (thankfully) broke our tests. It looks like the read loop is stuck like:

encoding -> flushing ->  encoding -> flushing -> ...

Since it has already read some bytes, read != 0, and so it will never ever return Poll::Pending after that. I have a branch where I'm just resetting read = 0 when I've flushed successfully. I don't know if that renders this fix useless or if I've broken something else.

jeromegn avatar Sep 19 '22 19:09 jeromegn

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

Geal avatar Sep 20 '22 18:09 Geal