Merge pull request 'Promote develop: vfs/cache promoteToFast race fix' (#24) from develop into main
Promote vfs/cache Stat snapshot + promoteToFast size fix.
This commit was merged in pull request #24.
This commit is contained in:
Vendored
+4
-2
@@ -186,7 +186,7 @@ func (tc *TieredCache) Capacity() int64 {
|
|||||||
func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
|
func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
|
||||||
defer func() { _ = reader.Close() }() // best-effort close; error secondary to promotion attempt (async best-effort path)
|
defer func() { _ = reader.Close() }() // best-effort close; error secondary to promotion attempt (async best-effort path)
|
||||||
|
|
||||||
// Get file info from slow tier to determine size
|
// Size for the space/ReadAll guards comes from a Stat snapshot, not the live in-map FileInfo.
|
||||||
var size int64
|
var size int64
|
||||||
if slow := tc.slow.Load(); slow != nil {
|
if slow := tc.slow.Load(); slow != nil {
|
||||||
if vfs, ok := slow.(vfs.VFS); ok {
|
if vfs, ok := slow.(vfs.VFS); ok {
|
||||||
@@ -210,7 +210,7 @@ func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Guard promotion ReadAll using already-fetched size (in addition to space check above)
|
// Guard promotion ReadAll using already-fetched size (in addition to space check above)
|
||||||
if size > 0 && size > (1<<30) { // conservative 1GB hard limit on promotion reads (aligns with typical max_object_size)
|
if size > (1 << 30) { // conservative 1GB hard limit on promotion reads (aligns with typical max_object_size)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
// Read the entire file content
|
// Read the entire file content
|
||||||
@@ -218,6 +218,8 @@ func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return // Skip promotion if read fails
|
return // Skip promotion if read fails
|
||||||
}
|
}
|
||||||
|
// Create with the bytes we actually hold so we never reuse a live FileInfo.Size.
|
||||||
|
size = int64(len(content))
|
||||||
|
|
||||||
// Create the file in fast tier
|
// Create the file in fast tier
|
||||||
if fast := tc.fast.Load(); fast != nil {
|
if fast := tc.fast.Load(); fast != nil {
|
||||||
|
|||||||
+9
-4
@@ -612,7 +612,9 @@ func (d *DiskFS) Delete(key string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Stat returns file information with lazy discovery
|
// Stat returns a snapshot of file information with lazy discovery.
|
||||||
|
// The returned *FileInfo is not the live cache entry; Close may update Size
|
||||||
|
// on the in-map object under d.mu.
|
||||||
func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
|
func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
|
||||||
if key == "" {
|
if key == "" {
|
||||||
return nil, vfserror.ErrInvalidKey
|
return nil, vfserror.ErrInvalidKey
|
||||||
@@ -627,9 +629,10 @@ func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
|
|||||||
keyMu.RLock()
|
keyMu.RLock()
|
||||||
d.mu.RLock()
|
d.mu.RLock()
|
||||||
if fi, ok := d.info[key]; ok {
|
if fi, ok := d.info[key]; ok {
|
||||||
|
snap := fi.Clone()
|
||||||
d.mu.RUnlock()
|
d.mu.RUnlock()
|
||||||
keyMu.RUnlock()
|
keyMu.RUnlock()
|
||||||
return fi, nil
|
return snap, nil
|
||||||
}
|
}
|
||||||
d.mu.RUnlock()
|
d.mu.RUnlock()
|
||||||
keyMu.RUnlock()
|
keyMu.RUnlock()
|
||||||
@@ -649,8 +652,9 @@ func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
|
|||||||
// Double-check after acquiring write lock
|
// Double-check after acquiring write lock
|
||||||
d.mu.Lock()
|
d.mu.Lock()
|
||||||
if fi, ok := d.info[key]; ok {
|
if fi, ok := d.info[key]; ok {
|
||||||
|
snap := fi.Clone()
|
||||||
d.mu.Unlock()
|
d.mu.Unlock()
|
||||||
return fi, nil
|
return snap, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Re-verify the file still exists on disk under the lock before inserting.
|
// Re-verify the file still exists on disk under the lock before inserting.
|
||||||
@@ -669,9 +673,10 @@ func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
|
|||||||
fi.UpdateAccessBatched(d.timeUpdater)
|
fi.UpdateAccessBatched(d.timeUpdater)
|
||||||
// Note: size not updated on lazy discovery (preserves prior behavior; initial on-disk accounted via bg populate at New time,
|
// Note: size not updated on lazy discovery (preserves prior behavior; initial on-disk accounted via bg populate at New time,
|
||||||
// subsequent files come via Create which accounts size).
|
// subsequent files come via Create which accounts size).
|
||||||
|
snap := fi.Clone()
|
||||||
d.mu.Unlock()
|
d.mu.Unlock()
|
||||||
|
|
||||||
return fi, nil
|
return snap, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// EvictLRU evicts the least recently used files to free up space
|
// EvictLRU evicts the least recently used files to free up space
|
||||||
|
|||||||
@@ -289,7 +289,8 @@ func (m *MemoryFS) Delete(key string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Stat returns file information
|
// Stat returns a snapshot of file information. The returned *FileInfo is not
|
||||||
|
// the live cache entry; Close may update Size on the in-map object under m.mu.
|
||||||
func (m *MemoryFS) Stat(key string) (*types.FileInfo, error) {
|
func (m *MemoryFS) Stat(key string) (*types.FileInfo, error) {
|
||||||
if key == "" {
|
if key == "" {
|
||||||
return nil, vfserror.ErrInvalidKey
|
return nil, vfserror.ErrInvalidKey
|
||||||
@@ -310,7 +311,7 @@ func (m *MemoryFS) Stat(key string) (*types.FileInfo, error) {
|
|||||||
defer m.mu.RUnlock()
|
defer m.mu.RUnlock()
|
||||||
|
|
||||||
if fi, ok := m.info[key]; ok {
|
if fi, ok := m.info[key]; ok {
|
||||||
return fi, nil
|
return fi.Clone(), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil, vfserror.ErrNotFound
|
return nil, vfserror.ErrNotFound
|
||||||
|
|||||||
@@ -346,6 +346,45 @@ func TestMemoryFS_ConcurrentCloseAndEvict_RaceFree(t *testing.T) {
|
|||||||
_ = m.LRU.Len()
|
_ = m.LRU.Len()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestMemoryFS_StatReturnsSnapshot(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
m, err := New(1024)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
w, err := m.Create("k", 10)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, err := w.Write([]byte("hello")); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := w.Close(); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
fi, err := m.Stat("k")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if fi.Size != 5 {
|
||||||
|
t.Fatalf("size %d want 5", fi.Size)
|
||||||
|
}
|
||||||
|
fi.Size = 999
|
||||||
|
fi.AccessCount = 0
|
||||||
|
|
||||||
|
fi2, err := m.Stat("k")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if fi2.Size != 5 {
|
||||||
|
t.Errorf("Stat returned live FileInfo; store size became %d", fi2.Size)
|
||||||
|
}
|
||||||
|
if fi2.AccessCount == 0 {
|
||||||
|
t.Error("Stat returned live FileInfo; AccessCount mutation leaked")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestMemoryFS_EvictVariantsAndErrors(t *testing.T) {
|
func TestMemoryFS_EvictVariantsAndErrors(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
m, err := New(800)
|
m, err := New(800)
|
||||||
|
|||||||
@@ -27,6 +27,17 @@ func NewFileInfo(key string, size int64) *FileInfo {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Clone returns a snapshot copy of fi. Stat returns Clone() so callers can
|
||||||
|
// read Size and other fields without racing Close/Open mutations of the
|
||||||
|
// in-map FileInfo.
|
||||||
|
func (fi *FileInfo) Clone() *FileInfo {
|
||||||
|
if fi == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
cp := *fi
|
||||||
|
return &cp
|
||||||
|
}
|
||||||
|
|
||||||
// NewFileInfoFromOS creates a FileInfo from os.FileInfo
|
// NewFileInfoFromOS creates a FileInfo from os.FileInfo
|
||||||
func NewFileInfoFromOS(info os.FileInfo, key string) *FileInfo {
|
func NewFileInfoFromOS(info os.FileInfo, key string) *FileInfo {
|
||||||
return &FileInfo{
|
return &FileInfo{
|
||||||
|
|||||||
@@ -16,6 +16,34 @@ func TestNewFileInfo(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestFileInfoClone(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
fi := NewFileInfo("k", 42)
|
||||||
|
fi.AccessCount = 7
|
||||||
|
cp := fi.Clone()
|
||||||
|
if cp == fi {
|
||||||
|
t.Fatal("Clone returned the same pointer")
|
||||||
|
}
|
||||||
|
if cp.Key != fi.Key || cp.Size != fi.Size || cp.AccessCount != fi.AccessCount {
|
||||||
|
t.Errorf("Clone mismatch: %+v vs %+v", cp, fi)
|
||||||
|
}
|
||||||
|
if !cp.ATime.Equal(fi.ATime) || !cp.CTime.Equal(fi.CTime) {
|
||||||
|
t.Error("Clone timestamps mismatch")
|
||||||
|
}
|
||||||
|
cp.Size = 99
|
||||||
|
cp.AccessCount = 1
|
||||||
|
if fi.Size != 42 || fi.AccessCount != 7 {
|
||||||
|
t.Error("mutating Clone affected original")
|
||||||
|
}
|
||||||
|
if NewFileInfo("x", 1).Clone() == nil {
|
||||||
|
t.Error("Clone of non-nil was nil")
|
||||||
|
}
|
||||||
|
var none *FileInfo
|
||||||
|
if none.Clone() != nil {
|
||||||
|
t.Error("Clone of nil was non-nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestUpdateAccess(t *testing.T) {
|
func TestUpdateAccess(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
fi := NewFileInfo("k", 1)
|
fi := NewFileInfo("k", 1)
|
||||||
|
|||||||
Reference in New Issue
Block a user