bugfix(client): Fix the logic of returning the cache to the cache pool

1.In cold volume scenario, fwriter is not released when file is opened in write mode first and then opened in read-only mode.So when file is closed, fwriter's buf is returned to cache pool twice. This will result in subsequent write operations obtaining two identical buffer addresses from the cache pool. 2.CloseStream function will return fileWriter's buf to cache pool to avoid OOM. close: cubefs#2258 cubefs#2257 Signed-off-by: chihe <[email protected]>
blissnmx · Jul 20, 2023 · 13e9526 · 13e9526
1 parent 3468bbd
commit 13e9526
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 6 deletions.
diff --git a/client/fs/file.go b/client/fs/file.go
@@ -97,7 +97,6 @@ func NewFile(s *Super, i *proto.InodeInfo, flag uint32, pino uint64, filename st
 			fReader = blobstore.NewReader(clientConf)
 		case syscall.O_WRONLY:
 			fWriter = blobstore.NewWriter(clientConf)
-
 		case syscall.O_RDWR:
 			fReader = blobstore.NewReader(clientConf)
 			fWriter = blobstore.NewWriter(clientConf)
@@ -255,15 +254,20 @@ func (f *File) Open(ctx context.Context, req *fuse.OpenRequest, resp *fuse.OpenR
 			FileSize:        uint64(fileSize),
 			CacheThreshold:  f.super.CacheThreshold,
 		}
-
+		f.fWriter.FreeCache()
 		switch req.Flags & 0x0f {
 		case syscall.O_RDONLY:
 			f.fReader = blobstore.NewReader(clientConf)
+			f.fWriter = nil
 		case syscall.O_WRONLY:
 			f.fWriter = blobstore.NewWriter(clientConf)
+			f.fReader = nil
 		case syscall.O_RDWR:
 			f.fReader = blobstore.NewReader(clientConf)
 			f.fWriter = blobstore.NewWriter(clientConf)
+		default:
+			f.fWriter = blobstore.NewWriter(clientConf)
+			f.fReader = nil
 		}
 		log.LogDebugf("TRACE file open,ino(%v)  req.Flags(%v) reader(%v)  writer(%v)", ino, req.Flags, f.fReader, f.fWriter)
 	}

diff --git a/libsdk/libsdk.go b/libsdk/libsdk.go
@@ -1289,17 +1289,20 @@ func (c *client) allocFD(ino uint64, flags, mode uint32, fileCache bool, fileSiz
 			FileSize:        fileSize,
 			CacheThreshold:  c.cacheThreshold,
 		}
-
+		f.fileWriter.FreeCache()
 		switch flags & 0xff {
 		case syscall.O_RDONLY:
 			f.fileReader = blobstore.NewReader(clientConf)
+			f.fileWriter = nil
 		case syscall.O_WRONLY:
 			f.fileWriter = blobstore.NewWriter(clientConf)
+			f.fileReader = nil
 		case syscall.O_RDWR:
 			f.fileReader = blobstore.NewReader(clientConf)
 			f.fileWriter = blobstore.NewWriter(clientConf)
 		default:
 			f.fileWriter = blobstore.NewWriter(clientConf)
+			f.fileReader = nil
 		}
 	}
 	c.fdmap[fd] = f
@@ -1377,6 +1380,7 @@ func (c *client) openStream(f *file) {
 func (c *client) closeStream(f *file) {
 	_ = c.ec.CloseStream(f.ino)
 	_ = c.ec.EvictStream(f.ino)
+	f.fileWriter.FreeCache()
 	f.fileWriter = nil
 	f.fileReader = nil
 }

diff --git a/sdk/data/blobstore/writer.go b/sdk/data/blobstore/writer.go
@@ -93,7 +93,7 @@ func NewWriter(config ClientConfig) (writer *Writer) {
 	writer.fileSize = config.FileSize
 	writer.cacheThreshold = config.CacheThreshold
 	writer.dirty = false
-	writer.AllocateCache()
+	writer.allocateCache()
 	writer.limitManager = writer.ec.LimitManager
 
 	return
@@ -616,13 +616,22 @@ func (writer *Writer) CacheFileSize() int {
 }
 
 func (writer *Writer) FreeCache() {
+	if writer == nil {
+		return
+	}
 	if buf.CachePool == nil {
 		return
 	}
-	buf.CachePool.Put(writer.buf)
+	writer.once.Do(func() {
+		tmpBuf := writer.buf
+		writer.buf = nil
+		if tmpBuf != nil {
+			buf.CachePool.Put(tmpBuf)
+		}
+	})
 }
 
-func (writer *Writer) AllocateCache() {
+func (writer *Writer) allocateCache() {
 	if buf.CachePool == nil {
 		return
 	}