JMLX42 opened a new issue, #7852:
URL: https://github.com/apache/opendal/issues/7852

   ### What happens
   
   In the sqlite service, `stat()` computes `content_length` by fetching the 
**entire value**:
   
   
https://github.com/apache/opendal/blob/5d84f2b34261d38f6c59254fc9e561f5941fb6ee/core/services/sqlite/src/backend.rs#L225-L235
   
   `self.core.get(&p)` runs `SELECT value FROM table WHERE key = ?`, 
materializes the whole blob into memory, and then only uses `bs.len()`.
   
   The efficient query already exists right next to it — 
`SqliteCore::get_length` does `SELECT LENGTH(value) ...`, which SQLite answers 
from the record header without loading the blob:
   
   
https://github.com/apache/opendal/blob/5d84f2b34261d38f6c59254fc9e561f5941fb6ee/core/services/sqlite/src/core.rs#L63-L83
   
   but `stat()` does not use it. (The range-read path was already fixed to push 
down via `SUBSTR` in #5701 / `get_range`; `stat` is the remaining whole-value 
read.)
   
   ### Why it's a footgun
   
   Any workflow that stats before reading pays a full-value read per stat. In 
our application (a glTF asset server using the sqlite service as blob storage), 
an HTTP range-download handler performed one `op.stat()` (integrity/length 
check) + one `reader.read(range)` per request. With a 66 MB stored value and ~3 
MiB ranges, every request silently re-read the entire 66 MB out of SQLite just 
to learn its length — invisible in code review because `stat()` reads like a 
cheap metadata call, which it is on fs/s3.
   
   ### Measured impact
   
   Methodology: 40 sequential HTTP range requests (3 MiB each, ascending 
offsets) against the same 66 MB value in a single-file sqlite backend, warm OS 
page cache, release build, timed at the HTTP client. Before/after removing the 
per-request `stat` (length taken from our own DB row instead; the first 
`read(range)` doubles as the existence check):
   
   | | median / request | p90 | 40-request total |
   |---|---|---|---|
   | with per-request `stat` | 99 ms | 110 ms | 4.36 s |
   | without | 30 ms | 33 ms | 1.49 s |
   
   ~3.3x per request. (Caveat: our patch also stopped rebuilding the `Operator` 
per request, so a small share of the delta is pool reuse — but the dominant 
cost was the 66 MB `SELECT` per stat, ~2.6 GB of redundant reads across those 
40 requests.)
   
   ### Suggested fix
   
   Have `stat()` call the existing `get_length()` instead of `get()`:
   
   ```rust
   let length = self.core.get_length(&p).await?;
   match length {
       Some(len) => Ok(RpStat::new(
           Metadata::new(EntryMode::from_path(&p)).with_content_length(len as 
u64),
       )),
       None => { /* existing directory-prefix fallback */ }
   }
   ```
   
   The `None` / directory-prefix fallback is unaffected. Alternatively (or 
additionally), size and other metadata could be persisted as columns alongside 
the value at write time — that generalizes to richer metadata (`last_modified`, 
etag) — but for `content_length` alone the `LENGTH()` one-liner seems 
sufficient and schema-compatible.
   
   Happy to send a PR if the `get_length` approach sounds right.
   
   ### Disclosure
   
   This investigation, the measurements, and this issue text were produced by 
an AI assistant (Claude, by Anthropic) working under my direction on our 
downstream project; I reviewed the findings before filing.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to