Repository navigation
Feature/tiered storage - #1049
Open
foodprocessor wants to merge 212 commits into
Open
foodprocessor wants to merge 212 commits into
foodprocessor wants to merge 212 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
4 of 9 tasks
Restore file_cache to its main-branch state. Its switch to cache_size.Tracker (from 0f4a1c8 and 84f945d) changes how file_cache enforces its hard limit, so it is being reviewed separately. common/cache_size stays here because tiered_storage uses it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 of 9 tasks
) * Move OS-specific file helpers into common Make tiered_storage platform-independent by moving the code that inspects fs.FileInfo.Sys() and queries free disk space into common/util_{linux,windows}.go: GetFileTimes, GetFileOwner and IsProtectedSystemFile. GetAvailFree now returns bytes on both platforms instead of 4096-byte "blocks", resolving its TODO: - Linux multiplies by Frsize, the unit statfs block counts are in. - Windows no longer swaps avail and free; the GetDiskFreeSpaceEx out-params were misnamed, so avail was total free bytes and free was the caller's quota-limited bytes. - Callers drop their * 4096 conversions, and block_cache StatFs now reports Bfree in units of its own block size. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Share local file attribute helpers with file_cache file_cache and tiered_storage built identical ObjAttrs from local file info, so move the builder to internal.NewLocalObjAttr. file_cache now also uses common.GetAvailFree in place of its own platform-specific getAvailableSize. On Windows, file_cache now sizes the cache from the caller's quota-limited free space instead of the volume's total free space; on Linux it uses Frsize instead of Bsize. These are equal on common configurations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Drop local sync from file_cache FlushFile The upload reads the cache file back through the OS page cache, so it sees all written data without first syncing it to the local disk. - Linux: syncFile duplicated and closed the fd, which writes nothing to a local disk, so removing it only saves two syscalls per flush. - Windows: syncFile called FlushFileBuffers, forcing a full disk flush on every dirty flush before the upload. Dropping it removes that cost; the cloud object, not the cache file, is the durable copy. FlushFile keeps returning EBADF for a dirty handle with no file object. This removes the last platform-specific code from file_cache. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix tiered storage smoke test --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical issues can cause destination data loss during failed renames and let Windows synchronization report success without persisting the only updated copy.
9 open issues
16 resolved since last review
Review effort: Balanced
Comment on lines
+278
to
+281
| } else if info, err := os.Stat(filepath.Join(q.localRoot, node.name)); err != nil { | ||
| log.Warn("lruQueue::nominate : dropping %s [%v]", node.name, err) | ||
| q.extractNode(node) | ||
| delete(q.nodes, node.name) |
Comment on lines
+418
to
+420
| localNames := make(map[string]struct{}, len(entries)) | ||
| for _, entry := range entries { | ||
| localNames[entry.Name()] = struct{}{} |
Comment on lines
+503
to
+504
| if _, err := os.Stat(dstLocal); err == nil { | ||
| return syscall.EEXIST |
| return nil, err | ||
| } | ||
| localPath := c.localPath(options.Name) | ||
| handle, err := c.openHandle(options.Name, localPath, os.O_RDWR, false, flock) |
| // It takes no lock: files move between trees by atomic rename. | ||
| func (c *TieredStorage) GetAttr(options internal.GetAttrOptions) (*internal.ObjAttr, error) { | ||
| localPath := c.localPath(options.Name) | ||
| info, err := os.Stat(localPath) |
…e (it just invalidates an attribute cache entry).
This branch had an error being deployed
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.
Add tiered storage component.