Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make timeout minimum 1s #380

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions component/file_cache/file_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ const (
defaultMaxThreshold = 80
defaultMinThreshold = 60
defaultFileCacheTimeout = 120
minimumFileCacheTimeout = 1
defaultCacheUpdateCount = 100
MB = 1024 * 1024
)
Expand Down Expand Up @@ -220,9 +221,9 @@ func (c *FileCache) Configure(_ bool) error {

c.createEmptyFile = conf.CreateEmptyFile
if config.IsSet(compName + ".file-cache-timeout-in-seconds") {
c.cacheTimeout = float64(conf.V1Timeout)
c.cacheTimeout = max(float64(conf.V1Timeout), minimumFileCacheTimeout)
} else if config.IsSet(compName + ".timeout-sec") {
c.cacheTimeout = float64(conf.Timeout)
c.cacheTimeout = max(float64(conf.Timeout), minimumFileCacheTimeout)
} else {
c.cacheTimeout = float64(defaultFileCacheTimeout)
}
Expand Down Expand Up @@ -341,7 +342,7 @@ func (c *FileCache) OnConfigChange() {
}

c.createEmptyFile = conf.CreateEmptyFile
c.cacheTimeout = float64(conf.Timeout)
c.cacheTimeout = max(float64(conf.Timeout), minimumFileCacheTimeout)
c.policyTrace = conf.EnablePolicyTrace
c.offloadIO = conf.OffloadIO
c.maxCacheSize = conf.MaxSizeMB
Expand Down
43 changes: 6 additions & 37 deletions component/file_cache/file_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func (suite *fileCacheTestSuite) TestConfigZero() {

suite.assert.Equal(suite.fileCache.createEmptyFile, createEmptyFile)
suite.assert.Equal(suite.fileCache.allowNonEmpty, allowNonEmptyTemp)
suite.assert.EqualValues(suite.fileCache.cacheTimeout, cacheTimeout)
suite.assert.EqualValues(suite.fileCache.cacheTimeout, minimumFileCacheTimeout)
suite.assert.Equal(suite.fileCache.cleanupOnStart, cleanupOnStart)
}

Expand Down Expand Up @@ -936,9 +936,8 @@ func (suite *fileCacheTestSuite) TestCloseFile() {
func (suite *fileCacheTestSuite) TestCloseFileTimeout() {
defer suite.cleanupTest()
suite.cleanupTest() // teardown the default file cache generated
cacheTimeout := 1
config := fmt.Sprintf("file_cache:\n path: %s\n offload-io: true\n timeout-sec: %d\n\nloopbackfs:\n path: %s",
suite.cache_path, cacheTimeout, suite.fake_storage_path)
suite.cache_path, minimumFileCacheTimeout, suite.fake_storage_path)
suite.setupTestHelper(config) // setup a new file cache with a custom config (teardown will occur after the test as usual)

path := "file10"
Expand All @@ -957,7 +956,7 @@ func (suite *fileCacheTestSuite) TestCloseFileTimeout() {

// loop until file does not exist - done due to async nature of eviction
_, err = os.Stat(filepath.Join(suite.cache_path, path))
for i := 0; i < (cacheTimeout*300) && !os.IsNotExist(err); i++ {
for i := 0; i < (minimumFileCacheTimeout*300) && !os.IsNotExist(err); i++ {
time.Sleep(10 * time.Millisecond)
_, err = os.Stat(filepath.Join(suite.cache_path, path))
}
Expand Down Expand Up @@ -993,9 +992,8 @@ func (suite *fileCacheTestSuite) TestOpenPreventsEviction() {
defer suite.cleanupTest()
// Setup
suite.cleanupTest() // teardown the default file cache generated
cacheTimeout := 1
config := fmt.Sprintf("file_cache:\n path: %s\n offload-io: true\n timeout-sec: %d\n\nloopbackfs:\n path: %s",
suite.cache_path, cacheTimeout, suite.fake_storage_path)
suite.cache_path, minimumFileCacheTimeout, suite.fake_storage_path)
suite.setupTestHelper(config) // setup a new file cache with a custom config (teardown will occur after the test as usual)

path := "file12"
Expand All @@ -1013,7 +1011,7 @@ func (suite *fileCacheTestSuite) TestOpenPreventsEviction() {
suite.assert.NoError(err)

// wait until file would be evicted (if not for being opened)
time.Sleep(time.Second * time.Duration(cacheTimeout*3))
time.Sleep(time.Second * time.Duration(minimumFileCacheTimeout*3))

// File should still be in cache
suite.assert.FileExists(filepath.Join(suite.cache_path, path))
Expand Down Expand Up @@ -1382,34 +1380,6 @@ func (suite *fileCacheTestSuite) TestRenameFileAndCacheCleanup() {
suite.assert.FileExists(suite.cache_path + "/" + dst) // Dst shall not exists in cache
}

func (suite *fileCacheTestSuite) TestRenameFileAndCacheCleanupWithNoTimeout() {
defer suite.cleanupTest()

src := "source5"
dst := "destination5"
handle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: src, Mode: 0666})
suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: handle})

// Path _might_ be in the file cache
// Path _should_ be in fake storage
suite.assert.FileExists(suite.fake_storage_path + "/" + src)

// RenameFile
err := suite.fileCache.RenameFile(internal.RenameFileOptions{Src: src, Dst: dst})
suite.assert.NoError(err)

// Dst _might_ exist in cache, and the rename should be complete in fake storage
suite.assert.NoFileExists(suite.fake_storage_path + "/" + src) // Src does not exist
suite.assert.FileExists(suite.fake_storage_path + "/" + dst) // Dst does exist

time.Sleep(200 * time.Millisecond) // Wait for the cache cleanup to occur
// cache should be completely clean
suite.assert.False(suite.fileCache.policy.IsCached(filepath.Join(suite.cache_path, src)))
suite.assert.False(suite.fileCache.policy.IsCached(filepath.Join(suite.cache_path, dst)))
suite.assert.NoFileExists(suite.cache_path + "/" + src)
suite.assert.NoFileExists(suite.cache_path + "/" + dst)
}

func (suite *fileCacheTestSuite) TestTruncateFileNotInCache() {
defer suite.cleanupTest()
// Setup
Expand Down Expand Up @@ -1485,9 +1455,8 @@ func (suite *fileCacheTestSuite) TestTruncateFileCase2() {

func (suite *fileCacheTestSuite) TestZZMountPathConflict() {
defer suite.cleanupTest()
cacheTimeout := 1
configuration := fmt.Sprintf("file_cache:\n path: %s\n offload-io: true\n timeout-sec: %d\n\nloopbackfs:\n path: %s",
suite.cache_path, cacheTimeout, suite.fake_storage_path)
suite.cache_path, minimumFileCacheTimeout, suite.fake_storage_path)

fileCache := NewFileCacheComponent()
config.ReadConfigFromReader(strings.NewReader(configuration))
Expand Down
62 changes: 27 additions & 35 deletions component/file_cache/lru_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ type lruPolicy struct {

const (
// Check for disk usage in below number of minutes
DiskUsageCheckInterval = 1
minimumEvictionInterval = 100 * time.Millisecond
DiskUsageCheckInterval = 1
)

var _ cachePolicy = &lruPolicy{}
Expand Down Expand Up @@ -126,9 +125,8 @@ func (p *lruPolicy) StartPolicy() error {

log.Info("lruPolicy::StartPolicy : Policy set with %v timeout", p.cacheTimeout)

// run the timeout monitor even with timeout set to zero
timeoutInterval := time.Duration(p.cacheTimeout) * time.Second
p.cacheTimeoutMonitor = time.Tick(max(timeoutInterval, minimumEvictionInterval))
// start the timeout monitor
p.cacheTimeoutMonitor = time.Tick(time.Duration(p.cacheTimeout) * time.Second)

go p.clearCache()
go p.asyncCacheValid()
Expand Down Expand Up @@ -230,7 +228,8 @@ func (p *lruPolicy) cacheValidate(name string) {
if node == p.head {
return
}
p.moveToHead(node)
p.extractNode(node)
p.setHead(node)

node.usage++
}
Expand Down Expand Up @@ -297,51 +296,44 @@ func (p *lruPolicy) removeNode(name string) {
node = val.(*lruNode)
node.deleted = true

if node == p.head {
p.head = node.next
p.head.prev = nil
node.next = nil
return
}

if node.next != nil {
node.next.prev = node.prev
}

if node.prev != nil {
node.prev.next = node.next
}
node.prev = nil
node.next = nil
p.extractNode(node)
}

func (p *lruPolicy) updateMarker() {
log.Trace("lruPolicy::updateMarker")

p.Lock()
p.moveToHead(p.lastMarker)
// evict everything when timeout is zero
if p.cacheTimeout == 0 {
p.moveToHead(p.currMarker)
} else {
// swap lastMarker with currMarker
swap := p.lastMarker
p.lastMarker = p.currMarker
p.currMarker = swap
}
p.extractNode(p.lastMarker)
p.setHead(p.lastMarker)
// swap lastMarker with currMarker
swap := p.lastMarker
p.lastMarker = p.currMarker
p.currMarker = swap

p.Unlock()
}

func (p *lruPolicy) moveToHead(node *lruNode) {
// remove the node from its position
func (p *lruPolicy) extractNode(node *lruNode) {
// remove the node from its position in the list

// head case
if node == p.head {
p.head = node.next
}

if node.next != nil {
node.next.prev = node.prev
}
if node.prev != nil {
node.prev.next = node.next
}
// and insert it at the head

node.prev = nil
node.next = nil
}

func (p *lruPolicy) setHead(node *lruNode) {
// insert node at the head
node.prev = nil
node.next = p.head
p.head.prev = node
Expand Down
2 changes: 1 addition & 1 deletion setup/baseConfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ file_cache:

# Attribute cache related configuration
attr_cache:
timeout-sec: <time attributes and directory contents can be cached (in sec). Default - 120 sec>
timeout-sec: <time attributes and directory contents can be cached (in sec). Minimum is 1. Default - 120 sec>
no-cache-on-list: true|false <do not cache attributes or directory contents during listing. Enabling may cause performance problems.>
enable-symlinks: true|false <enable symlink support. When false, symlinks will be treated like regular files. Enabling may cause performance problems.>
max-files: <maximum number of files in the attribute cache at a time. Default - 5000000>
Expand Down
Loading