liaoxin01 opened a new pull request, #69693:
URL: https://github.com/apache/doris/pull/69693
### What problem does this PR solve?
Issue Number: None
Related PR: #59892, #60411, #64995
Problem Summary:
When packed file is enabled, the first segment file (`.dat`) and the index
file (`.idx`) of a rowset are written through `PackedFileSystem`. The packed
path decides whether to cache each small file from
`PackedAppendContext::write_file_cache`, which defaults to `true`.
`RowsetWriterContext` never sets this field.
`PackedFileSystem::create_file_impl` only copied the caller's
`FileWriterOptions::write_file_cache` into the append context when
`enable_file_cache_write_index_file_only` was on. With the default config, the
packed path ignored the caller's choice and always wrote the small files to the
NORMAL file cache queue.
So a load with `disable_file_cache=true` still filled the NORMAL queue
whenever packed file was enabled. Files written directly to S3 honor the same
flag and skip the cache. `test_load_cache` turns `enable_packed_file` on or off
at random, so it fails intermittently with:
```
Scenario 1: Normal queue should be 0 bytes, but got 237288 bytes
```
The 237288 bytes are exactly the total size of the three packed segment
files written by the load.
This PR always copies `opts->write_file_cache` into the append context when
the caller passes options. Packed and direct writes now make the same cache
decision. When `opts` is null, the packed path keeps the `write_file_cache`
value already in the append context.
Other callers are not affected:
- Rowset files always get their options from
`RowsetWriterContext::get_file_writer_options`, so with write-index-only
enabled the result is the same as before.
- Compaction and schema change set `allow_packed_file=false`, so adaptive
file cache write is not affected.
- `DeleteBitmapFileWriter` builds `PackedFileWriter` directly, so it is not
affected.
### Release note
Fix loads with `disable_file_cache=true` still writing data to the file
cache when packed file is enabled.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. With packed file enabled, writes whose options set
`write_file_cache=false` (for example a load with `disable_file_cache=true`) no
longer populate the file cache.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
