Repository navigation
Feature/tiered storage - #1049
foodprocessor wants to merge 104 commits into
Conversation
…t OpenFile test pass
… with no file attatched, also added some config path
…ion), added tests for OpenFile O_Create
Feature/tiered storage setup
Feature/tiered storage
…cially on Windows)
There was a problem hiding this comment.
🟡 Changes recommended
Several rename, pagination, accounting, and persistence defects can cause hangs, inconsistent listings, or data loss.
16 open findings
Prevent internal metadata names from colliding with user data · New Avoid deleting destination on unrelated rename failures · New Synchronize handle path lookup with concurrent renames · New Mark partial writes dirty and update tracked size · New Handle same-path rename as a no-op · New Handle destination replacement state and open handles during rename · New Update cloud state when overwriting cloud-only rename destination · New Validate stream and tiered_storage as mutually exclusive · New Preserve concurrent cache size updates during Refresh · New Materialize cloud-only parent before creating a directory · New Clean up local directory after cloud creation failure · New Use stable pagination when merging local and cloud entries · New Create local parent before renaming into cloud-only directory · New Cap reported free blocks to prevent invalid filesystem statistics · New Add the missing “to” · New Correct default thresholds to 90/80 · New
What changed in this PR
Adds a local-primary tiered storage component with cloud overflow, LRU eviction, restart recovery, and cross-platform support.
Changes:
- Implements tiered file operations, persistence, capacity tracking, and eviction.
- Adds configuration, documentation, registration, and pipeline validation.
- Adds unit tests and an S3 mount smoke test.
| File | Description |
|---|---|
.github/workflows/unit-test.yml |
Adds tiered-storage smoke testing. |
README.md |
Documents tiered-storage usage. |
cmd/imports.go |
Registers the new component. |
cmd/mount_linux_test.go |
Verifies cleanup preserves tiered data. |
common/lock_map.go |
Adds atomic handle counts and try-locking. |
common/util.go |
Extends pipeline validation. |
common/util_test.go |
Tests tiered pipeline combinations. |
component/tiered_storage/cache_size.go |
Tracks local capacity usage. |
component/tiered_storage/lru_policy.go |
Implements parallel LRU eviction. |
component/tiered_storage/lru_policy_test.go |
Tests eviction behavior. |
component/tiered_storage/persistence.go |
Persists and recovers local state. |
component/tiered_storage/tiered_storage.go |
Implements the core component. |
component/tiered_storage/tiered_storage_linux.go |
Adds Linux filesystem support. |
component/tiered_storage/tiered_storage_linux_test.go |
Tests Linux ownership persistence. |
component/tiered_storage/tiered_storage_test.go |
Tests component operations and recovery. |
component/tiered_storage/tiered_storage_windows.go |
Adds Windows filesystem support. |
sample_configs/sampleTieredStorageConfigS3.yaml |
Provides an S3 example. |
setup/advancedConfig.yaml |
Documents advanced configuration. |
setup/baseConfig.yaml |
Adds baseline configuration options. |
testdata/config/s3_key_tiered_storage.yaml |
Adds smoke-test configuration. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| tieredStorageSnapshotPath = ".tieredStorageSnapshot.gob" | ||
| tieredStorageSnapshotVersion = 1 |
| if rmErr := os.Remove(dst); rmErr != nil && !errors.Is(rmErr, os.ErrNotExist) { | ||
| return err | ||
| } | ||
| return os.Rename(src, dst) |
| var node *FileNode | ||
| if val, ok := c.fileMap.Load(options.Handle.Path); ok { | ||
| node = val.(*FileNode) | ||
| } |
| if err == nil { | ||
| c.setHandleDirty(options.Handle) | ||
| if node != nil { | ||
| node.isDirty.Store(true) | ||
| if info, statErr := f.Stat(); statErr == nil { | ||
| c.cacheSize.Add(info.Size() - node.size.Swap(info.Size())) | ||
| } | ||
| } | ||
| } else { | ||
| log.Err( | ||
| "TieredStorage::WriteFile : failed to write %s [%s]", | ||
| options.Handle.Path, | ||
| err.Error(), | ||
| ) | ||
| } |
| } | ||
|
|
||
| func (c *TieredStorage) RenameFile(options internal.RenameFileOptions) error { | ||
| log.Trace("TieredStorage::RenameFile : src=%s, dst=%s", options.Src, options.Dst) |
| if token == "" { | ||
| for name, attr := range localAttrs { | ||
| if _, found := listed[name]; !found { | ||
| attrs = append(attrs, attr) |
| if err := os.Rename(srcPath, dstPath); err != nil { | ||
| return err | ||
| } |
| available := max(int64(c.maxCacheSize)-c.cacheSize.Used(), 0) | ||
| stat := &common.Statfs_t{ | ||
| Blocks: uint64(c.maxCacheSize) / blockSize, | ||
| Bavail: uint64(available) / blockSize, | ||
| Bfree: physicalFree / blockSize, | ||
| Bsize: blockSize, |
| ### Expanding Existing Local Storage Using Tiered Storage | ||
|
|
||
| When using `tiered_storage` to add cloud capacity to an existing local storage location, please ensure all access is done through the Cloudfuse mount path / virtual drive. The `tiered_storage` `path` and all its existing contents will become *internal* local storage. Accessing the `tiered_storage` `path` directly may have unpredictable results. | ||
| For example, if you want expand the capacity of your existing local drive, `D:`, you could change that local drive's letter to `E:` first, then set `tiered_storage` `path` to `E:` and mount cloudfuse to `D:`. Then any existing applications or workflows that normally use `D:` would continue as normal, now pointed at the cloudfuse mount location. |
| high-threshold: <% local storage consumed which triggers eviction. Default - 80> | ||
| low-threshold: <% local storage consumed which eviction targets. Must be less than high-threshold. Default - 60> |
| if options.Handle == nil { | ||
| handle, err := c.OpenFile(internal.OpenFileOptions{ | ||
| Name: options.Name, | ||
| Flags: os.O_RDWR, |
There was a problem hiding this comment.
If there is no open handle and new size is 0, we should probably just open with O_TRUNC, so we don't end up downloading the whole file. I think some applications do that.
| return nil | ||
| } | ||
|
|
||
| func (c *TieredStorage) RenameFile(options internal.RenameFileOptions) error { |
There was a problem hiding this comment.
I think rename has some edge cases to think through. If the src is in the cloud only, and dst is local only, then doing that rename, I think will end up uploading the dst once eviction happens and not the src. So I think it should check if dst exists, and delete the local copy and remove it from the queue.
| c.policy.Dequeue(options.Name) | ||
| c.fileMap.Delete(options.Name) | ||
|
|
||
| return c.purgeLocal(options.Name) |
There was a problem hiding this comment.
Other parts of the code do purgeLocal first, and then the deletion happens, in case it fails (which maybe could happen on Windows if the file is open). So I think that pattern should follow here.
| return c.NextComponent().CloseDir(options) | ||
| } | ||
|
|
||
| func (c *TieredStorage) RenameDir(options internal.RenameDirOptions) error { |
There was a problem hiding this comment.
Here we are doing local first then cloud. In case there is an issue with renaming a file in the cloud, probably safer to do cloud first then local.
There was a problem hiding this comment.
Also, we probably need some locks when doing a directory rename. I could imagine some race conditions that could probably come up here.
| type persistedFileState struct { | ||
| Size int64 | ||
| Mtime int64 | ||
| CloudBacked bool |
There was a problem hiding this comment.
There is an edge case concern with cloudBacked. It currently does two things (Is there an object in the cloud with this name, and is this file in the cloud and the local copy is a temp copy). Most times these are the same, but not always. Consider a local file A and a cloud file B. Then we mv A B. The answer to 1 is yes, but two is no. But cloudBaked would be copied from A, which would say no. Then the old B is still in the cloud and does not get removed until later.
| capacityPollInterval = time.Second | ||
| reconcileCapacityInterval = 5 * time.Minute | ||
| partialDownloadSuffix = ".cloudfuse-partial" | ||
| // Cache holds user data and is only accessed by this process (CWE-732). |
There was a problem hiding this comment.
Is there a reason for storing these in the mount path, and not in a separate folder?
| l.mtx.Unlock() | ||
| } | ||
|
|
||
| func (l *LockMapItem) TryLock() bool { |
There was a problem hiding this comment.
I think this is an anti pattern. Go says that correct uses of TryLock are rare, and I'm not sure it is correctly used here. It probably points to a bigger locking problem.
| } | ||
|
|
||
| // EvictNow synchronously makes room for growBytes. | ||
| func (q *lruQueue) EvictNow(growBytes int64) bool { |
There was a problem hiding this comment.
It seems expensive to do eviction synchronously. Any reason to not follow file cache and do eviction in the background?



Add tiered storage component.