Skip to content

branch-4.1:[improvement](file-cache) Enable file cache by default. - #66684

Merged
yiguolei merged 2 commits into
apache:branch-4.1from
hubgeter:worktree/apache-branch-4.1-20260812
Sep 15, 2026
Merged

yiguolei merged 2 commits into
apache:branch-4.1from
hubgeter:worktree/apache-branch-4.1-20260812

Conversation

@hubgeter

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Problem Summary:
Enable file cache by default.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hubgeter
hubgeter requested a review from yiguolei as a code owner August 12, 2026 08:29
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 100% (0/0) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/835) 🎉
Increment coverage report
Complete coverage report

@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 100% (0/0) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.48% (31455/42234)
Line Coverage 58.72% (350560/597044)
Region Coverage 55.45% (292596/527713)
Branch Coverage 56.30% (132031/234493)

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 100% (0/0) 🎉
Increment coverage report
Complete coverage report

@yiguolei
yiguolei merged commit d63f2c6 into apache:branch-4.1 Sep 15, 2026
33 of 35 checks passed
morningman added a commit to morningman/doris that referenced this pull request Oct 9, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#66684

Problem Summary: With `ignore_broken_disk=true`, `FileCacheFactory::create_file_caches`
skips every cache path that fails to initialize. When all of them are skipped it still
returns OK and leaves the factory with no cache instance. `get_by_path(hash)` then computes
`% _caches.size()`, so BE crashes at the first cached remote read, or earlier at the first
rowset cleanup: `Rowset::clear_cache()` looks up the cache for every removed rowset whenever
`enable_file_cache` is on.

Once `enable_file_cache` defaults to true, a storage-compute-coupled BE reaches this state
when its default cache directory `${DORIS_HOME}/file_cache` cannot be created (for example a
read-only install directory) and `ignore_broken_disk=true`. Storage and spill paths already
refuse to start when every path is broken ("All disks are broken, exit."). This makes
`create_file_caches` fail in the same situation, so startup stops with
"failed to init file cache ... All N file cache paths are broken" instead of crashing later.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - `run-be-ut.sh --run --filter='BlockFileCacheTest.create_file_caches*:BlockFileCacheTest.test_factory_*'`:
      5 tests passed, including the new
      `create_file_caches_rejects_a_factory_left_without_any_cache` (every path skipped is
      rejected; with one usable path left the broken one is still skipped)
- Behavior changed: Yes. A BE whose file cache paths are all broken now fails to start even
  with `ignore_broken_disk=true`; before, it started and crashed on first use of the cache.
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit to morningman/doris that referenced this pull request Oct 9, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#62023, apache#66684

Problem Summary: Since apache#62023 moved TVF file listing onto the filesystem SPI,
`ExternalFileTableValuedFunction#parseFile` builds each `TBrokerFileStatus` from a
`FileEntry` without its modification time, although the S3, HDFS, Azure and local listings
all fill `FileEntry#modificationTime`. Every `s3()` / `hdfs()` TVF range therefore reaches
BE with mtime 0.

BE keys cached blocks of external files by `path:mtime`. With the file cache on, which
becomes the default, an object overwritten in place under the same key keeps serving the
blocks cached from its old content. branch-4.x does not have this problem: its listing goes
through `BrokerUtil.parseFile`, which copies the listed modification time. This sets the
modification time again.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - `run-fe-ut.sh --run` on `ExternalFileTableValuedFunctionTest` (new
      `testListedFilesCarryTheirModificationTime`), `TVFScanNodeTest`,
      `SessionVariablesTest`, `VariableMgrTest`: 40 tests, 0 failures
    - `mvn checkstyle:check` on fe-core: 0 violations
- Behavior changed: No (TVF ranges carry the listed modification time, as on branch-4.x)
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit to morningman/doris that referenced this pull request Oct 9, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#66684

Problem Summary: `ExecEnv::init_file_cache_factory` evaluated
`s3_write_buffer_size % file_cache_each_block_size` before
`validate_hdfs_write_batch_buffer_size` rejected a nonpositive block size. With
`file_cache_each_block_size = 0` the modulo divides by zero, so on x86-64 BE stops with
SIGFPE instead of reporting the invalid setting. Once `enable_file_cache` defaults to true,
a BE that kept the cache disabled with such a value reaches this check on upgrade.

Run the HDFS batch-size validation first: it already rejects a nonpositive block size with
"file_cache_each_block_size 0 must be positive". Valid configurations take the same checks
as before, only in a different order.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - `HdfsFileSystemTest.RejectsCacheBlockSizeNotDividingHdfsBatch` adds the zero and
      negative block sizes. Compile-checked locally; the BE UT run is left to CI because the
      local UT build would need a full rebuild.
