Skip to content

Commit 260f4b3

Browse files
authored
revert: remove MFS auto-flush mechanism (#1041)
removes the cache auto-flush feature from #1037 that could cause data corruption with parallel MFS operations. letting applications implement their own safety mechanisms is safer than enforcing automatic flushing in the library. related to ipfs/kubo#10842
1 parent 6084860 commit 260f4b3

File tree

3 files changed

+0
-73
lines changed

3 files changed

+0
-73
lines changed

CHANGELOG.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ The following emojis are used to highlight certain changes:
5858
- `DagModifier` now supports appending data to a `RawNode` by automatically converting it into a UnixFS file structure where the original `RawNode` becomes the first leaf block, fixing previously impossible append operations that would fail with "expected protobuf dag node" errors
5959
- `mfs`:
6060
- Files with identity CIDs now properly inherit full CID prefix from parent directories (version, codec, hash type, length), not just hash type ([#1018](https://github.com/ipfs/boxo/pull/1018))
61-
- Fixed unbounded memory growth when using deferred flushing and user forgets to flush manually. Added `SetMaxCacheSize()` to limit directory cache growth. Default 256 entries, set to 0 to disable. ([#1035](https://github.com/ipfs/boxo/pull/1035))
6261

6362
### Security
6463

mfs/dir.go

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,6 @@ var (
2323
ErrDirExists = errors.New("directory already has entry by that name")
2424
)
2525

26-
const (
27-
// DefaultMaxCacheSize is the default maximum number of entries
28-
// that can be cached in a directory before auto-flush is triggered.
29-
// This prevents unbounded memory growth when using --flush=false.
30-
// The value matches HAMT shard size (256).
31-
// TODO: make this configurable
32-
// See https://github.com/ipfs/kubo/issues/10842
33-
DefaultMaxCacheSize = 256
34-
)
35-
3626
// TODO: There's too much functionality associated with this structure,
3727
// let's organize it (and if possible extract part of it elsewhere)
3828
// and document the main features of `Directory` here.
@@ -52,10 +42,6 @@ type Directory struct {
5242
// reading and editing directories.
5343
unixfsDir uio.Directory
5444

55-
// Maximum number of entries to cache before triggering auto-flush.
56-
// Set to 0 to disable cache size limiting.
57-
maxCacheSize int
58-
5945
prov provider.MultihashProvider
6046
}
6147

@@ -96,7 +82,6 @@ func NewDirectory(ctx context.Context, name string, node ipld.Node, parent paren
9682
unixfsDir: db,
9783
prov: prov,
9884
entriesCache: make(map[string]FSNode),
99-
maxCacheSize: DefaultMaxCacheSize,
10085
}, nil
10186
}
10287

@@ -136,7 +121,6 @@ func NewEmptyDirectory(ctx context.Context, name string, parent parent, dserv ip
136121
unixfsDir: db,
137122
prov: prov,
138123
entriesCache: make(map[string]FSNode),
139-
maxCacheSize: DefaultMaxCacheSize,
140124
}, nil
141125
}
142126

@@ -232,25 +216,14 @@ func (d *Directory) cacheNode(name string, nd ipld.Node) (FSNode, error) {
232216
// inherited from the parent.
233217
ndir.unixfsDir.SetMaxLinks(d.unixfsDir.GetMaxLinks())
234218
ndir.unixfsDir.SetMaxHAMTFanout(d.unixfsDir.GetMaxHAMTFanout())
235-
// Inherit cache size limit from parent
236-
ndir.maxCacheSize = d.maxCacheSize
237-
238219
d.entriesCache[name] = ndir
239-
// Check cache size after adding entry
240-
if err := d.checkCacheSize(); err != nil {
241-
return nil, err
242-
}
243220
return ndir, nil
244221
case ft.TFile, ft.TRaw, ft.TSymlink:
245222
nfi, err := NewFile(name, nd, d, d.dagService, d.prov)
246223
if err != nil {
247224
return nil, err
248225
}
249226
d.entriesCache[name] = nfi
250-
// Check cache size after adding entry
251-
if err := d.checkCacheSize(); err != nil {
252-
return nil, err
253-
}
254227
return nfi, nil
255228
case ft.TMetadata:
256229
return nil, ErrNotYetImplemented
@@ -263,10 +236,6 @@ func (d *Directory) cacheNode(name string, nd ipld.Node) (FSNode, error) {
263236
return nil, err
264237
}
265238
d.entriesCache[name] = nfi
266-
// Check cache size after adding entry
267-
if err := d.checkCacheSize(); err != nil {
268-
return nil, err
269-
}
270239
return nfi, nil
271240
default:
272241
return nil, errors.New("unrecognized node type in cache node")
@@ -404,9 +373,6 @@ func (d *Directory) MkdirWithOpts(name string, opts MkdirOpts) (*Directory, erro
404373
return nil, err
405374
}
406375

407-
// Inherit cache size limit from parent
408-
dirobj.maxCacheSize = d.maxCacheSize
409-
410376
ndir, err := dirobj.GetNode()
411377
if err != nil {
412378
return nil, err
@@ -418,40 +384,9 @@ func (d *Directory) MkdirWithOpts(name string, opts MkdirOpts) (*Directory, erro
418384
}
419385

420386
d.entriesCache[name] = dirobj
421-
422-
// Check cache size after adding new directory
423-
if err := d.checkCacheSize(); err != nil {
424-
return nil, err
425-
}
426-
427387
return dirobj, nil
428388
}
429389

430-
// checkCacheSize checks if the cache has exceeded the maximum size
431-
// and triggers an auto-flush if needed to prevent unbounded growth.
432-
// Must be called with d.lock held.
433-
func (d *Directory) checkCacheSize() error {
434-
// Skip check if cache limiting is disabled (maxCacheSize == 0)
435-
if d.maxCacheSize > 0 && len(d.entriesCache) >= d.maxCacheSize {
436-
// Auto-flush to prevent unbounded cache growth
437-
log.Debugf("mfs: auto-flushing directory cache (size: %d >= limit: %d)", len(d.entriesCache), d.maxCacheSize)
438-
err := d.cacheSync(true)
439-
if err != nil {
440-
return err
441-
}
442-
}
443-
return nil
444-
}
445-
446-
// SetMaxCacheSize sets the maximum number of entries to cache before
447-
// triggering an auto-flush. Set to 0 to disable cache size limiting.
448-
// This method is thread-safe.
449-
func (d *Directory) SetMaxCacheSize(size int) {
450-
d.lock.Lock()
451-
defer d.lock.Unlock()
452-
d.maxCacheSize = size
453-
}
454-
455390
func (d *Directory) Unlink(name string) error {
456391
d.lock.Lock()
457392
defer d.lock.Unlock()

mfs/root.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,6 @@ func (kr *Root) GetDirectory() *Directory {
170170
return kr.dir
171171
}
172172

173-
// SetMaxCacheSize sets the maximum number of entries to cache in each
174-
// directory before triggering an auto-flush. Set to 0 to disable cache
175-
// size limiting. This setting is propagated to all directories in the tree.
176-
func (kr *Root) SetMaxCacheSize(size int) {
177-
kr.dir.SetMaxCacheSize(size)
178-
}
179-
180173
// Flush signals that an update has occurred since the last publish,
181174
// and updates the Root republisher.
182175
// TODO: We are definitely abusing the "flush" terminology here.

0 commit comments

Comments
 (0)