"error decoding response body: request or response body error: operation timed out" even when response stream is making progress

[alamb]

Describe the bug
When downloading a large enough file on a slow enough network, retrieving streaming results from ObjectStore::get will still report "operation timed out" even when it is making active progress

To Reproduce

use std::time::{Duration, Instant};
use futures::StreamExt;
use object_store::ObjectStore;
use object_store::path::Path;


#[tokio::main]
async fn main() {
    let start = Instant::now();
    let object_store_url = "https://datasets.clickhouse.com";
    let client_options = object_store::ClientOptions::default()
        // default the timeout to 1 second
        .with_timeout(Duration::from_secs(1));

    let object_store = object_store::http::HttpBuilder::new()
        .with_client_options(client_options)
        .with_url(object_store_url)
        .build()
        .unwrap();

    // this is a 14GB file
    let file_path = Path::from("hits_compatible/hits.parquet");
    let response = object_store.get(&file_path).await.unwrap();

    // read the response body relatively slowly
    let mut stream = response.into_stream();
    while let Some(chunk) = stream.next().await {
        let chunk = chunk.unwrap();
        // throttle the read speed
        tokio::time::sleep(Duration::from_millis(100)).await;
    }

    println!("{:?} Done", start.elapsed());
}

Results in the following after about 15 seconds:

thread 'main' panicked at src/main.rs:29:27:
called `Result::unwrap()` on an `Err` value: Generic { store: "HTTP", source: HttpError { kind: Timeout, source: reqwest::Error { kind: Body, source: reqwest::Error { kind: Decode, source: reqwest::Error { kind: Body, source: TimedOut } } } } }
stack backtrace:
...

It doesn't error after 1 second because the request times out and is retried 10 times

Expected behavior
As long as the client is consuming data and the server doesn't shut the connection, I expect that the program will successfully complete and read the entire file.

Additional context
The fix for #15 from @tustvold makes this much better (now the first 10 timeouts are retried) but eventually the timeout still happens

**Potential ideas **

1. Reset the retry counter once any data has been successfully read from the result stream
2. Have separate timeout / retry policies that are applied to timeout errors specifically

[tustvold]

I'm not sure this is a bug, we retry up to the number of max retries, by default 10. If people want more retries, they should raise this limit.

https://docs.rs/object_store/0.12.0/object_store/struct.RetryConfig.html

I think we should probably add a TransferManager style abstraction for people looking to download large assets, infinitely retrying a request IMO is problematic

[alamb]

I changed this to a feature request

From my end user perspective, if I make a request to fetch an object, and the connection is actively returning data that I am consuming, I don't expect the connection to "timeout" -- I expect timeouts to happen when there is no activity / no progress is being made

[alamb]

Retrying (with increasing backoff) of timeouts when the connection is slow also seems non ideal (as the backoffs just slow down the overall transfer)

Maybe the issue is that connections that are "active" shouldn't be timing out / being retried in the first place and we should consider some other mechanism 🤔

[alamb]

To be clear, the improvement in #383 is great and handles many other transient errors very nicely 👌

I am just thinking that the "timeout" maybe needs to be handled specially

I think we should probably add a TransferManager style abstraction for people looking to download large assets, infinitely retrying a request IMO is problematic

This is also an interesting idea -- however, it if is a separate structure then people need to decide when to use it (which depends on the remote object size and the network speed 🤔 )

[tustvold]

are "active" shouldn't be timing out / being retried in the first place and we should consider some other mechanism

I agree, IMO the intent of the retry machinery is to recover from transient errors, e.g. network interruption, not to allow operations to complete that would fail in the absence of retries. If a request is so large that it could never be completed in a single get operation, it needs to be broken up.

What does this chunking is potentially up for debate, I personally think we should add a TransferManager component that can automatically split up each of potentially multiple downloads into multiple concurrent parts. This is inline with the design goal of ObjectStore mirroring the object store APIs, and mimising the "magic" that makes reasoning about IO very difficult, whilst providing higher-level abstractions where applicable.

It has also been suggested that we build this into ObjectStore::get directly, perhaps as a wrapper, but I'm sceptical that this can be done completely transparently. It'll be a complex tradeoff between memory, network throughput, RTT, cost, etc... with relatively limited context on which to make this judgement.

FYI @crepererum

[alamb]

A Chunked Wrapper sounds like a reasonable approach (I think we have one in InfluxDB IOx)

One potential idea to provide user control is to have a set of wrappers that can be combined for various tradeoffs. This idea is discussed in

• [DISCUSSION] [object_store] New crate with object store combinators / utilitles #14