Skip to content

Remove platform-specific files from tiered_storage and file_cache - #1123

Merged
foodprocessor merged 6 commits into
feature/tiered_storagefrom
refactor/common-file-helpers
Nov 10, 2026
Merged

Remove platform-specific files from tiered_storage and file_cache#1123
foodprocessor merged 6 commits into
feature/tiered_storagefrom
refactor/common-file-helpers

Conversation

@foodprocessor

@foodprocessor foodprocessor commented Nov 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What type of Pull Request is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Describe your changes in brief

Removes all platform-specific source files from tiered_storage and file_cache by moving the OS-specific primitives into common/util_{linux,windows}.go. Stacked on #1049; merge that first (GitHub will retarget this PR to main when the base branch is deleted).

Commits are meant to be reviewed one at a time:

  1. Move OS-specific file helpers into common. Adds common.GetFileTimes, GetFileOwner and IsProtectedSystemFile. GetAvailFree now returns bytes on both OSes, resolving its TODO:
    • Linux multiplies by Frsize, the unit statfs block counts are in.
    • Bug fix: on Windows, the GetDiskFreeSpaceEx out-params were misnamed, so GetAvailFree returned (total free, caller-available), the reverse of Linux. It now returns (caller-available, total free).
    • Callers (cmd/config.go, block_cache) drop their * 4096 conversions. Block cache StatFs now reports Bfree in its own block-size units instead of a raw block count from /.
  2. Share local file attribute helpers with file_cache. The identical newObjAttr/newTieredStorageObjAttr builders become internal.NewLocalObjAttr, and file_cache uses common.GetAvailFree.
  3. Drop local sync from file_cache FlushFile. The upload reads the cache file back through the OS page cache, so it doesn't need a sync first.
    • On Linux, syncFile duplicated and closed the fd, which writes nothing to a local disk.
    • On Windows it called FlushFileBuffers on every dirty flush, a full disk flush before each upload.
    • The EBADF check for a handle with no file object is kept.

Behavior changes to watch for:

  • Windows file_cache, block_cache and tiered_storage size their caches from the caller's quota-limited free space rather than the volume's total free space. These are equal without disk quotas.
  • Linux free space uses Frsize instead of Bsize. These are equal on ext4, xfs and btrfs.
  • Windows file_cache no longer forces a disk flush before uploading.

Checklist

  • Tested locally
  • Added new dependencies
  • Updated documentation
  • Added tests

Validation (Linux; Windows checked by cross-compiling):

  • go build -tags fuse3 ./... and GOOS=windows go build ./...: passed
  • go vet on changed packages, Linux and GOOS=windows: passed. The Windows run shows an existing unsafe.Pointer warning in component/block_cache/block_windows.go, which this PR doesn't touch.
  • go test ./component/file_cache/ ./component/tiered_storage/ ./component/block_cache/ ./internal/ --tags=unittest,fuse3: passed. block_cache TestEmpty failed once in three runs and also fails occasionally without these changes.
  • go test ./common/ ./cmd/ --tags=unittest,fuse3: TestIsMountActiveNoMount and TestSyncNoS3Credentials fail the same way on the base branch (environment-dependent).
  • golangci-lint run --tests=false --build-tags fuse3 --max-issues-per-linter=0 --new-from-rev=origin/feature/tiered_storage: 0 issues (Linux and GOOS=windows).
  • The former Linux-only TestEvictionUploadsLocalModeAndOwner now runs on both OSes. On Windows it asserts that no chmod or chown is sent.

Related Issues

@foodprocessor foodprocessor mentioned this pull request Nov 10, 2026
Merged
3 of 9 tasks
foodprocessor and others added 3 commits November 10, 2026 03:17
@foodprocessor
Move OS-specific file helpers into common
d748187 
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>
@foodprocessor
Share local file attribute helpers with file_cache
829ce3b 
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>
@foodprocessor
Drop local sync from file_cache FlushFile
bd8f68a 
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>
@foodprocessor
foodprocessor force-pushed the refactor/common-file-helpers branch from d1662d0 to bd8f68a Compare November 10, 2026 10:19
@foodprocessor
foodprocessor marked this pull request as ready for review November 10, 2026 16:49
jfantinhardesty
jfantinhardesty approved these changes Nov 10, 2026
View reviewed changes

@jfantinhardesty jfantinhardesty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@foodprocessor
foodprocessor removed this pull request from stack #1128 November 10, 2026 23:14
@foodprocessor
foodprocessor merged commit 6daf6e3 into feature/tiered_storage Nov 10, 2026
26 checks passed
@foodprocessor
foodprocessor deleted the refactor/common-file-helpers branch November 10, 2026 23:14

This branch was previously deployed

1 inactive deployment
testing — 03b798e4 Deployed Nov 10, 2026 by foodprocessor via Cleanup Test Accounts (1.27, linux) #3947
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jfantinhardesty jfantinhardesty jfantinhardesty approved these changes
@Dabnsky Dabnsky Awaiting requested review from Dabnsky Dabnsky is a code owner
@Ferelith-maker Ferelith-maker Awaiting requested review from Ferelith-maker Ferelith-maker is a code owner
@brayan-trejo brayan-trejo Awaiting requested review from brayan-trejo brayan-trejo is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@foodprocessor @jfantinhardesty