mirror of
https://github.com/octoleo/syncthing.git
synced 2024-12-22 19:08:58 +00:00
chore(fs): only cache the cache for case FS, not the entire FS (#9701)
This would have addressed a recent issue that arose when re-ordering our "filesystem layers". Specifically moving the caseFilesystem to the outermost layer. The previous cache included the filesystem, and as such all the layers below. This isn't desirable (to put it mildly), as you can create different variants of filesystems with different layers for the same path and options. Concretely this did happen with the mtime layer, which isn't always present. A test for the mtime related breakage was added in #9687, and I intend to redo the caseFilesystem reordering after this. Ref: #9677 Followup to: #9687
This commit is contained in:
parent
0ea90dd932
commit
ac8b3342ac
@ -50,7 +50,7 @@ type fskey struct {
|
|||||||
// caseFilesystemRegistry caches caseFilesystems and runs a routine to drop
|
// caseFilesystemRegistry caches caseFilesystems and runs a routine to drop
|
||||||
// their cache every now and then.
|
// their cache every now and then.
|
||||||
type caseFilesystemRegistry struct {
|
type caseFilesystemRegistry struct {
|
||||||
fss map[fskey]*caseFilesystem
|
caseCaches map[fskey]*caseCache
|
||||||
mut sync.RWMutex
|
mut sync.RWMutex
|
||||||
startCleaner sync.Once
|
startCleaner sync.Once
|
||||||
}
|
}
|
||||||
@ -77,18 +77,15 @@ func (r *caseFilesystemRegistry) get(fs Filesystem) Filesystem {
|
|||||||
// take a write lock and try again.
|
// take a write lock and try again.
|
||||||
|
|
||||||
r.mut.RLock()
|
r.mut.RLock()
|
||||||
caseFs, ok := r.fss[k]
|
cache, ok := r.caseCaches[k]
|
||||||
r.mut.RUnlock()
|
r.mut.RUnlock()
|
||||||
|
|
||||||
if !ok {
|
if !ok {
|
||||||
r.mut.Lock()
|
r.mut.Lock()
|
||||||
caseFs, ok = r.fss[k]
|
cache, ok = r.caseCaches[k]
|
||||||
if !ok {
|
if !ok {
|
||||||
caseFs = &caseFilesystem{
|
cache = newCaseCache()
|
||||||
Filesystem: fs,
|
r.caseCaches[k] = cache
|
||||||
realCaser: newDefaultRealCaser(fs),
|
|
||||||
}
|
|
||||||
r.fss[k] = caseFs
|
|
||||||
r.startCleaner.Do(func() {
|
r.startCleaner.Do(func() {
|
||||||
go r.cleaner()
|
go r.cleaner()
|
||||||
})
|
})
|
||||||
@ -96,7 +93,13 @@ func (r *caseFilesystemRegistry) get(fs Filesystem) Filesystem {
|
|||||||
r.mut.Unlock()
|
r.mut.Unlock()
|
||||||
}
|
}
|
||||||
|
|
||||||
return caseFs
|
return &caseFilesystem{
|
||||||
|
Filesystem: fs,
|
||||||
|
realCaser: &defaultRealCaser{
|
||||||
|
fs: fs,
|
||||||
|
cache: cache,
|
||||||
|
},
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *caseFilesystemRegistry) cleaner() {
|
func (r *caseFilesystemRegistry) cleaner() {
|
||||||
@ -109,19 +112,19 @@ func (r *caseFilesystemRegistry) cleaner() {
|
|||||||
// within the loop.
|
// within the loop.
|
||||||
|
|
||||||
r.mut.RLock()
|
r.mut.RLock()
|
||||||
toProcess := make([]*caseFilesystem, 0, len(r.fss))
|
toProcess := make([]*caseCache, 0, len(r.caseCaches))
|
||||||
for _, caseFs := range r.fss {
|
for _, cache := range r.caseCaches {
|
||||||
toProcess = append(toProcess, caseFs)
|
toProcess = append(toProcess, cache)
|
||||||
}
|
}
|
||||||
r.mut.RUnlock()
|
r.mut.RUnlock()
|
||||||
|
|
||||||
for _, caseFs := range toProcess {
|
for _, cache := range toProcess {
|
||||||
caseFs.dropCache()
|
cache.Purge()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
var globalCaseFilesystemRegistry = caseFilesystemRegistry{fss: make(map[fskey]*caseFilesystem)}
|
var globalCaseFilesystemRegistry = caseFilesystemRegistry{caseCaches: make(map[fskey]*caseCache)}
|
||||||
|
|
||||||
// OptionDetectCaseConflicts ensures that the potentially case-insensitive filesystem
|
// OptionDetectCaseConflicts ensures that the potentially case-insensitive filesystem
|
||||||
// behaves like a case-sensitive filesystem. Meaning that it takes into account
|
// behaves like a case-sensitive filesystem. Meaning that it takes into account
|
||||||
@ -392,21 +395,20 @@ func (f *caseFilesystem) checkCaseExisting(name string) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type defaultRealCaser struct {
|
type defaultRealCaser struct {
|
||||||
cache caseCache
|
cache *caseCache
|
||||||
|
fs Filesystem
|
||||||
|
mut sync.Mutex
|
||||||
}
|
}
|
||||||
|
|
||||||
func newDefaultRealCaser(fs Filesystem) *defaultRealCaser {
|
type caseCache = lru.TwoQueueCache[string, *caseNode]
|
||||||
|
|
||||||
|
func newCaseCache() *caseCache {
|
||||||
cache, err := lru.New2Q[string, *caseNode](caseCacheItemLimit)
|
cache, err := lru.New2Q[string, *caseNode](caseCacheItemLimit)
|
||||||
// New2Q only errors if given invalid parameters, which we don't.
|
// New2Q only errors if given invalid parameters, which we don't.
|
||||||
if err != nil {
|
if err != nil {
|
||||||
panic(err)
|
panic(err)
|
||||||
}
|
}
|
||||||
return &defaultRealCaser{
|
return cache
|
||||||
cache: caseCache{
|
|
||||||
fs: fs,
|
|
||||||
TwoQueueCache: cache,
|
|
||||||
},
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *defaultRealCaser) realCase(name string) (string, error) {
|
func (r *defaultRealCaser) realCase(name string) (string, error) {
|
||||||
@ -416,7 +418,7 @@ func (r *defaultRealCaser) realCase(name string) (string, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for _, comp := range PathComponents(name) {
|
for _, comp := range PathComponents(name) {
|
||||||
node := r.cache.getExpireAdd(realName)
|
node := r.getExpireAdd(realName)
|
||||||
|
|
||||||
if node.err != nil {
|
if node.err != nil {
|
||||||
return "", node.err
|
return "", node.err
|
||||||
@ -440,26 +442,20 @@ func (r *defaultRealCaser) dropCache() {
|
|||||||
r.cache.Purge()
|
r.cache.Purge()
|
||||||
}
|
}
|
||||||
|
|
||||||
type caseCache struct {
|
|
||||||
*lru.TwoQueueCache[string, *caseNode]
|
|
||||||
fs Filesystem
|
|
||||||
mut sync.Mutex
|
|
||||||
}
|
|
||||||
|
|
||||||
// getExpireAdd gets an entry for the given key. If no entry exists, or it is
|
// getExpireAdd gets an entry for the given key. If no entry exists, or it is
|
||||||
// expired a new one is created and added to the cache.
|
// expired a new one is created and added to the cache.
|
||||||
func (c *caseCache) getExpireAdd(key string) *caseNode {
|
func (r *defaultRealCaser) getExpireAdd(key string) *caseNode {
|
||||||
c.mut.Lock()
|
r.mut.Lock()
|
||||||
defer c.mut.Unlock()
|
defer r.mut.Unlock()
|
||||||
node, ok := c.Get(key)
|
node, ok := r.cache.Get(key)
|
||||||
if !ok {
|
if !ok {
|
||||||
node := newCaseNode(key, c.fs)
|
node := newCaseNode(key, r.fs)
|
||||||
c.Add(key, node)
|
r.cache.Add(key, node)
|
||||||
return node
|
return node
|
||||||
}
|
}
|
||||||
if node.expires.Before(time.Now()) {
|
if node.expires.Before(time.Now()) {
|
||||||
node = newCaseNode(key, c.fs)
|
node = newCaseNode(key, r.fs)
|
||||||
c.Add(key, node)
|
r.cache.Add(key, node)
|
||||||
}
|
}
|
||||||
return node
|
return node
|
||||||
}
|
}
|
||||||
|
@ -175,7 +175,10 @@ func BenchmarkWalkCaseFakeFS100k(b *testing.B) {
|
|||||||
// Construct the casefs manually or it will get cached and the benchmark is invalid.
|
// Construct the casefs manually or it will get cached and the benchmark is invalid.
|
||||||
casefs := &caseFilesystem{
|
casefs := &caseFilesystem{
|
||||||
Filesystem: fsys,
|
Filesystem: fsys,
|
||||||
realCaser: newDefaultRealCaser(fsys),
|
realCaser: &defaultRealCaser{
|
||||||
|
fs: fsys,
|
||||||
|
cache: newCaseCache(),
|
||||||
|
},
|
||||||
}
|
}
|
||||||
var fakefs *fakeFS
|
var fakefs *fakeFS
|
||||||
if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok {
|
if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok {
|
||||||
@ -201,7 +204,10 @@ func BenchmarkWalkCaseFakeFS100k(b *testing.B) {
|
|||||||
// Construct the casefs manually or it will get cached and the benchmark is invalid.
|
// Construct the casefs manually or it will get cached and the benchmark is invalid.
|
||||||
casefs := &caseFilesystem{
|
casefs := &caseFilesystem{
|
||||||
Filesystem: fsys,
|
Filesystem: fsys,
|
||||||
realCaser: newDefaultRealCaser(fsys),
|
realCaser: &defaultRealCaser{
|
||||||
|
fs: fsys,
|
||||||
|
cache: newCaseCache(),
|
||||||
|
},
|
||||||
}
|
}
|
||||||
var fakefs *fakeFS
|
var fakefs *fakeFS
|
||||||
if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok {
|
if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok {
|
||||||
|
@ -221,7 +221,7 @@ func TestRepro9677MissingMtimeFS(t *testing.T) {
|
|||||||
|
|
||||||
// Now syncthing gets upgraded (or even just restarted), which resets the
|
// Now syncthing gets upgraded (or even just restarted), which resets the
|
||||||
// case FS registry as it lives in memory.
|
// case FS registry as it lives in memory.
|
||||||
globalCaseFilesystemRegistry = caseFilesystemRegistry{fss: make(map[fskey]*caseFilesystem)}
|
globalCaseFilesystemRegistry = caseFilesystemRegistry{caseCaches: make(map[fskey]*caseCache)}
|
||||||
|
|
||||||
// This time we first create some filesystem without a database and thus no
|
// This time we first create some filesystem without a database and thus no
|
||||||
// mtime-FS, which is used in various places outside of the folder code. We
|
// mtime-FS, which is used in various places outside of the folder code. We
|
||||||
|
Loading…
Reference in New Issue
Block a user