This is an automated email from the ASF dual-hosted git repository.
erickguan pushed a commit to branch fix-reader
in repository https://gitbox.apache.org/repos/asf/opendal.git
The following commit(s) were added to refs/heads/fix-reader by this push:
new 68bcee2dd chore: restore read range RFC wording
68bcee2dd is described below
commit 68bcee2dd256ba6f2df826e4c16c3a6640889ffd
Author: Erick Guan <[email protected]>
AuthorDate: Sun Jul 5 12:26:02 2026 +0800
chore: restore read range RFC wording
---
.../docs/rfcs/7660_move_read_range_to_reader.md | 101 ++++++++++-----------
1 file changed, 46 insertions(+), 55 deletions(-)
diff --git a/core/core/src/docs/rfcs/7660_move_read_range_to_reader.md
b/core/core/src/docs/rfcs/7660_move_read_range_to_reader.md
index 1881e1780..623906f60 100644
--- a/core/core/src/docs/rfcs/7660_move_read_range_to_reader.md
+++ b/core/core/src/docs/rfcs/7660_move_read_range_to_reader.md
@@ -6,19 +6,19 @@
# Summary
Change OpenDAL's raw read contract so byte ranges are selected by the raw
reader
-instead of `Service::read`.
+instead of `Access::read`.
-Today `Service::read(path, OpRead { range, .. })` opens one concrete range
+Today `Access::read(path, OpRead { range, .. })` opens one concrete range
stream. This works well for HTTP object stores, where each range read is an
independent request. It is a poor fit for services such as `fs`, `sftp`,
`hdfs`, `hdfs-native`, and `monoiofs`, where the service may keep an opened
handle and execute multiple cursor-independent range reads against it. The
-current core reader planner still calls `Service::read` for every planned
range,
+current core reader planner still calls `Access::read` for every planned range,
forcing those services to repeatedly open and close handles.
-This RFC keeps the `Service::read` operation, but changes its contract:
+This RFC keeps the `Access::read` operation, but changes its contract:
-- `Service::read(path, OpRead)` creates a reusable raw `oio::Read` for the path
+- `Access::read(path, OpRead)` creates a reusable raw `oio::Read` for the path
and read condition set.
- `oio::Read::open(range)` opens a range-scoped byte stream.
- `oio::Read::read(range)` reads one bounded planner range into one `Buffer`.
@@ -41,12 +41,12 @@ let parts = reader.fetch(vec![0..1024, 4096..8192]).await?;
However, the current implementation plans these public calls by generating new
raw range streams. `ReadGenerator::next_reader` and `ChunkedReader` both call
-`Service::read(path, args.with_range(range))` for every range. For `s3`, `gcs`,
+`Access::read(path, args.with_range(range))` for every range. For `s3`, `gcs`,
and `azblob`, this is natural because each range is a new HTTP request. For
handle based services, this means every range repeats open and seek work even
when the target data can be represented by a reusable handle.
-The root cause is that `Service::read` is modeled as a transport-level range
+The root cause is that `Access::read` is modeled as a transport-level range
request. OpenDAL needs a storage-neutral raw model:
- Object stores can implement a raw reader that issues an independent ranged
@@ -84,7 +84,7 @@ let ranges = reader.fetch(vec![0..1024, 4096..8192]).await?;
The difference is internal. OpenDAL creates one reusable raw reader for
`reader("path/to/file")`. After that, range operations are executed by this raw
-reader instead of reopening through `Service::read` for every range.
+reader instead of reopening through `Access::read` for every range.
For HTTP object stores, the raw reader stores the path and read arguments, then
sends a new ranged request for each range. Behavior and cost stay the same.
@@ -98,18 +98,17 @@ a shared seek cursor. It must use cursor-independent reads
such as `pread` /
## Raw read contract
-`Service::read` remains the raw read operation:
+`Access::read` remains the raw read operation:
```rust,ignore
-pub trait Service: Send + Sync + Debug + Unpin + 'static {
+pub trait Access: Send + Sync + Debug + Unpin + 'static {
type Reader: oio::Read;
fn read(
&self,
- ctx: &OperationContext,
path: &str,
args: OpRead,
- ) -> Result<Self::Reader>;
+ ) -> impl Future<Output = Result<(RpRead, Self::Reader)>> + MaybeSend;
}
```
@@ -118,7 +117,7 @@ The meaning changes:
- `path` and `OpRead` select a path and read condition set.
- The returned `Self::Reader` is a reusable raw read executor for that path and
condition set.
-- No range is selected by `Service::read`.
+- No range is selected by `Access::read`.
`OpRead` should no longer contain `BytesRange`:
@@ -137,7 +136,7 @@ pub struct OpRead {
`OpReader` remains an internal execution policy used by OpenDAL's public
`Reader`. It controls `concurrent`, `chunk`, `gap`, `prefetch`, and
-`content_length_hint`. It must not become part of the backend `Service::read`
+`content_length_hint`. It must not become part of the backend `Access::read`
contract.
`ReadOptions` and `ReaderOptions` should be converted differently:
@@ -278,15 +277,10 @@ does not require every service to optimize handle reuse
immediately.
range, and execution policy:
```rust,ignore
-async fn read_inner(
- ctx: OperationContext,
- srv: Servicer,
- path: String,
- opts: ReadOptions,
-) -> Result<Buffer> {
+async fn read_inner(acc: Accessor, path: String, opts: ReadOptions) ->
Result<Buffer> {
let (range, op_read, op_reader) = opts.into();
- let raw_reader = srv.read(&ctx, &path, op_read)?;
- let reader = Reader::new(ctx, srv, path, raw_reader, op_reader);
+ let (rp, raw_reader) = acc.read(&path, op_read).await?;
+ let reader = Reader::new(acc, path, raw_reader, op_reader, rp);
reader.read(range.to_range()).await
}
```
@@ -294,19 +288,14 @@ async fn read_inner(
`Operator::reader_options` should create the reusable raw reader once:
```rust,ignore
-async fn reader_inner(
- ctx: OperationContext,
- srv: Servicer,
- path: String,
- opts: ReaderOptions,
-) -> Result<Reader> {
+async fn reader_inner(acc: Accessor, path: String, opts: ReaderOptions) ->
Result<Reader> {
let (op_read, op_reader) = opts.into();
- let raw_reader = srv.read(&ctx, &path, op_read)?;
- Ok(Reader::new(ctx, srv, path, raw_reader, op_reader))
+ let (rp, raw_reader) = acc.read(&path, op_read).await?;
+ Ok(Reader::new(acc, path, raw_reader, op_reader, rp))
}
```
-For services where `Service::read` currently performs remote I/O, the returned
+For services where `Access::read` currently performs remote I/O, the returned
raw reader should be lazy. This preserves the lazy reader behavior: creating
`op.reader(path)` should not require opening the underlying data request.
@@ -316,7 +305,7 @@ raw reader should be lazy. This preserves the lazy reader
behavior: creating
```rust,ignore
pub struct ReadContext {
- srv: Servicer,
+ acc: Accessor,
path: String,
args: OpRead,
options: OpReader,
@@ -325,19 +314,20 @@ pub struct ReadContext {
}
```
-`srv`, `path`, and `args` are still useful for planning, especially when
+`acc`, `path`, and `args` are still useful for planning, especially when
OpenDAL needs `stat` to resolve unbounded ranges for chunked reads or
`AsyncSeek` adapters. Actual raw range I/O should go through `reader.open` or
-`reader.read`, not through repeated `srv.read` calls.
+`reader.read`, not through repeated `acc.read` calls.
`Reader::metadata()` remains cache-only. The cache is updated from `RpRead`
-returned by `open` or `read`. Lazy backends may return `RpRead::default()` from
-the raw reader until the first raw range operation. Public `Reader::fetch`
-observes metadata from the planned raw `read` calls it executes.
+returned by `Access::read`, `open`, or `read`. Lazy backends may return
+`RpRead::default()` from `Access::read` and fill metadata after the first raw
+range operation. Public `Reader::fetch` observes metadata from the planned raw
+`read` calls it executes.
## Layers
-Existing layers continue to wrap `Service::read`, but the wrapping point moves
+Existing layers continue to wrap `Access::read`, but the wrapping point moves
from a range stream to a reusable raw reader.
### Type eraser
@@ -345,10 +335,11 @@ from a range stream to a reusable raw reader.
The type eraser should erase raw readers using the chosen object-safe
container:
```rust,ignore
-fn read(&self, ctx: &OperationContext, path: &str, args: OpRead) ->
Result<oio::Reader> {
+async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead,
oio::Reader)> {
self.inner
- .read(ctx, path, args)
- .map(|r| Box::new(r) as oio::Reader)
+ .read(path, args)
+ .await
+ .map(|(rp, r)| (rp, Box::new(r) as oio::Reader))
}
```
@@ -358,7 +349,7 @@ specific type alias.
### Complete layer
-`CompleteLayer` previously read `args.range().size()` in `Service::read` and
+`CompleteLayer` currently reads `args.range().size()` in `Access::read` and
wraps the returned range stream. After this RFC, range size is only known when
`open` or `read` is called.
@@ -380,13 +371,13 @@ every range operation:
### Retry layer
-`RetryLayer` previously stored `OpRead` and advanced `args.range_mut()` after
+`RetryLayer` currently stores `OpRead` and advances `args.range_mut()` after
each successful stream read. After this RFC, range progress must move from
`OpRead` into range operation wrappers.
The new retry model should be:
-- retry `Service::read(path, OpRead)` while creating the reusable raw reader;
+- retry `Access::read(path, OpRead)` while creating the reusable raw reader;
- retry `raw_reader.open(range)` when opening a range stream fails before bytes
are returned;
- wrap the returned `ReadStream` and track bytes already emitted;
@@ -400,7 +391,7 @@ for the same path and read condition set.
### Correctness check layer
`CorrectnessCheckLayer` can keep checking `read_with_*` capabilities in
-`Service::read`, because those options still belong to the path and read
+`Access::read`, because those options still belong to the path and read
condition set.
## Presign read
@@ -429,7 +420,7 @@ OpReader)` and discards `OpReader`.
Services like `s3`, `gcs`, `azblob`, `oss`, and `http` should return a raw
reader that stores `core`, `path`, and `OpRead`.
-Each `open(range)` sends the same request that the current `Service::read`
sends
+Each `open(range)` sends the same request that the current `Access::read` sends
today. For example, S3's current logic:
```rust,ignore
@@ -475,8 +466,8 @@ public `Reader` keep the same user-facing behavior.
The raw API is breaking:
-- third-party `Service` implementations must remove range handling from
- `Service::read`;
+- third-party `Access` implementations must remove range handling from
+ `Access::read`;
- current range-scoped readers must become `ReadStream`;
- services must return reusable raw readers;
- layers must wrap reusable raw readers and range operation results.
@@ -486,7 +477,7 @@ Migration can be staged mechanically:
1. Introduce `ReadStream` and the new raw `Read` trait shape.
2. Move `BytesRange` out of `OpRead`.
3. Update type erasure and core read context.
-4. Convert object-store services by moving current `Service::read` request
logic
+4. Convert object-store services by moving current `Access::read` request logic
into `open(range)`.
5. Convert memory-like services by slicing in `read(range)`.
6. Convert handle based services first with independent per-range opens, then
@@ -509,9 +500,9 @@ The implementation should be considered complete when:
# Drawbacks
-- This is a breaking raw API change. Third-party `Service` implementations must
+- This is a breaking raw API change. Third-party `Access` implementations must
be updated.
-- It touches many services because every `Service::read` implementation
currently
+- It touches many services because every `Access::read` implementation
currently
receives a range.
- The rename from range-scoped `oio::Read` to `oio::ReadStream` is large but
necessary to avoid overloading one trait with two meanings.
@@ -534,7 +525,7 @@ progress, and request sizing into core and layers. OpenDAL
already has
`open(range)` for streaming and backpressure. The raw `read(range)` primitive
should instead express a complete, core-planned, bounded range read.
-## Add `Service::open_read_at`
+## Add `Access::open_read_at`
Rejected. It adds a parallel backend operation for an execution optimization.
OpenDAL's raw operation surface should stay centered on semantic operations:
@@ -561,10 +552,10 @@ ranges from it gives one stream two responsibilities and
makes layer wrapping
harder. The reusable raw reader should be the place that knows how to execute
additional ranges.
-## Add `ReadPlan` beside `Service::read`
+## Add `ReadPlan` beside `Access::read`
Rejected. It improves execution planning but keeps the old range-scoped
-`Service::read` as the primary contract. This RFC fixes the raw contract
instead
+`Access::read` as the primary contract. This RFC fixes the raw contract instead
of adding a second planning hook.
# Prior art
@@ -577,7 +568,7 @@ reduce extra `open` syscalls, and listed future
`read_ranges` and native
`read_at` support.
The current raw contract did not fully realize that intent because range
-selection stayed in `Service::read`. This RFC keeps RFC-4382's public direction
+selection stayed in `Access::read`. This RFC keeps RFC-4382's public direction
and moves the raw contract to the place where handle reuse and batch range
reads
can be implemented.