- Behavior changed: No
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman pushed a commit to apache/doris-website that referenced this pull request Oct 10, 2026
…4202)

### What problem does this PR solve?

Apache Doris enables the block file cache by default since 4.2: both the
BE config `enable_file_cache` and the session variable
`enable_file_cache` default to `true` (apache/doris#66684,
apache/doris#68797). The docs still described the old behavior (`false`
for non-cloud mode, and "disabled by default" for Data Cache).

### What is changed?

- `enable_file_cache` default updated from `false` to `true` in the BE
config reference, the Data Cache page, and the compute-storage-decoupled
File Cache page.
- Clarified that an external file scan reads through the cache only when
the BE config, the session variable, and cache admission control are all
enabled; the session variable has no effect on internal tables.
- Added caveats for the new default: the default cache path
`${DORIS_HOME}/file_cache` may grow until the disk is 85%–88% full
(bound it with `total_size`, or set `enable_file_cache = false` to keep
the old behavior); BE configs are not persisted while session variables
are persisted in the FE image, so a cluster upgraded from an older
version keeps its previous value and needs `SET GLOBAL enable_file_cache
= true` to cache external tables.
- Applied to `docs/` (dev) and `versioned_docs/version-4.x/`, English
and Chinese.

## Versions 

- [x] dev
- [x] 4.x
- [ ] 3.x
- [ ] 2.1 or older (not covered by version/language sync gate)

## Languages

- [x] Chinese
- [x] English

## Docs Checklist

- [x] Checked by AI
- [ ] Test Cases Built
- [x] Updated required version and language counterparts, or explained
why not
- [x] If only one language changed, confirmed whether source/translation
counterparts need sync
morningman added a commit to morningman/doris that referenced this pull request Oct 12, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#66684

Problem Summary:

