chore: [cache-dir-size-fix] Part 1: Add disk-size utilities and shared-dir locker utilities

[gargnitingoogle]

Description

• Adds some disk-utilities and a directory-locker utility, which will be used in later commits for the fix in cache-dir-size calculation
• Dependency for chore: [cache-dir-size-fix] Part 2: Add configuration flags and parsing for file cache size fix #4415

Link to the issue in case of a bug fix.

b/477828938

Testing details

1. Manual - Nothing to test as this change doesn't affect gcsfuse functionality in any way
2. Unit tests - Added new tests
3. Integration tests - Nothing to test as this change doesn't affect gcsfuse functionality in any way

Any backward incompatible change? If so, please explain.

[gargnitingoogle]

/gemini review

[gargnitingoogle]

/gemini summary

[gemini-code-assist]

Code Review

This pull request introduces valuable utilities for calculating disk size and a striped locking mechanism for directories to enhance performance and prevent race conditions. The SharedDirLocker is a good approach to reduce lock contention, and the new SafeCreateFile function correctly uses it. The disk size calculation is implemented concurrently, and the test coverage for the new functionality is thorough. I have one suggestion to improve the performance of the disk size calculation by adjusting a concurrency parameter.

[gemini-code-assist]

Summary of Changes

This pull request lays foundational groundwork for a cache directory size fix by introducing robust utilities for disk space management and concurrent directory access. It provides mechanisms to precisely measure disk consumption, ensure the integrity of directory structures during file creation, and safely handle operations across multiple cache directories through a striped locking mechanism. These changes are crucial for reliable cache management and preventing race conditions.

Highlights

• Shared Directory Locking: Introduced a new SharedDirLocker utility that uses striped read-write mutexes to manage concurrent access to cache directories, improving thread safety for file operations.
• Disk Usage Utilities: Added a new disk_util package with functions to accurately calculate disk usage, determine the file system's block size, and recursively remove empty directories.
• Safe File Creation: Enhanced the CreateFile function to automatically retry directory creation if the parent directory is missing, and added a SafeCreateFile function that leverages the new DirLocker interface for thread-safe file creation.

Changelog

• internal/cache/util/shared_dir_locker.go
  • Added SharedDirLocker struct and NewSharedDirLocker constructor.
  • Implemented ReadLock, ReadUnlock, WriteLock, and WriteUnlock methods using striped sync.RWMutex.
• internal/cache/util/shared_dir_locker_test.go
  • Added unit tests for SharedDirLocker covering basic locking, concurrent readers, writer blocking, and striping behavior.
• internal/cache/util/util.go
  • Imported baseutil for the new DirLocker interface.
  • Modified CreateFile to include a retry mechanism for os.MkdirAll if os.OpenFile fails due to a missing directory.
  • Added SafeCreateFile function that acquires a read lock on the parent directory before calling CreateFile.
• internal/util/disk_util.go
  • Added getDiskUsageFromInfo and getDiskUsage functions to calculate allocated disk space for files and directories.
  • Defined the DirLocker interface for directory locking.
  • Implemented GetSizeOnDisk and GetSizeOnDiskWithLocker for concurrent calculation of directory sizes on disk, with optional error ignoring and locker integration.
  • Added GetSpeculativeFileSizeOnDisk to estimate file size based on volume block size.
  • Implemented GetVolumeBlockSize to retrieve the file system's block size.
  • Added RemoveEmptyDirs and RemoveEmptyDirsWithLocker for recursively removing empty directories, with optional locker integration.
• internal/util/disk_util_test.go
  • Added a DiskUtilTest suite for disk utility functions.
  • Included tests for GetSpeculativeFileSizeOnDisk, GetSizeOnDisk (normal, only directories, permission denied scenarios), GetVolumeBlockSize, and RemoveEmptyDirs.

Activity

• gargnitingoogle requested a gemini review.
• gargnitingoogle requested a gemini summary.
• gemini-code-assist[bot] suggested increasing the concurrency level for directory walking in GetSizeOnDisk for better performance on I/O-bound tasks.

[gemini-code-assist]

Code Review

This pull request introduces several utility functions for disk size calculation and directory locking, which are essential for managing the cache directory safely and efficiently. The new SharedDirLocker uses striped locking to minimize contention, and SafeCreateFile leverages it to prevent race conditions during file creation. The overall implementation is good, but I've identified a significant concurrency issue in the removeEmptyDirs function that could lead to race conditions and potential deadlocks. I've also noted a minor issue with a vague error message. My review includes suggestions to address these points.

[codecov]

Codecov Report

❌ Patch coverage is 74.40000% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.67%. Comparing base (4dc8db7) to head (77e24b9).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/util/disk_util.go	68.31%	19 Missing and 13 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4414      +/-   ##
==========================================
- Coverage   83.70%   83.67%   -0.03%     
==========================================
  Files         163      165       +2     
  Lines       20077    20202     +125     
==========================================
+ Hits        16805    16904      +99     
- Misses       2648     2663      +15     
- Partials      624      635      +11     

Flag	Coverage Δ
unittests	83.67% <74.40%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gargnitingoogle]

/gemini review

[gemini-code-assist]

Code Review

The pull request introduces a robust SharedDirLocker utility and comprehensive disk utility functions, which are well-tested with a good suite of unit tests. However, the PR description should be updated to accurately reflect the presence of these new unit tests, as it currently states that unit tests are 'NA'.

[github-actions]

Hi @ashmeenkaur, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you!

[github-actions]

Hi @ashmeenkaur, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you!

[github-actions]

Hi @ashmeenkaur, @kislaykishore, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you!

[gargnitingoogle]

Moving it back to draft as this approach is currently under review. Will restructure or mark it ready for review later again, if needed.