New Block Cache Changeset#2172
Open
syeleti-msft wants to merge 42 commits into
Open
Conversation
syeleti-msft
requested review from
bhansalivikas,
jainakanksha-msft and
souravgupta-msft
as code owners
March 19, 2026 05:16
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new block-cache changeset and expands the integrity/stress testing coverage (dd/fio/fsx + scenario loops), while also adjusting loopback behavior to support block-list semantics and updating CI pipelines to run the new scenarios.
Changes:
- Add a new
component/block_cacheimplementation (buffer pool, freelist eviction, async worker pool, access-pattern detection) plus unit tests. - Extend data-integrity tooling and CI (new dd/fio scripts, new fio job files, scenario loops, fsx install/run).
- Update loopback component to maintain committed block lists in-memory and adjust file lifecycle ops; add/extend scenario tests.
Reviewed changes
Copilot reviewed 58 out of 62 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/config/config_bc_loopback.yaml | Adds loopback-backed block-cache stress config for pipeline scenarios |
| test/scripts/extended_dd.sh | Adds extended dd-based integrity/stress scenarios including concurrency |
| test/scripts/dd.sh | Adds basic dd-based read/write integrity script used by pipelines |
| test/scenarios/write_test.go | Adds append + sparse/hole-related scenario tests |
| test/scenarios/truncate_test.go | Adds additional truncate/ftruncate cases |
| test/scenarios/read_write_test.go | Ensures file handles are closed after read/write test loop |
| test/scenarios/init_test.go | Improves integrity-check assertion messages with mountpoint/file context |
| test/fio/verify-sharedfile-no-overlap.fio | Adds fio verify workload definition |
| test/fio/verify-mixed.fio | Adds fio mixed verify workload definition |
| test/fio/verify-basic.fio | Adds fio basic verify workload definition |
| test/fio/seq-write-1f-10th.fio | Enables direct=1 for sequential write fio test |
| test/fio/hole_over_blocks.fio | Enables direct=1 for hole-over-blocks fio test |
| test/fio/hole_inside_blocks.fio | Enables direct=1 for hole-inside-blocks fio test |
| test/fio/fio_fuse_integrity.sh | Adds a scripted fio integrity suite wrapper |
| internal/handlemap/handle_map.go | Extends Handle with IFObj for interface-specific per-handle state |
| go.mod | Adds github.com/prometheus/procfs indirect dependency |
| go.sum | Adds checksums for github.com/prometheus/procfs |
| component/loopback/util.go | Adds helper to remove staged block files by prefix |
| component/loopback/loopback_fs_test.go | Extends loopback tests for committed block-list persistence and lifecycle ops |
| component/loopback/loopback_fs.go | Adds in-memory committed block-list tracking; changes Delete/Rename/Truncate behavior; adjusts GetAttr error return |
| component/libfuse/libfuse.go | Changes default for disable-writeback-cache flag |
| component/block_cache_old/threadpool_test.go | Renames package to block_cache_old |
| component/block_cache_old/threadpool.go | Renames package to block_cache_old |
| component/block_cache_old/stream.go | Renames package to block_cache_old |
| component/block_cache_old/blockpool_test.go | Renames package to block_cache_old |
| component/block_cache_old/blockpool.go | Renames package to block_cache_old |
| component/block_cache_old/block_test.go | Adds/keeps old block-cache tests under block_cache_old |
| component/block_cache_old/block.go | Adds/keeps old block-cache block implementation under block_cache_old |
| component/block_cache/worker.go | Adds async worker pool for block upload/download |
| component/block_cache/test_helpers_test.go | Adds shared test helpers for freelist/worker-pool setup |
| component/block_cache/pattern_detector_test.go | Adds unit tests for access-pattern detection heuristic |
| component/block_cache/pattern_detector.go | Adds access-pattern detector (sequential vs random) |
| component/block_cache/info.txt | Adds local dev benchmark notes/results for block cache |
| component/block_cache/handle_test.go | Adds tests for open-handle/file-map lifecycle and buffer release |
| component/block_cache/handle.go | Adds handle-to-file mapping + cleanup logic for last-handle-close |
| component/block_cache/freelist_test.go | Adds tests for freelist allocation/release and victim selection |
| component/block_cache/freelist.go | Adds freelist + eviction policy and async buffer reset mechanism |
| component/block_cache/buffer_pool_test.go | Adds tests for buffer pool behavior and invariants |
| component/block_cache/buffer_pool.go | Adds buffer pool implementation with bounded allocation |
| component/block_cache/buffer_mgr_test.go | Adds tests for buffer table manager behavior |
| component/block_cache/buffer_mgr.go | Adds buffer table manager (lookup/allocate/evict/remove) |
| component/block_cache/buffer_descriptor_test.go | Adds tests for buffer descriptor lifecycle, reset, refcounting |
| component/block_cache/buffer_descriptor.go | Adds buffer descriptor (refcount, dirty/valid, sync) |
| component/block_cache/block_test.go | Reworks block-cache block tests for the new implementation |
| component/block_cache/block.go | Adds new block model (states, validation, scheduling upload/download) |
| common/util.go | Adds GetAvailableMemoryInMB() using procfs |
| common/types.go | Adds BlockIDLenghtBase64 constant |
| cmd/mountv1_test.go | Switches v1 stream options import to block_cache_old |
| cmd/mountv1.go | Switches v1 stream options import to block_cache_old |
| cmd/imports.go | Registers both new block_cache and legacy block_cache_old components |
| blobfuse2-release.yaml | Comments out fuse2 release matrix entry |
| blobfuse2-nightly.yaml | Re-enables block-cache scenario + RMAN templates (previously commented) |
| azure-pipeline-templates/scenario.yml | Adds block-cache loopback stress config + repeated scenario runs; adds fsx install/run; adds trace dump on failure |
| azure-pipeline-templates/fio-data-integrity.yml | Adds fio integrity script + new verify job runs; adds logs/trace on failure |
| azure-pipeline-templates/data.yml | Extends data generation to include sparse files and hole-at-end files |
| azure-pipeline-templates/data-integrity.yml | Adds dd and extended-dd tests into integrity pipeline stages; tweaks artifact publishing and failure diagnostics |
You can also share your feedback on Copilot code review. Take the survey.
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
4 times, most recently
from
March 21, 2026 14:41
85841e0 to
f520694
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
7 times, most recently
from
March 29, 2026 13:40
cdc5023 to
238e463
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
from
March 31, 2026 10:05
238e463 to
67f6e0a
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
2 times, most recently
from
April 9, 2026 07:24
b34ad5e to
457de89
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
2 times, most recently
from
April 24, 2026 09:11
d9c0f26 to
5a80dfa
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
from
May 7, 2026 04:38
5a80dfa to
6cb4238
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
3 times, most recently
from
May 18, 2026 08:53
5d6443f to
f3e26a2
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
from
June 22, 2026 09:12
424ee6e to
ef06f6e
Compare
syeleti-msft
force-pushed
the
syeleti/bc-first
branch
from
July 10, 2026 04:47
bf6ca09 to
8448205
Compare
Comment on lines
+206
to
+212
| func validateBlockList(blkList *internal.CommittedBlockList, f *file, blockSize uint64) error { | ||
| if blkList == nil || len(*blkList) == 0 { | ||
| return ErrInvalidBlockList | ||
| } | ||
| listLen := len(*blkList) | ||
| newblkList := make([]*block, 0, listLen) | ||
|
|
Comment on lines
+86
to
+89
| // This read should be served from the newly created file | ||
| _, err = file2.Read(readContent) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, content2, readContent) |
Comment on lines
+147
to
+150
| // This read should be served from the newly created file | ||
| _, err = file2.Read(readContent) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, content2, readContent) |
Comment on lines
+606
to
+611
| // Store the committed block list for this file | ||
| if len(committedList) > 0 { | ||
| lfs.blockLists.Store(options.Name, &committedList) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+316
to
+327
| err := os.Rename(oldPath, newPath) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Move the block list from old name to new name | ||
| if value, ok := lfs.blockLists.Load(options.Src); ok { | ||
| lfs.blockLists.Store(options.Dst, value) | ||
| lfs.blockLists.Delete(options.Src) | ||
| } | ||
|
|
||
| return nil |
| expectedContent = expectedContent[:finalSize] | ||
| } | ||
| assert.Equal(t, string(expectedContent), string(readContent)) | ||
| assert.Equal(t, string(expectedContent), string(readContent), filePath) |
Comment on lines
+141
to
+145
| // Note: Pending tasks in the channel are abandoned. Callers should ensure | ||
| // all important tasks complete before destroying the pool. | ||
| func (wp *workerPool) destroy() { | ||
| close(wp.close) | ||
| wp.wg.Wait() |
Comment on lines
+184
to
+188
| pids="" | ||
| for i in {1..50}; do | ||
| run_integrity_test "conc_job_$i" "1M" "100" & | ||
| pids="$pids $!" | ||
| done |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 59 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
component/loopback/loopback_fs.go:595
- CommitData no longer deletes the staged block files written by StageData ("_"). Over time this can accumulate orphan files on disk even after successful commits, increasing disk usage and potentially impacting test reliability.
Consider removing each staged block file after it has been successfully copied into the main file.
Comment on lines
+269
to
283
| // Remove the file | ||
| err := os.Remove(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Also remove the saved block list for this file, and all the blocks data. | ||
| lfs.blockLists.Delete(options.Name) | ||
| err = removeAllFilesWithGivenPrefix(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| } |
Comment on lines
+321
to
+326
| // Move the block list from old name to new name | ||
| if value, ok := lfs.blockLists.Load(options.Src); ok { | ||
| lfs.blockLists.Store(options.Dst, value) | ||
| lfs.blockLists.Delete(options.Src) | ||
| } | ||
|
|
Comment on lines
+622
to
+629
| // Return (nil, nil) if the file exists but has no committed block list yet (equivalent to a small blob committed via PutBlob). | ||
| // If the file does not exist, return the underlying os error. | ||
| path := filepath.Join(lfs.path, name) | ||
| if _, err := os.Lstat(path); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &list, nil | ||
| return nil, nil |
Comment on lines
+91
to
+98
| echo -e "${YELLOW}[Scenario 1] Standard Block Sizes${NC}" | ||
| run_integrity_test "std_128k" "128k" "100" # 12.8 MB | ||
| run_integrity_test "std_256k" "256K" "1000" # 256 MB | ||
| run_integrity_test "std_1M" "1M" "20" # 20 MB | ||
| run_integrity_test "std_5M" "5M" "10" # 50 MB | ||
| run_integrity_test "std_10M_small" "10M" "5" # 50 MB | ||
| run_integrity_test "std_10M_large" "10M" "100" # 1000 MB | ||
|
|
Comment on lines
+184
to
+188
| pids="" | ||
| for i in {1..50}; do | ||
| run_integrity_test "conc_job_$i" "1M" "100" & | ||
| pids="$pids $!" | ||
| done |
Comment on lines
+206
to
+210
| func validateBlockList(blkList *internal.CommittedBlockList, f *file, blockSize uint64) error { | ||
| if blkList == nil || len(*blkList) == 0 { | ||
| return ErrInvalidBlockList | ||
| } | ||
| listLen := len(*blkList) |
Comment on lines
+606
to
+611
| // Store the committed block list for this file | ||
| if len(committedList) > 0 { | ||
| lfs.blockLists.Store(options.Name, &committedList) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+321
to
+327
| // Move the block list from old name to new name | ||
| if value, ok := lfs.blockLists.Load(options.Src); ok { | ||
| lfs.blockLists.Store(options.Dst, value) | ||
| lfs.blockLists.Delete(options.Src) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+132
to
+142
| // Parameters: | ||
| // - bufSize: Size of each buffer in bytes (typically equals block size) | ||
| // - memSize: Total memory for buffer pool (0 = auto-calculate from RAM) | ||
| // | ||
| // Returns an error if buffer pool initialization fails. | ||
| // | ||
| // Memory Calculation: | ||
| // | ||
| // If memSize is 0, uses 50% of available system RAM (configurable). | ||
| // This ensures BlockCache doesn't consume excessive memory while | ||
| // still providing good cache hit rates. |
Comment on lines
+18
to
+24
| # Configuration | ||
| TEST_FILENAME="integrity_test.dat" | ||
| SOURCE_FILE="/tmp/fuse_test_source.tmp" | ||
| READ_BACK_FILE="/tmp/fuse_test_readback.tmp" | ||
| FILE_SIZE_MB=100 # Size of the test file in MB (needs to be larger than the largest block size) | ||
| BLOCK_SIZES=("128K" "1M" "10M") | ||
|
|
Comment on lines
+201
to
+205
| func TestFilePunchHoleThenWrite(t *testing.T) { | ||
| t.Parallel() | ||
| filename := "testfile_punch_hole_then_write.txt" | ||
| initialContent := make([]byte, 20*1024*1024) // 20MB of data | ||
| _, err := io.ReadFull(rand.Reader, initialContent) |
| expectedContent = expectedContent[:finalSize] | ||
| } | ||
| assert.Equal(t, string(expectedContent), string(readContent)) | ||
| assert.Equal(t, string(expectedContent), string(readContent), filePath) |
Comment on lines
+285
to
+290
| if err != nil { | ||
| log.Err("BlockCache::getBlockIDList : Failed to write block for %v, ID: %v, file: %s [%v]", | ||
| task.path, task.blockID, task.path, err) | ||
| bufDesc.uploadErr = err | ||
| block.file.err.Store(&err) | ||
| } else { |
Comment on lines
+36
to
+57
| import ( | ||
| "errors" | ||
| "os" | ||
| "path/filepath" | ||
| ) | ||
|
|
||
| func removeAllFilesWithGivenPrefix(prefix string) error { | ||
| pattern := prefix + "_*" | ||
|
|
||
| matches, err := filepath.Glob(pattern) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| for _, file := range matches { | ||
| err1 := os.Remove(file) | ||
| if err1 != nil { | ||
| err = errors.Join(err, err1) | ||
| } | ||
| } | ||
| return err | ||
| } |
Comment on lines
311
to
+327
| func (lfs *LoopbackFS) RenameFile(options internal.RenameFileOptions) error { | ||
| log.Trace("LoopbackFS::RenameFile : %s -> %s", options.Src, options.Dst) | ||
| oldPath := filepath.Join(lfs.path, options.Src) | ||
| newPath := filepath.Join(lfs.path, options.Dst) | ||
| return os.Rename(oldPath, newPath) | ||
|
|
||
| err := os.Rename(oldPath, newPath) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Move the block list from old name to new name | ||
| if value, ok := lfs.blockLists.Load(options.Src); ok { | ||
| lfs.blockLists.Store(options.Dst, value) | ||
| lfs.blockLists.Delete(options.Src) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+606
to
+611
| // Store the committed block list for this file | ||
| if len(committedList) > 0 { | ||
| lfs.blockLists.Store(options.Name, &committedList) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
416
to
+428
| func (lfs *LoopbackFS) TruncateFile(options internal.TruncateFileOptions) error { | ||
| log.Trace("LoopbackFS::TruncateFile : name=%s", options.Name) | ||
| fsPath := filepath.Join(lfs.path, options.Name) | ||
| return os.Truncate(fsPath, options.NewSize) | ||
| err := os.Truncate(fsPath, options.NewSize) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Clear the saved block list when truncating | ||
| // The file structure has changed, so the old block list is no longer valid | ||
| lfs.blockLists.Delete(options.Name) | ||
|
|
||
| return nil |
Comment on lines
+18
to
+23
| # Configuration | ||
| TEST_FILENAME="integrity_test.dat" | ||
| SOURCE_FILE="/tmp/fuse_test_source.tmp" | ||
| READ_BACK_FILE="/tmp/fuse_test_readback.tmp" | ||
| FILE_SIZE_MB=100 # Size of the test file in MB (needs to be larger than the largest block size) | ||
| BLOCK_SIZES=("128K" "1M" "10M") |
Comment on lines
+123
to
+148
| // createFreeList initializes the free list and buffer pool. | ||
| // | ||
| // This function is called during BlockCache.Start() to set up buffer management. | ||
| // It performs: | ||
| // | ||
| // 1. Calculates number of buffers based on config or system RAM | ||
| // 2. Allocates buffer descriptors for all buffers | ||
| // 3. Initializes free list linking all buffers | ||
| // | ||
| // Parameters: | ||
| // - bufSize: Size of each buffer in bytes (typically equals block size) | ||
| // - memSize: Total memory for buffer pool (0 = auto-calculate from RAM) | ||
| // | ||
| // Returns an error if buffer pool initialization fails. | ||
| // | ||
| // Memory Calculation: | ||
| // | ||
| // If memSize is 0, uses 50% of available system RAM (configurable). | ||
| // This ensures BlockCache doesn't consume excessive memory while | ||
| // still providing good cache hit rates. | ||
| // | ||
| // Why maxBuffers can be large: | ||
| // | ||
| // The number of buffers is calculated as memSize / bufSize. | ||
| // With large block sizes (e.g., 16 MB), this may result in | ||
| // relatively few buffers (e.g., 1 GB / 16 MB = 64 buffers). |
using 64K for tracking writes in a buffer is not working as write back caching is not aligning to the 64K when it evicts from the page cache. but the size of the write buffer would always align with 4K.
Comment on lines
+8
to
+10
| auth: | ||
| type: key | ||
| account-account-key: { STO_ACC_KEY } |
Comment on lines
538
to
540
| func (lfs *LoopbackFS) CommitData(options internal.CommitDataOptions) error { | ||
| log.Trace("LoopbackFS::StageData : name=%s", options.Name) | ||
|
|
Comment on lines
568
to
573
| data := make([]byte, info.Size()) | ||
| n, err := block.Read(data) | ||
| if int64(n) != info.Size() { | ||
| log.Err("LoopbackFS::CommitData : error [could not read entire file]") | ||
| return err | ||
| } |
Comment on lines
+575
to
+576
| offset := int64(idx * (int)(options.BlockSize)) | ||
| n, err = blob.WriteAt(data, offset) |
Comment on lines
+321
to
+327
| // Move the block list from old name to new name | ||
| if value, ok := lfs.blockLists.Load(options.Src); ok { | ||
| lfs.blockLists.Store(options.Dst, value) | ||
| lfs.blockLists.Delete(options.Src) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+606
to
+611
| // Store the committed block list for this file | ||
| if len(committedList) > 0 { | ||
| lfs.blockLists.Store(options.Name, &committedList) | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+73
to
+80
| if [ "$ref_md5" == "$fuse_md5" ]; then | ||
| echo -e "${GREEN}PASS${NC}" | ||
| else | ||
| echo -e "${RED}FAIL (Checksum Mismatch)${NC}" | ||
| echo " Ref : $ref_md5" | ||
| echo " FUSE: $fuse_md5" | ||
| return 1 | ||
| fi |
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.
Type of Change
Description
How Has This Been Tested?
Checklist
Related Links