Skip to content

Commit

Permalink
Refactor cache management and hardlink creation
Browse files Browse the repository at this point in the history
- Simplified the directory cache retrieval process by removing redundant hardlink checks and streamlining the file path determination.
- Introduced utility functions `CachePath` and `WipFile` in `utils.go` to centralize path and temporary file creation logic, improving code reuse and readability.
- Updated `CreateLink` in `hardlink.go` to use `CachePath` for consistent path generation.
- Removed redundant methods `cachePath` and `wipFile` from `directoryCache`, replacing their functionality with the new utility functions.

Signed-off-by: ChengyuZhu6 <[email protected]>
  • Loading branch information
ChengyuZhu6 committed Feb 5, 2025
1 parent 39666b4 commit b879c95
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 32 deletions.
45 changes: 16 additions & 29 deletions cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,20 +228,7 @@ func (dc *directoryCache) Get(key string, opts ...Option) (Reader, error) {
opt = o(opt)
}

// 1. Try to get from hardlink first
if dc.hlManager != nil {
if linkPath, exists := dc.hlManager.GetLink(key); exists {
if r, err := os.Open(linkPath); err == nil {
return &reader{
ReaderAt: r,
closeFunc: r.Close,
}, nil
}
// If failed to open, continue with normal flow
}
}

// 2. Try to get from memory cache
// Try to get from memory cache
if !dc.direct && !opt.direct {
if b, done, ok := dc.cache.Get(key); ok {
return &reader{
Expand All @@ -265,13 +252,21 @@ func (dc *directoryCache) Get(key string, opts ...Option) (Reader, error) {
}
}

// 3. Read from disk
file, err := os.Open(dc.cachePath(key))
// Try to get from hardlink first
filepath := CachePath(dc.directory, key)
if dc.hlManager != nil {
if linkPath, exists := dc.hlManager.GetLink(key); exists {
filepath = linkPath
}
}

// Read from disk
file, err := os.Open(filepath)
if err != nil {
return nil, fmt.Errorf("failed to open blob file for %q: %w", key, err)
}

// 4. If in direct mode, don't cache file descriptor
// If in direct mode, don't cache file descriptor
if dc.direct || opt.direct {
return &reader{
ReaderAt: file,
Expand All @@ -284,7 +279,7 @@ func (dc *directoryCache) Get(key string, opts ...Option) (Reader, error) {
}, nil
}

// 5. Cache file descriptor
// Cache file descriptor
return &reader{
ReaderAt: file,
closeFunc: func() error {
Expand Down Expand Up @@ -323,7 +318,7 @@ func (dc *directoryCache) Add(key string, opts ...Option) (Writer, error) {
}

// Create temporary file
w, err := dc.wipFile(key)
w, err := WipFile(dc.wipDirectory, key)
if err != nil {
return nil, err
}
Expand All @@ -337,7 +332,7 @@ func (dc *directoryCache) Add(key string, opts ...Option) (Writer, error) {
}

// Commit file
targetPath := dc.cachePath(key)
targetPath := CachePath(dc.directory, key)
if err := os.MkdirAll(filepath.Dir(targetPath), 0700); err != nil {
return fmt.Errorf("failed to create cache directory: %w", err)
}
Expand Down Expand Up @@ -393,14 +388,6 @@ func (dc *directoryCache) isClosed() bool {
return closed
}

func (dc *directoryCache) cachePath(key string) string {
return filepath.Join(dc.directory, key[:2], key)
}

func (dc *directoryCache) wipFile(key string) (*os.File, error) {
return os.CreateTemp(dc.wipDirectory, key+"-*")
}

func NewMemoryCache() BlobCache {
return &MemoryCache{
Membuf: map[string]*bytes.Buffer{},
Expand Down Expand Up @@ -491,7 +478,7 @@ func (dc *directoryCache) CreateHardlink(key string) error {
return nil
}
log.L.Debugf("Creating hardlink for key %q", key)
return dc.hlManager.CreateLink(key, dc.cachePath(key))
return dc.hlManager.CreateLink(key, CachePath(dc.directory, key))
}

func (dc *directoryCache) HasHardlink(key string) bool {
Expand Down
5 changes: 2 additions & 3 deletions cache/hardlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,13 @@ func (hm *HardlinkManager) CreateLink(key string, sourcePath string) error {
}

// Create hardlink in hardlinks directory
linkPath := filepath.Join(hm.hlDir, key[:2], key)
linkPath := CachePath(hm.hlDir, key)
if err := os.MkdirAll(filepath.Dir(linkPath), 0700); err != nil {
return fmt.Errorf("failed to create link dir: %w", err)
}

// Create temporary link path
tmpLinkPath := linkPath + ".tmp"

tmpLinkPath := CachePath(hm.hlDir, key+".wip")
// Remove temporary link if it exists
os.Remove(tmpLinkPath)

Expand Down
36 changes: 36 additions & 0 deletions cache/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
Copyright The containerd Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package cache

import (
"fmt"
"os"
"path/filepath"
)

// WipFile creates a temporary file in the given directory with the given key pattern
func WipFile(wipDirectory string, key string) (*os.File, error) {
if err := os.MkdirAll(wipDirectory, 0700); err != nil {
return nil, fmt.Errorf("failed to create wip directory: %w", err)
}
return os.CreateTemp(wipDirectory, key+"-*")
}

// CachePath returns the path for a cache entry with the given key
func CachePath(directory string, key string) string {

Check failure on line 34 in cache/utils.go

View workflow job for this annotation

GitHub Actions / Linter (.)

exported: func name will be used as cache.CachePath by other packages, and that stutters; consider calling this Path (revive)
return filepath.Join(directory, key[:2], key)
}

0 comments on commit b879c95

Please sign in to comment.