bug in block cache open call

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 2.4.1 (Unreleased)
 **Bug Fixes**
 - Create block pool only in the child process.
+- Prevent the block cache to truncate the file size to zero when the file is opened in O_WRONLY mode when writebackcache is disabled.
 - Correct statFS results to reflect block-cache in memory cache status.
 
 **Other Changes**

component/block_cache/block_cache.go

@@ -388,7 +388,7 @@ func (bc *BlockCache) CreateFile(options internal.CreateFileOptions) (*handlemap
 
 // OpenFile: Create a handle for the file user has requested to open
 func (bc *BlockCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Handle, error) {
-	log.Trace("BlockCache::OpenFile : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode)
+	log.Trace("BlockCache::OpenFile : name=%s, flags=%X, mode=%s", options.Name, options.Flags, options.Mode)
 
 	attr, err := bc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name})
 	if err != nil {
@@ -403,7 +403,7 @@ func (bc *BlockCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Han
 	log.Debug("BlockCache::OpenFile : Size of file handle.Size %v", handle.Size)
 	bc.prepareHandleForBlockCache(handle)
 
-	if options.Flags&os.O_TRUNC != 0 || (options.Flags&os.O_WRONLY != 0 && options.Flags&os.O_APPEND == 0) {
+	if options.Flags&os.O_TRUNC != 0 {
 		// If file is opened in truncate or wronly mode then we need to wipe out the data consider current file size as 0
 		log.Debug("BlockCache::OpenFile : Truncate %v to 0", options.Name)
 		handle.Size = 0

component/block_cache/block_cache_test.go

@@ -2678,6 +2678,79 @@ func (suite *blockCacheTestSuite) TestZZZZZStreamToBlockCacheConfig() {
 	}
 }
 
+func (suite *blockCacheTestSuite) TestSizeOfFileInOpen() {
+	// Write-back cache is turned on by default while mounting.
+	config := "block_cache:\n  block-size-mb: 1\n  mem-size-mb: 20\n  prefetch: 12\n  parallelism: 1"
+	tobj, err := setupPipeline(config)
+	suite.assert.Nil(err)
+	defer tobj.cleanupPipeline()
+
+	path := getTestFileName(suite.T().Name())
+	storagePath := filepath.Join(tobj.fake_storage_path, path)
+	localPath := filepath.Join(tobj.disk_cache_path, path)
+
+	// ------------------------------------------------------------------
+	// Create a local file
+	fh, err := os.Create(localPath)
+	suite.assert.Nil(err)
+
+	// write 1MB data at offset 0
+	n, err := fh.WriteAt(dataBuff[:_1MB], 0)
+	suite.assert.Nil(err)
+	suite.assert.Equal(n, int(_1MB))
+
+	err = fh.Close()
+	suite.assert.Nil(err)
+	// ------------------------------------------------------------------
+	// Create a file using Mountpoint
+	options := internal.CreateFileOptions{Name: path, Mode: 0777}
+	h, err := tobj.blockCache.CreateFile(options)
+	suite.assert.Nil(err)
+	suite.assert.NotNil(h)
+	suite.assert.Equal(h.Size, int64(0))
+	suite.assert.False(h.Dirty())
+
+	// write 1MB data at offset 0
+	n, err = tobj.blockCache.WriteFile(internal.WriteFileOptions{Handle: h, Offset: 0, Data: dataBuff[:_1MB]})
+	suite.assert.Nil(err)
+	suite.assert.Equal(n, int(_1MB))
+	suite.assert.True(h.Dirty())
+
+	err = tobj.blockCache.CloseFile(internal.CloseFileOptions{Handle: h})
+	suite.assert.Nil(err)
+	//---------------------------------------------------------------------
+
+	//Open and close the file using the given flag in local and mountpoint and
+	// check the size is same or not.
+	check := func(flag int) int {
+		lfh, err := os.OpenFile(localPath, flag, 0666)
+		suite.assert.Nil(err)
+		suite.assert.NotNil(lfh)
+		err = lfh.Close()
+		suite.assert.Nil(err)
+
+		openFileOptions := internal.OpenFileOptions{Name: path, Flags: flag, Mode: 0777}
+		rfh, err := tobj.blockCache.OpenFile(openFileOptions)
+		suite.assert.Nil(err)
+		err = tobj.blockCache.CloseFile(internal.CloseFileOptions{Handle: rfh})
+		suite.assert.Nil(err)
+
+		statInfoLocal, err := os.Stat(localPath)
+		suite.assert.Nil(err)
+		sizeInLocal := statInfoLocal.Size()
+
+		statInfoMount, err := os.Stat(storagePath)
+		suite.assert.Nil(err)
+		sizeInMount := statInfoMount.Size()
+		suite.assert.Equal(sizeInLocal, sizeInMount)
+		return int(sizeInLocal)
+	}
+	size := check(os.O_WRONLY) // size of the file would be 1MB
+	suite.assert.Equal(size, int(_1MB))
+	size = check(os.O_TRUNC) // size of the file would be zero here.
+	suite.assert.Equal(size, int(0))
+}
+
 func (suite *blockCacheTestSuite) TestStrongConsistency() {
 	tobj, err := setupPipeline("")
 	defer tobj.cleanupPipeline()

component/loopback/loopback_fs.go

@@ -478,6 +478,13 @@ func (lfs *LoopbackFS) CommitData(options internal.CommitDataOptions) error {
 		return err
 	}
 
+	if len(options.List) == 0 {
+		err = blob.Truncate(0)
+		if err != nil {
+			return err
+		}
+	}
+
 	for idx, id := range options.List {
 		path := fmt.Sprintf("%s_%s", filepath.Join(lfs.path, options.Name), strings.ReplaceAll(id, "/", "_"))
 		info, err := os.Lstat(path)

component/loopback/loopback_fs_test.go

@@ -309,6 +309,30 @@ func (suite *LoopbackFSTestSuite) TestStageAndCommitData() {
 	assert.Nil(err)
 }
 
+// This test is for opening the file in O_TRUNC on the existing file
+// must result in resetting the filesize to 0
+func (suite *LoopbackFSTestSuite) TestCommitNilDataToExistingFile() {
+	defer suite.cleanupTest()
+	assert := assert.New(suite.T())
+
+	lfs := &LoopbackFS{}
+
+	lfs.path = common.ExpandPath("~/blocklfstest")
+	err := os.MkdirAll(lfs.path, os.FileMode(0777))
+	assert.Nil(err)
+	defer os.RemoveAll(lfs.path)
+	Filepath := filepath.Join(lfs.path, "testFile")
+	os.WriteFile(Filepath, []byte("hello"), 0777)
+
+	blockList := []string{}
+	err = lfs.CommitData(internal.CommitDataOptions{Name: "testFile", List: blockList})
+	assert.Nil(err)
+
+	info, err := os.Stat(Filepath)
+	assert.Nil(err)
+	assert.Equal(info.Size(), int64(0))
+}
+
 func TestLoopbackFSTestSuite(t *testing.T) {
 	suite.Run(t, new(LoopbackFSTestSuite))
 }