idea: RangeReader should send API based on input read buffer's size

[Xuanwo]

RangeReader will send request to read the entire range everytime no matter how large the read buffer size is. So opendal doesn't work well in this case:

let r = op.reader(path).await?;
r.read(&mut [u8;1]).await?;
r.seek(SeekFrom::Current(1)).await?;
r.read(&mut [u8;1]).await?;
r.seek(SeekFrom::Current(1)).await?;

For every read call, range reader will send a new request to read the entire range but only consume a small piece:

• The connection will have to abort, makes it can't be reuse.
• And more data will be fecthed from server side.

Maybe we can send request based on user's input buffer size instead. For users we want to do small read, they can add a buffer over the reader instead.

[Xuanwo]

A possible regression: read_to_end(&mut Vec::new()) could be very slow in this case.

[Xuanwo]

After this change, adding buffer on reader makes more sense.

[Xuanwo]

I mixed two different thing together.

It doesn't make sense to use users input buffer size as the io size. The performance could drop to 50% for the read_to_end cases.

[WenyXu]

I mixed two different things together.

It doesn't make sense to use users input buffer size as the io size. The performance could drop to 50% for the read_to_end cases.

Currently, the operator in our system that enables the cache(it always returns a file-backed reader), looks like the following:

S3Accessor                FsAccessor
ErrorContextLayer         ErrorContextLayer
CompleteLayer             CompleteLayer
    ▲                         ▲  │
    │                         │  │
    │`inner`           `cache`│  │
    │                         │  │
    │                         │  │
    │                         │  │
    └───── LruCacheLayer ─────┘  │
                 ▲               │      
                 │               │
                 │               │ 
                 │               ▼
                 │               FileReader::new(oio::TokioReader<tokio::fs::File>) 
                 │ 
                 Invoking(`reader`, `reader_with`)  

I think your system might also have a similar approach, resulting in us noticing this issue until recently.

It seems like we need another way to work around it. How about just keeping the IncomingAsyncBody in the memory if the underlying data is fully fetched?

[Xuanwo]

It seems like we need another way to work around it.

It's acceptable to execute such tasks at the application level, but not within OpenDAL. I benchmarked the cost of terminating a connection; it takes approximately 600us (0.6ms), which is acceptable.

I'm also thinking about adding position read (like read_at) API in opendal, but not figure out how to add it.

[WenyXu]

such tasks

Do you mean keeping the IncomingAsyncBody in the memory(or similar approaches)?

[Xuanwo]

Do you mean keeping the IncomingAsyncBody in the memory(or similar approaches)?

Yes, we can't do such thing unless they are zero cost. Buffering body in memory could panic users when they meet OOM unexpectedly.

[WenyXu]

Do you mean keeping the IncomingAsyncBody in the memory(or similar approaches)?

Yes, we can't do such thing unless they are zero cost. Buffering body in memory could panic users when they meet OOM unexpectedly.

I see. There are some tradeoffs we have to make.

[Xuanwo]

Thanks for understanding. This issue is not valid anymore, I'm going to close it now.