diff --git a/file/helper_aix.go b/file/helper_aix.go index e9355923..dbc49bd8 100644 --- a/file/helper_aix.go +++ b/file/helper_aix.go @@ -23,10 +23,14 @@ import ( ) // SafeFileRotate safely rotates an existing file under path and replaces it with the tempfile -func SafeFileRotate(path, tempfile string) error { +func SafeFileRotate(path, tempfile string, opts ...RotateOpt) error { + options := rotateOpts{} + for _, opt := range opts { + opt(&options) + } - if e := os.Rename(tempfile, path); e != nil { - return e + if err := rename(tempfile, path, options); err != nil { + return err } // best-effort fsync on parent directory. The fsync is required by some diff --git a/file/helper_common.go b/file/helper_common.go new file mode 100644 index 00000000..c5363dcd --- /dev/null +++ b/file/helper_common.go @@ -0,0 +1,62 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package file + +import ( + "fmt" + "os" + "time" +) + +type rotateOpts struct { + renameRetryDuration time.Duration + renameRetryInterval time.Duration +} + +type RotateOpt func(*rotateOpts) + +func WithRenameRetries(duration, interval time.Duration) RotateOpt { + return func(opts *rotateOpts) { + opts.renameRetryDuration = duration + opts.renameRetryInterval = interval + } +} + +func rename(src, dst string, options rotateOpts) error { + // Perform a regular (non-retrying) rename unless all retry options are specified. + if options.renameRetryDuration == 0 || options.renameRetryInterval == 0 { + return os.Rename(src, dst) + } + + // Attempt rename with retries every options.RenameRetryInterval until options.RenameRetryDuration + // has elapsed. This is useful in cases where the destination file may be locked or in use. + var err error + for start := time.Now(); time.Since(start) < options.renameRetryDuration; time.Sleep(options.renameRetryInterval) { + err = os.Rename(src, dst) + if err == nil { + // Rename succeeded; no more retries needed + return nil + } + } + + if err != nil { + return fmt.Errorf("failed to rename %s to %s after %v: %w", src, dst, options.renameRetryDuration, err) + } + + return nil +} diff --git a/file/helper_other.go b/file/helper_other.go index c5586c0b..6d786ecc 100644 --- a/file/helper_other.go +++ b/file/helper_other.go @@ -25,10 +25,14 @@ import ( ) // SafeFileRotate safely rotates an existing file under path and replaces it with the tempfile -func SafeFileRotate(path, tempfile string) error { +func SafeFileRotate(path, tempfile string, opts ...RotateOpt) error { + options := rotateOpts{} + for _, opt := range opts { + opt(&options) + } - if e := os.Rename(tempfile, path); e != nil { - return e + if err := rename(tempfile, path, options); err != nil { + return err } // best-effort fsync on parent directory. The fsync is required by some diff --git a/file/helper_windows.go b/file/helper_windows.go index 73821e38..ae2e8bb4 100644 --- a/file/helper_windows.go +++ b/file/helper_windows.go @@ -20,12 +20,25 @@ package file import ( "os" "path/filepath" + "time" ) +const windowsRenameRetryInterval = 50 * time.Millisecond +const windowsRenameRetryDuration = 2 * time.Second + // SafeFileRotate safely rotates an existing file under path and replaces it with the tempfile -func SafeFileRotate(path, tempfile string) error { +func SafeFileRotate(path, tempfile string, opts ...RotateOpt) error { + // On Windows, retry the rename operation by default. This is useful in cases where + // path, the destination file, may be locked or in use. + options := rotateOpts{ + renameRetryDuration: windowsRenameRetryDuration, + renameRetryInterval: windowsRenameRetryInterval, + } + for _, opt := range opts { + opt(&options) + } + old := path + ".old" - var e error // In Windows, one cannot rename a file if the destination already exists, at least // not with using the os.Rename function that Golang offers. @@ -37,8 +50,8 @@ func SafeFileRotate(path, tempfile string) error { // ignore error in case path doesn't exist _ = os.Rename(path, old) - if e = os.Rename(tempfile, path); e != nil { - return e + if err := rename(tempfile, path, options); err != nil { + return err } // .old file will still exist if path file is already there, it should be removed diff --git a/file/helper_windows_test.go b/file/helper_windows_test.go new file mode 100644 index 00000000..7f0f2426 --- /dev/null +++ b/file/helper_windows_test.go @@ -0,0 +1,62 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package file + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestSafeFileRotate creates two files, dest and src, and calls +// SafeFileRotate to replace dest with src. However, before the test +// makes that call, it deliberately keeps a handle open on dest for +// a short period of time to ensure that the rotation takes place anyway +// after the handle has been released. +func TestSafeFileRotate(t *testing.T) { + // Create destination file + tmpDir := t.TempDir() + dest := filepath.Join(tmpDir, "dest.txt") + err := os.WriteFile(dest, []byte("existing content"), 0644) + require.NoError(t, err) + + // Create source file + src := filepath.Join(tmpDir, "src.txt") + err = os.WriteFile(src, []byte("new content"), 0644) + require.NoError(t, err) + + // Open handle on dest file for 1.5 seconds + destFile, err := os.Open(dest) + require.NoError(t, err) + time.AfterFunc(1500*time.Millisecond, func() { + destFile.Close() // Close the handle after 1.5 seconds + }) + defer destFile.Close() + + // Try to replace dest with new + err = SafeFileRotate(dest, src, WithRenameRetries(2*time.Second, 100*time.Millisecond)) + require.NoError(t, err) + + // Check that dest file has been replaced with new file + data, err := os.ReadFile(dest) + require.NoError(t, err) + require.Equal(t, "new content", string(data)) +}