apache / apache/opendal

bug: FoyerLayer ignores read args while filling full-object cache

Open
#7,686 2 comments 1 reaction 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
5.4k
Forks
825
Avg merge
1d 14m
Merged PRs (30d)
127

Description

## Describe the bug

`FoyerLayer` can ignore `OpRead` / `OpStat` arguments while filling the full-object cache.

In `core/layers/foyer/src/full.rs`, `FullReader::fallback_open` and `fallback_read` correctly call the inner accessor with `self.args.clone()`. However, `read_full_object()` fills the cache by calling:

- `stat(&path_clone, OpStat::default())`
- `read(&path_clone, OpRead::default())`

The cache key only includes `path` and `version`, derived from `self.args.version()`.

This can make the cache-fill path observe different read conditions than the original reader request.

## Steps to Reproduce

1. Enable `FoyerLayer`.
2. Read an object through `reader_with(path)` or `read_with(path)` with read arguments such as `version`, `if_match`, `if_none_match`, or other conditional read options.
3. Trigger a full-object cache miss so `read_full_object()` fills the cache.
4. Observe that the cache-fill path uses default read/stat args instead of the original request args.

Relevant code:

- `fallback_open` uses `self.args.clone()`: `core/layers/foyer/src/full.rs`
- `fallback_read` uses `self.args.clone()`: `core/layers/foyer/src/full.rs`
- cache-fill uses `OpStat::default()` and `OpRead::default()`: `core/layers/foyer/src/full.rs`
- cache key only includes `path` and `version`: `core/layers/foyer/src/full.rs`

## Expected Behavior

The cache-fill path should preserve the semantics of the original read request.

At minimum:

- `stat` / `read` used for cache fill should be derived from `self.args`.
- Versioned reads should not accidentally fetch latest content and store it under a versioned cache key.
- Conditional reads such as `if_match` / `if_none_match` should either be respected during cache fill or explicitly bypass the cache after validating the condition.

## Additional Context

This was found during post-merge review of the read-reader refactor. The new design makes `Access::read` return a reusable raw reader, so layers need to preserve `OpRead` semantics across both reader creation and later reader-side IO.

Suggested tests:

- `FoyerLayer + reader_with(path).version(v)` cache miss/hit behavior.
- `FoyerLayer + read_with(path).if_match(...)` cache miss/hit behavior.
- Ensure fallback and cache-fill paths observe the same read arguments.

## Are you willing to submit a PR to fix this bug?

- [ ] Yes, I would like to submit a PR.

Contributor guide

Open the contributing guide

Research direction

Start in core/layers/foyer/src/full.rs and trace FullReader::fallback_open, fallback_read, and read_full_object(). Compare how self.args is passed on fallback paths with the default arguments used during cache filling, then inspect the cache key's path and version inputs. Add or run FoyerLayer tests covering versioned and conditional cache-miss/hit behavior; done means cache filling preserves the original read semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.