Repository navigation
branch-4.1:[improvement](file-cache) Enable file cache by default. - #66684
Merged
yiguolei merged 2 commits intoSep 15, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
yiguolei
approved these changes
Sep 15, 2026
3 of 16 tasks
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]>
Merged
7 of 10 tasks
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]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Problem Summary:
Enable file cache by default.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)