You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "WenyXu (via GitHub)" <gi...@apache.org> on 2023/12/12 09:31:05 UTC
[I] idea: tailing buffering [incubator-opendal]
WenyXu opened a new issue, #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745
Many modern file formats, such as Parquet and ORC (Optimized Row Columnar), utilize a method called "footer" to store metadata.
Such as:
https://github.com/datafusion-contrib/datafusion-orc/blob/f5a5b21148d2787ff1cd34996b7cf6dadfd643e9/src/reader/metadata.rs#L128-L163
https://github.com/apache/arrow-rs/blob/8aa55ddc1cce8893d0ea09029b149f0239060646/parquet/src/arrow/async_reader/mod.rs#L180-L199
In typical scenarios, the reader initially retrieves the `metadata_len` followed by the `metadata`. In the case of a backend like S3, buffering slightly more during the first read operation can help circumvent the need for additional requests (The latency difference between reading 1KiB and 32KiB from S3 can be slight).
See also #3734
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [incubator-opendal]
Posted by "WenyXu (via GitHub)" <gi...@apache.org>.
WenyXu commented on issue #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745#issuecomment-1852040180
> Seems it's not related to buffer.
>
> ```rust
> let assume_footer_len = file_len.min(DEFAULT_FOOTER_SIZE);
> ```
>
> It's a prediction from the application side (we know how large the footer will be). The root cause from my side is more like this issue: #3747
I got it; my mistake. I thought it was already implemented. Once we introduce the feature mentioned in #3747, adding a tailing buffer for this scenario will be straightforward.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745#issuecomment-1853144170
> However, if we enable the tailing buffer feature, it only requires one remote fetch in most cases.
After this reader has been configured to tailing buffer, what will happen it seek to the start to read the real data? For example:
```rust
let r = op.reader(path).tailing(1024 * 16).await?;
r.seek(SeekFrom::Start(1024)).await?; <-- we will buffer data in [0..1024)
r.read(&mut dst).await?;
r.seek(SeekFrom::Start(2048)).await?; <-- we will buffer data in [0..2048)
r.read(&mut dst).await?;
```
It looks we are building a specified reader for memtadata reading. Why not just:
```rust
let assume_footer_len = file_len.min(DEFAULT_FOOTER_SIZE);
// Reads the metadata's len
r.seek(SeekFrom::End(-assume_footer_len)).await?;
r.read(&mut dst).await?;
// perform an extra read if we need it.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [incubator-opendal]
Posted by "WenyXu (via GitHub)" <gi...@apache.org>.
WenyXu commented on issue #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745#issuecomment-1853197509
> > However, if we enable the tailing buffer feature, it only requires one remote fetch in most cases.
>
> After this reader has been configured to tailing buffer, what will happen it seek to the start to read the real data? For example:
>
> ```rust
> let r = op.reader(path).tailing(1024 * 16).await?;
>
> r.seek(SeekFrom::Start(1024)).await?; <-- we will buffer data in [0..1024)
> r.read(&mut dst).await?;
>
> r.seek(SeekFrom::Start(2048)).await?; <-- we will buffer data in [0..2048)
> r.read(&mut dst).await?;
> ```
We can put the tailing buffer into a separate buffer.
i.e.,
```rust
struct FooReader{
buf: Buffer,
tailing: Buffer
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [incubator-opendal]
Posted by "WenyXu (via GitHub)" <gi...@apache.org>.
WenyXu commented on issue #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745#issuecomment-1853195355
> It looks we are building a specified reader for memtadata reading. Why not just:
Yes, we can do this.
However, I think there are two major reasons:
1. **The file format reader doesn't allow users to read the metadata separately.** Not all readers like `ParquertAsyncReader` allow us to read the metadata separately. BTW, during the implementation of [datafusion-orc](https://github.com/datafusion-contrib/datafusion-orc), I didn't notice this point that we should allow users to read metadata separately.
2. Putting `metadata` in the footer of the file is a common approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745#issuecomment-1852027723
Seems it's not related to buffer.
```rust
let assume_footer_len = file_len.min(DEFAULT_FOOTER_SIZE);
```
It's a prediction from the application side (we know how large the footer will be). The root cause from my side is more like this issue: https://github.com/apache/incubator-opendal/issues/3747
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed issue #3745: idea: tailing buffering
URL: https://github.com/apache/opendal/issues/3745
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #3745:
URL: https://github.com/apache/opendal/issues/3745#issuecomment-2047976164
OpenDAL has transitioned to range-based reading. I believe this issue is no longer relevant. Please feel free to reopen it if you think it's still pertinent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] idea: tailing buffering [incubator-opendal]
Posted by "WenyXu (via GitHub)" <gi...@apache.org>.
WenyXu commented on issue #3745:
URL: https://github.com/apache/incubator-opendal/issues/3745#issuecomment-1852141261
> Seems it's not related to buffer.
And one more thing I need to add:
The tailing buffer is designed for the following scenario:
```rust
let r = op.reader(path).await?;
// Reads the metadata's len
r.seek(SeekFrom::End(-8)).await?;
r.read(&mut dst).await?; // <---- Fetches and buffers range(-8, end)
// Reads the metadata
r.seek(SeekFrom::End(-8192)).await?;
r.read(&mut dst).await?; // <---- Fetches and buffers range(-8192, end)
```
However, if we enable the tailing buffer feature:
```rust
let r = op.reader(path).tailing(1024 * 16).await?;
// Reads the metadata's len
r.seek(SeekFrom::End(-8)).await?;
r.read(&mut dst).await?; // <---- Fetches and buffers range(-16*1024, end)
// Reads the metadata
r.seek(SeekFrom::End(-8192)).await?;
r.read(&mut dst).await?; // <---- Reads from buffer
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org