branch-4.1 (apache#66684, from 4.1.5) and branch-4.2 enable the block file
cache by default: both the BE config `enable_file_cache` and the session
variable `enable_file_cache` default to true there. master still
defaults both to false.

The two defaults behave differently across upgrades. FE persists every
session variable value in its image, so an upgraded cluster keeps its
value whatever the new default is. BE configs are not persisted.
Consider a storage-compute-coupled BE whose be.conf does not set
`enable_file_cache`: upgraded from 4.1.5 / 4.2 to a release built from
master, it silently loses its file cache. External table scans and
reads of rowsets cooled down to remote storage then go back to remote
storage on every query. Cloud mode forces the BE config on, so it is
not affected.

Port apache#66684 and default both to true. In coupled mode this:
- reads external file scans and cooled-down rowsets through the file
  cache; local rowsets are read as before;
- assigns external scan ranges to backends by consistent hashing instead
  of round robin, for cache affinity;
- on a BE without `file_cache_path`, caches under
  `${DORIS_HOME}/file_cache`, sized to the whole file system and evicted
  once it is 88% full.

### Release note

The block file cache is now enabled by default: the BE config
`enable_file_cache` and the session variable `enable_file_cache` default
to true. On a storage-compute-coupled BE without `file_cache_path`, the
cache is created under `${DORIS_HOME}/file_cache` and may grow until the
disk is 85%-88% full. To bound it, set `file_cache_path` with a
`total_size`; to keep the old behavior, set `enable_file_cache = false`.
Clusters upgraded from older versions keep their persisted session
variable value; run `SET GLOBAL enable_file_cache = true` to read
external tables through the cache.

### Check List (For Author)

- Test: Unit Test
    - FE UT: `run-fe-ut.sh --run org.apache.doris.qe.SessionVariablesTest,org.apache.doris.qe.VariableMgrTest`, 32 tests, 0 failures.
    - BE UT and regression pipelines are left to CI.
- Behavior changed: Yes. Both defaults change from false to true.
- Does this need documentation: Yes. The documented defaults change; a doris-website PR will follow.

Co-authored-by: daidai <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit to morningman/doris that referenced this pull request Oct 12, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#66684

Problem Summary: With `ignore_broken_disk=true`, `FileCacheFactory::create_file_caches`
skips every cache path that fails to initialize. When all of them are skipped it still
returns OK and leaves the factory with no cache instance. `get_by_path(hash)` then computes
`% _caches.size()`, so BE crashes at the first cached remote read, or earlier at the first
rowset cleanup: `Rowset::clear_cache()` looks up the cache for every removed rowset whenever
`enable_file_cache` is on.

Once `enable_file_cache` defaults to true, a storage-compute-coupled BE reaches this state
when its default cache directory `${DORIS_HOME}/file_cache` cannot be created (for example a
read-only install directory) and `ignore_broken_disk=true`. Storage and spill paths already
refuse to start when every path is broken ("All disks are broken, exit."). This makes
`create_file_caches` fail in the same situation, so startup stops with
"failed to init file cache ... All N file cache paths are broken" instead of crashing later.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - `run-be-ut.sh --run --filter='BlockFileCacheTest.create_file_caches*:BlockFileCacheTest.test_factory_*'`:
      5 tests passed, including the new
      `create_file_caches_rejects_a_factory_left_without_any_cache` (every path skipped is
      rejected; with one usable path left the broken one is still skipped)
- Behavior changed: Yes. A BE whose file cache paths are all broken now fails to start even
  with `ignore_broken_disk=true`; before, it started and crashed on first use of the cache.
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit to morningman/doris that referenced this pull request Oct 12, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#62023, apache#66684

Problem Summary: Since apache#62023 moved TVF file listing onto the filesystem SPI,
`ExternalFileTableValuedFunction#parseFile` builds each `TBrokerFileStatus` from a
`FileEntry` without its modification time, although the S3, HDFS, Azure and local listings
all fill `FileEntry#modificationTime`. Every `s3()` / `hdfs()` TVF range therefore reaches
BE with mtime 0.

BE keys cached blocks of external files by `path:mtime`. With the file cache on, which
becomes the default, an object overwritten in place under the same key keeps serving the
blocks cached from its old content. branch-4.x does not have this problem: its listing goes
through `BrokerUtil.parseFile`, which copies the listed modification time. This sets the
modification time again.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - `run-fe-ut.sh --run` on `ExternalFileTableValuedFunctionTest` (new
      `testListedFilesCarryTheirModificationTime`), `TVFScanNodeTest`,
      `SessionVariablesTest`, `VariableMgrTest`: 40 tests, 0 failures
    - `mvn checkstyle:check` on fe-core: 0 violations
- Behavior changed: No (TVF ranges carry the listed modification time, as on branch-4.x)
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit to morningman/doris that referenced this pull request Oct 12, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#66684

Problem Summary: `ExecEnv::init_file_cache_factory` evaluated
`s3_write_buffer_size % file_cache_each_block_size` before
`validate_hdfs_write_batch_buffer_size` rejected a nonpositive block size. With
`file_cache_each_block_size = 0` the modulo divides by zero, so on x86-64 BE stops with
SIGFPE instead of reporting the invalid setting. Once `enable_file_cache` defaults to true,
a BE that kept the cache disabled with such a value reaches this check on upgrade.

Run the HDFS batch-size validation first: it already rejects a nonpositive block size with
"file_cache_each_block_size 0 must be positive". Valid configurations take the same checks
as before, only in a different order.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - `HdfsFileSystemTest.RejectsCacheBlockSizeNotDividingHdfsBatch` adds the zero and
      negative block sizes. Compile-checked locally; the BE UT run is left to CI because the
      local UT build would need a full rebuild.
- Behavior changed: No
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
morningman added a commit that referenced this pull request Oct 12, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: #66684

Original author: @hubgeter (the change is ported from #66684 on
branch-4.1; the commit carries Co-authored-by)

Problem Summary:

**In short.** branch-4.1 (from 4.1.5) and branch-4.2 enable the block
file cache by default. master still has it off. This difference matters
most on upgrade, through the BE config: BE configs are not persisted. A
storage-compute-coupled cluster that relies on the default therefore
loses its file cache when it moves from 4.1.5 / 4.2 to a release built
from master. This PR ports #66684 and makes both the BE config
`enable_file_cache` and the session variable `enable_file_cache` default
to true.

**Background**

- Two switches gate the block file cache:
- The BE config `enable_file_cache` decides whether a BE creates the
cache at all. In cloud mode, BE forces it on after reading its config
file and refuses to start without it.
- The session variable `enable_file_cache` decides whether external file
scans read through the cache. `FileFactory::get_reader_options` requires
the BE config, the session variable and file cache admission to all be
true. The session variable has no effect on internal tables.
- With the session variable on, FE also assigns external scan ranges by
consistent hashing instead of round robin (`ExternalScanNode`). A file
then keeps landing on the same backends, so its cached blocks get
reused.
- Session variables and BE configs behave differently across upgrades:
- FE writes every session variable value into its image
(`VariableMgr.write` → `SessionVariable.toJson`). An upgraded cluster
keeps the values it had whatever the new default is; only a new cluster
gets a new default.
- BE configs are not persisted. A BE uses the default compiled into its
binary unless be.conf sets the key.

**The problem, and what it cost**

| Default | master | branch-4.2 | 4.1.4 | 4.1.5 |
|---|---|---|---|---|
| BE config `enable_file_cache` | false | true | false | true |
| session variable `enable_file_cache` | false | true | false | true |

- Take a storage-compute-coupled cluster whose be.conf does not set
`enable_file_cache`. When it is upgraded from 4.1.5 / 4.2 to a release
built from master, it silently loses its file cache. External table
scans and reads of rowsets cooled down to remote storage go back to
remote storage on every query. The `${DORIS_HOME}/file_cache` directory
is left on disk.
- New clusters built from master behave differently from new 4.1.5 / 4.2
clusters. In coupled mode they have no file cache at all, and in either
mode external scans are not cached.
- Cloud clusters are not affected by the BE default, because cloud mode
forces it on. Upgraded clusters keep their session variable value either
way.

**How this PR fixes it**

The first commit makes both defaults true, as in #66684:
`DEFINE_Bool(enable_file_cache, "true")` in `be/src/common/config.cpp`
and `enableFileCache = true` in `SessionVariable`.

On a storage-compute-coupled BE that does not configure the cache, this
turns on the following:

- The cache lives under `${DORIS_HOME}/file_cache` (the default
`file_cache_path`). Without `total_size` it may use the whole file
system. Eviction starts once the file system is 88% full and stops at
85% (`file_cache_enter/exit_need_evict_cache_in_advance_percent`).
`storage_root_path` defaults to the same disk, whose flood stage is 90%,
so a busy cache keeps a shared disk at 85%–88%. To bound it, set
`file_cache_path` with a `total_size`; to keep the old behavior, set
`enable_file_cache = false` in be.conf.
- Only reads through a remote file system go through the cache: external
file scans and cooled-down rowsets. Local rowsets are read exactly as
before.

The second commit keeps out one side effect of the new default.
`SniiCompoundWriter` pads an SNII inverted index container of 32 or more
file cache blocks with zeros, so that it ends on a block boundary. Only
`CachedRemoteFileReader` repays that padding: a read confined to the
last block then fetches one block instead of two. The padding was gated
on `enable_file_cache`, which stood for "cloud mode" as long as the flag
defaulted to false (cloud mode forces it on). With the new default, a
coupled BE would also pad its SNII containers on local disk, by up to
half a block each. There `LocalFileReader` reads them and the padding is
never repaid; branch-4.2 does this today. The padding is now gated on
cloud mode, and its UTs toggle the deploy mode instead of the file
cache.

Two more commits close gaps that the new default makes reachable:

- The third commit refuses to start a BE that has no usable file cache
path. With `ignore_broken_disk=true`,
`FileCacheFactory::create_file_caches` skipped every path that failed to
initialize and still returned OK when all of them were skipped.
`get_by_path(hash)` then took `% 0` and crashed BE at the first cached
read, or earlier at the first rowset cleanup, since
`Rowset::clear_cache()` looks up the cache for every removed rowset
while `enable_file_cache` is on. A coupled BE gets there when
`${DORIS_HOME}/file_cache` cannot be created. `create_file_caches` now
fails in that case, the way storage and spill paths already do ("All
disks are broken, exit.").
- The fourth commit restores the modification time of files listed by
`s3()` / `hdfs()` TVFs. Since #62023,
`ExternalFileTableValuedFunction#parseFile` built `TBrokerFileStatus`
without `FileEntry#modificationTime`, so every TVF range reached BE with
mtime 0. BE keys cached blocks of external files by `path:mtime`, so
with the cache on, an object overwritten in place would keep serving its
old cached blocks. branch-4.x copies the listed mtime
(`BrokerUtil.parseFile`); master does again.

**Results**

| Scenario | Before | After |
|---|---|---|
| New coupled cluster, external table scan | not cached, scan ranges by
round robin | cached, scan ranges by consistent hashing |
| New cloud cluster, external table scan | not cached | cached |
| Coupled cluster upgraded from 4.1.5 / 4.2, be.conf without
`enable_file_cache` | file cache turned off by the upgrade | file cache
stays on |
| Coupled cluster upgraded from 4.1.4 or older | unchanged | BE cache on
(cooled-down rowsets cached). The session variable keeps its persisted
`false`, so external scans stay uncached until `SET GLOBAL
enable_file_cache = true` |
| Internal tables on local disks | unchanged | unchanged |
| SNII index containers, coupled BE | padded only if the BE had turned
the cache on | not padded |
| SNII index containers, cloud mode | padded | padded (unchanged) |
| Every file cache path broken, `ignore_broken_disk=true` | BE starts,
then crashes on first use of the cache | BE refuses to start: "All N
file cache paths are broken" |
| `s3()` / `hdfs()` TVF, object overwritten in place, cache on | could
serve blocks cached from the old content (mtime 0) | the new mtime is
part of the cache key |

Co-authored-by: daidai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants