Utils: Renaming files across devices

config/localTemplate.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/algorand/go-algorand/protocol"
+	"github.com/algorand/go-algorand/util"
 	"github.com/algorand/go-algorand/util/codecs"
 )
 
@@ -893,7 +894,7 @@ func moveDirIfExists(logger logger, srcdir, dstdir string, files ...string) erro
 	// then, check if any files exist in srcdir, and move them to dstdir
 	for _, file := range files {
 		if _, err := os.Stat(filepath.Join(srcdir, file)); err == nil {
-			if err := os.Rename(filepath.Join(srcdir, file), filepath.Join(dstdir, file)); err != nil {
+			if err := util.MoveFile(filepath.Join(srcdir, file), filepath.Join(dstdir, file)); err != nil {
 				return fmt.Errorf("failed to move file %s from %s to %s: %v", file, srcdir, dstdir, err)
 			}
 			logger.Infof("Moved DB file %s from ColdDataDir %s to HotDataDir %s", file, srcdir, dstdir)

logging/cyclicWriter.go

@@ -26,6 +26,7 @@ import (
 	"text/template"
 	"time"
 
+	"github.com/algorand/go-algorand/util"
 	"github.com/algorand/go-deadlock"
 )
 
@@ -173,7 +174,7 @@ func (cyclic *CyclicFileWriter) Write(p []byte) (n int, err error) {
 			shouldBz2 = true
 			archivePath = archivePath[:len(archivePath)-4]
 		}
-		if err = os.Rename(cyclic.liveLog, archivePath); err != nil {
+		if err = util.MoveFile(cyclic.liveLog, archivePath); err != nil {
 			panic(fmt.Sprintf("CyclicFileWriter: cannot archive full log %v", err))
 		}
 		if shouldGz {

logging/cyclicWriter_test.go

@@ -18,16 +18,17 @@ package logging
 
 import (
 	"os"
+	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/algorand/go-algorand/test/partitiontest"
 	"github.com/stretchr/testify/require"
 )
 
-func TestCyclicWrite(t *testing.T) {
-	partitiontest.PartitionTest(t)
-	liveFileName := "live.test"
-	archiveFileName := "archive.test"
+func testCyclicWrite(t *testing.T, liveFileName, archiveFileName string) {
+	t.Helper()
+
 	defer os.Remove(liveFileName)
 	defer os.Remove(archiveFileName)
 
@@ -60,3 +61,33 @@ func TestCyclicWrite(t *testing.T) {
 		require.Equal(t, byte('A'), oldData[i])
 	}
 }
+
+func TestCyclicWrite(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+
+	liveFileName := filepath.Join(tmpDir, "live.test")
+	archiveFileName := filepath.Join(tmpDir, "archive.test")
+
+	testCyclicWrite(t, liveFileName, archiveFileName)
+}
+
+func TestCyclicWriteAcrossFilesystems(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	t.Skip("This is a manual test that must be run on a linux system")
+
+	os.Mkdir("/mnt/tmpfs", os.ModePerm)
+	defer os.Remove("/mnt/tmpfs")
+
+	err := exec.Command("mount", "-t", "tmpfs", "-o", "size=2K", "tmpfs", "/mnt/tmpfs").Run()
+	require.NoError(t, err)
+	defer exec.Command("umount", "/mnt/tmpfs").Run()
+
+	liveFileName := filepath.Join(t.TempDir(), "live.test")
+	archiveFileName := "/mnt/tmpfs/archive.test"
+
+	testCyclicWrite(t, liveFileName, archiveFileName)
+}

util/io.go

@@ -24,6 +24,71 @@
 	"strings"
 )
 
+// MoveFile moves a file from src to dst. The advantages of using this over
+// os.Rename() is that it can move files across different filesystems.
+func MoveFile(src, dst string) error {
+	err := os.Rename(src, dst)
+	if err != nil {
+		// os.Rename() may have failed because src and dst are on different
+		// filesystems. Let's try to move the file by copying and deleting the
+		// source file.
+		return moveFileByCopying(src, dst)
+	}
+	return err
+}
+
+func moveFileByCopying(src, dst string) error {
+	// Lstat is specifically used to detect if src is a symlink. We could
+	// support moving symlinks by deleting src and creating a new symlink at
+	// dst, but we don't currently expect to encounter that case, so it has not
+	// been implemented.
+	srcInfo, srcErr := os.Lstat(src)
+	if srcErr != nil {
+		return srcErr
+	}
+	if !srcInfo.Mode().IsRegular() {
+		return fmt.Errorf("cannot move source file '%s': it is not a regular file (%v)", src, srcInfo.Mode())
+	}
+
+	if dstInfo, dstErr := os.Lstat(dst); dstErr == nil {
+		if dstInfo.Mode().IsDir() {
+			return fmt.Errorf("cannot move source file '%s' to destination '%s': destination is a directory", src, dst)
+		}
+		if os.SameFile(dstInfo, srcInfo) {
+			return fmt.Errorf("cannot move source file '%s' to destination '%s': source and destination are the same file", src, dst)
+		}
+	}
+
+	dstDir := filepath.Dir(dst)
+	dstBase := filepath.Base(dst)
+
+	tmpDstFile, errTmp := os.CreateTemp(dstDir, dstBase+".tmp-")
+	if errTmp != nil {
+		return errTmp
+	}
+	tmpDst := tmpDstFile.Name()
+	if errClose := tmpDstFile.Close(); errClose != nil {
+		return errClose
+	}
+
+	if _, err := CopyFile(src, tmpDst); err != nil {
+		// If the copy fails, try to clean up the temporary file
+		_ = os.Remove(tmpDst)
+		return err
+	}
+	if err := os.Rename(tmpDst, dst); err != nil {
+		// If the rename fails, try to clean up the temporary file
+		_ = os.Remove(tmpDst)
+		return err
+	}
+	if err := os.Remove(src); err != nil {
+		// Don't try to clean up the destination file here. Duplicate data is
+		// better than lost/incomplete data.
+		return fmt.Errorf("failed to remove source file '%s' after moving it to '%s': %w", src, dst, err)
+	}
+	return nil
+}
+
 // CopyFile uses io.Copy() to copy a file to another location
 // This was copied from https://opensource.com/article/18/6/copying-files-go
 func CopyFile(src, dst string) (int64, error) {

util/io_test.go

@@ -17,25 +17,170 @@
 package util
 
 import (
+	"fmt"
 	"os"
-	"path"
+	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/algorand/go-algorand/test/partitiontest"
 )
 
 func TestIsEmpty(t *testing.T) {
 	partitiontest.PartitionTest(t)
+	t.Parallel()
 
-	testPath := path.Join(os.TempDir(), "this", "is", "a", "long", "path")
+	testPath := filepath.Join(t.TempDir(), "this", "is", "a", "long", "path")
 	err := os.MkdirAll(testPath, os.ModePerm)
 	assert.NoError(t, err)
 	defer os.RemoveAll(testPath)
 	assert.True(t, IsEmpty(testPath))
 
-	_, err = os.Create(path.Join(testPath, "file.txt"))
+	_, err = os.Create(filepath.Join(testPath, "file.txt"))
 	assert.NoError(t, err)
 	assert.False(t, IsEmpty(testPath))
 }
+
+func testMoveFileSimple(t *testing.T, src, dst string) {
+	t.Helper()
+
+	require.NoFileExists(t, src)
+	require.NoFileExists(t, dst)
+
+	defer os.Remove(src)
+	defer os.Remove(dst)
+
+	f, err := os.Create(src)
+	require.NoError(t, err)
+
+	_, err = f.WriteString("test file contents")
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	err = MoveFile(src, dst)
+	require.NoError(t, err)
+
+	require.FileExists(t, dst)
+	require.NoFileExists(t, src)
+
+	dstContents, err := os.ReadFile(dst)
+	require.NoError(t, err)
+	assert.Equal(t, "test file contents", string(dstContents))
+}
+
+func TestMoveFile(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+
+	src := filepath.Join(tmpDir, "src.txt")
+	dst := filepath.Join(tmpDir, "dst.txt")
+	testMoveFileSimple(t, src, dst)
+}
+
+func TestMoveFileAcrossFilesystems(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	t.Skip("This is a manual test that must be run on a linux system")
+
+	os.Mkdir("/mnt/tmpfs", os.ModePerm)
+	defer os.Remove("/mnt/tmpfs")
+
+	err := exec.Command("mount", "-t", "tmpfs", "-o", "size=1K", "tmpfs", "/mnt/tmpfs").Run()
+	require.NoError(t, err)
+	defer exec.Command("umount", "/mnt/tmpfs").Run()
+
+	src := filepath.Join(t.TempDir(), "src.txt")
+	dst := "/mnt/tmpfs/dst.txt"
+
+	testMoveFileSimple(t, src, dst)
+}
+
+func TestMoveFileSourceDoesNotExist(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+
+	src := filepath.Join(tmpDir, "src.txt")
+	dst := filepath.Join(tmpDir, "dst.txt")
+
+	err := MoveFile(src, dst)
+	var pathError *os.PathError
+	require.ErrorAs(t, err, &pathError)
+	require.Equal(t, "lstat", pathError.Op)
+	require.Equal(t, src, pathError.Path)
+}
+
+func TestMoveFileSourceIsASymlink(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+
+	root := filepath.Join(tmpDir, "root.txt")
+	src := filepath.Join(tmpDir, "src.txt")
+	dst := filepath.Join(tmpDir, "dst.txt")
+
+	_, err := os.Create(root)
+	require.NoError(t, err)
+
+	err = os.Symlink(root, src)
+	require.NoError(t, err)
+
+	// os.Rename should work in this case
+	err = MoveFile(src, dst)
+	require.NoError(t, err)
+
+	// Undo the move
+	require.NoError(t, MoveFile(dst, src))
+
+	// But our moveFileByCopying should fail, since we haven't implemented this case
+	err = moveFileByCopying(src, dst)
+	require.ErrorContains(t, err, fmt.Sprintf("cannot move source file '%s': it is not a regular file", src))
+}
+
+func TestMoveFileSourceAndDestinationAreSame(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	require.NoError(t, os.Mkdir(filepath.Join(tmpDir, "folder"), os.ModePerm))
+
+	src := filepath.Join(tmpDir, "src.txt")
+	dst := src[:len(src)-len("src.txt")] + "folder/../src.txt"
+
+	// dst refers to the same file as src, but with a different path
+	require.NotEqual(t, src, dst)
+	require.Equal(t, src, filepath.Clean(dst))
+
+	_, err := os.Create(src)
+	require.NoError(t, err)
+
+	// os.Rename can handle this case, but our moveFileByCopying should fail
+	err = moveFileByCopying(src, dst)
+	require.ErrorContains(t, err, fmt.Sprintf("cannot move source file '%s' to destination '%s': source and destination are the same file", src, dst))
+}
+
+func TestMoveFileDestinationIsADirectory(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+
+	src := filepath.Join(tmpDir, "src.txt")
+	dst := filepath.Join(tmpDir, "dst.txt")
+
+	_, err := os.Create(src)
+	require.NoError(t, err)
+
+	err = os.Mkdir(dst, os.ModePerm)
+	require.NoError(t, err)
+
+	err = MoveFile(src, dst)
+	require.ErrorContains(t, err, fmt.Sprintf("cannot move source file '%s' to destination '%s': destination is a directory", src, dst))
+}