From daa0ab75b49a0e09b144e7e0369cf15d5d030f47 Mon Sep 17 00:00:00 2001 From: DaneEveritt Date: Sat, 29 Oct 2022 13:01:04 -0700 Subject: [PATCH] only use vhd when there is a disk limit configured --- server/filesystem/disk_space.go | 59 +++++++++++++++++++-------------- server/filesystem/filesystem.go | 11 +++--- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/server/filesystem/disk_space.go b/server/filesystem/disk_space.go index 67c8c6b..db36a8f 100644 --- a/server/filesystem/disk_space.go +++ b/server/filesystem/disk_space.go @@ -45,8 +45,8 @@ func (fs *Filesystem) SetDiskLimit(i int64) { atomic.SwapInt64(&fs.diskLimit, i) } -// The same concept as HasSpaceAvailable however this will return an error if there is -// no space, rather than a boolean value. +// HasSpaceErr is the same concept as HasSpaceAvailable however this will return +// an error if there is no space, rather than a boolean value. func (fs *Filesystem) HasSpaceErr(allowStaleValue bool) error { if !fs.HasSpaceAvailable(allowStaleValue) { return newFilesystemError(ErrCodeDiskSpace, nil) @@ -54,30 +54,32 @@ func (fs *Filesystem) HasSpaceErr(allowStaleValue bool) error { return nil } -// Determines if the directory a file is trying to be added to has enough space available -// for the file to be written to. +// HasSpaceAvailable determines if the directory a file is trying to be added to +// has enough space available for the file to be written to. // -// Because determining the amount of space being used by a server is a taxing operation we -// will load it all up into a cache and pull from that as long as the key is not expired. +// Because determining the amount of space being used by a server is a taxing +// operation we will load it all up into a cache and pull from that as long as +// the key is not expired. This operation will potentially block unless +// allowStaleValue is set to true. See the documentation on DiskUsage for how +// this affects the call. // -// This operation will potentially block unless allowStaleValue is set to true. See the -// documentation on DiskUsage for how this affects the call. +// If the current size of the disk is larger than the maximum allowed size this +// function will return false, in all other cases it will return true. We do +// not check the existence of a virtual disk at this point since this logic is +// used to return friendly error messages to users, and also prevent us wasting +// time on more taxing operations when we know the result will end up failing due +// to space limits. +// +// If the servers disk limit is set to 0 it means there is no limit, however the +// DiskUsage method is still called to keep the cache warm. This function will +// always return true for a server with no limit set. func (fs *Filesystem) HasSpaceAvailable(allowStaleValue bool) bool { size, err := fs.DiskUsage(allowStaleValue) if err != nil { - log.WithField("root", fs.root).WithField("error", err).Warn("failed to determine root fs directory size") + log.WithField("root", fs.root).WithField("error", err). + Warn("failed to determine root fs directory size") } - - // If space is -1 or 0 just return true, means they're allowed unlimited. - // - // Technically we could skip disk space calculation because we don't need to check if the - // server exceeds its limit but because this method caches the disk usage it would be best - // to calculate the disk usage and always return true. - if fs.MaxDisk() == 0 { - return true - } - - return size <= fs.MaxDisk() + return fs.MaxDisk() == 0 || size <= fs.MaxDisk() } // CachedUsage returns the cached value for the amount of disk space used by the @@ -107,17 +109,24 @@ func (fs *Filesystem) DiskUsage(allowStaleValue bool) (int64, error) { return 0, nil } - if !fs.lastLookupTime.Get().After(time.Now().Add(time.Second * fs.diskCheckInterval * -1)) { + since := time.Now().Add(time.Second * fs.diskCheckInterval * -1) + // If the last lookup time was before our calculated limit we will re-execute this + // checking logic. If the lookup time was after the oldest possible timestamp we will + // continue returning the cached value. + if fs.lastLookupTime.Get().Before(since) { // If we are now allowing a stale response go ahead and perform the lookup and return the fresh // value. This is a blocking operation to the calling process. if !allowStaleValue { return fs.updateCachedDiskUsage() - } else if !fs.lookupInProgress.Load() { - // Otherwise, if we allow a stale value and there isn't a valid item in the cache and we aren't - // currently performing a lookup, just do the disk usage calculation in the background. + } + + // Otherwise, if we allow a stale value and there isn't a valid item in the cache and we aren't + // currently performing a lookup, just do the disk usage calculation in the background. + if !fs.lookupInProgress.Load() { go func(fs *Filesystem) { if _, err := fs.updateCachedDiskUsage(); err != nil { - log.WithField("root", fs.root).WithField("error", err).Warn("failed to update fs disk usage from within routine") + log.WithField("root", fs.root).WithField("error", err). + Warn("failed to update fs disk usage from within routine") } }(fs) } diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go index 80cd15b..3243f62 100644 --- a/server/filesystem/filesystem.go +++ b/server/filesystem/filesystem.go @@ -54,7 +54,10 @@ func New(uuid string, size int64, denylist []string) *Filesystem { denylist: ignore.CompileIgnoreLines(denylist...), } - if vhd.Enabled() { + // If VHD support is enabled but this server is configured with no disk size + // limit we cannot actually use a virtual disk. In that case fall back to using + // the default driver. + if vhd.Enabled() && size > 0 { fs.vhd = vhd.New(size, vhd.DiskPath(uuid), fs.root) } @@ -86,9 +89,9 @@ func (fs *Filesystem) File(p string) (*os.File, Stat, error) { return f, st, nil } -// Acts by creating the given file and path on the disk if it is not present already. If -// it is present, the file is opened using the defaults which will truncate the contents. -// The opened file is then returned to the caller. +// Touch acts by creating the given file and path on the disk if it is not present +// already. If it is present, the file is opened using the defaults which will +// truncate the contents. The opened file is then returned to the caller. func (fs *Filesystem) Touch(p string, flag int) (*os.File, error) { cleaned, err := fs.SafePath(p) if err != nil {