Merge pull request #17208 from serathius/dont-flock-3.4

[release-3.4] Don't flock snapshot files
etcd-io · Jan 8, 2024 · 8c185ee · 8c185ee
2 parents c7eab9d + 73814a4
commit 8c185ee
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 21 deletions.
diff --git a/etcdserver/server.go b/etcdserver/server.go
@@ -837,8 +837,8 @@ func (s *EtcdServer) purgeFile() {
 	var dberrc, serrc, werrc <-chan error
 	var dbdonec, sdonec, wdonec <-chan struct{}
 	if s.Cfg.MaxSnapFiles > 0 {
-		dbdonec, dberrc = fileutil.PurgeFileWithDoneNotify(s.getLogger(), s.Cfg.SnapDir(), "snap.db", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
-		sdonec, serrc = fileutil.PurgeFileWithDoneNotify(s.getLogger(), s.Cfg.SnapDir(), "snap", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
+		dbdonec, dberrc = fileutil.PurgeFileWithoutFlock(s.getLogger(), s.Cfg.SnapDir(), "snap.db", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
+		sdonec, serrc = fileutil.PurgeFileWithoutFlock(s.getLogger(), s.Cfg.SnapDir(), "snap", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
 	}
 	if s.Cfg.MaxWALFiles > 0 {
 		wdonec, werrc = fileutil.PurgeFileWithDoneNotify(s.getLogger(), s.Cfg.WALDir(), "wal", s.Cfg.MaxWALFiles, purgeFileInterval, s.stopping)

diff --git a/pkg/fileutil/purge.go b/pkg/fileutil/purge.go
@@ -25,18 +25,24 @@ import (
 )
 
 func PurgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval time.Duration, stop <-chan struct{}) <-chan error {
-	return purgeFile(lg, dirname, suffix, max, interval, stop, nil, nil)
+	return purgeFile(lg, dirname, suffix, max, interval, stop, nil, nil, true)
 }
 
 func PurgeFileWithDoneNotify(lg *zap.Logger, dirname string, suffix string, max uint, interval time.Duration, stop <-chan struct{}) (<-chan struct{}, <-chan error) {
 	doneC := make(chan struct{})
-	errC := purgeFile(lg, dirname, suffix, max, interval, stop, nil, doneC)
+	errC := purgeFile(lg, dirname, suffix, max, interval, stop, nil, doneC, true)
+	return doneC, errC
+}
+
+func PurgeFileWithoutFlock(lg *zap.Logger, dirname string, suffix string, max uint, interval time.Duration, stop <-chan struct{}) (<-chan struct{}, <-chan error) {
+	doneC := make(chan struct{})
+	errC := purgeFile(lg, dirname, suffix, max, interval, stop, nil, doneC, false)
 	return doneC, errC
 }
 
 // purgeFile is the internal implementation for PurgeFile which can post purged files to purgec if non-nil.
 // if donec is non-nil, the function closes it to notify its exit.
-func purgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval time.Duration, stop <-chan struct{}, purgec chan<- string, donec chan<- struct{}) <-chan error {
+func purgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval time.Duration, stop <-chan struct{}, purgec chan<- string, donec chan<- struct{}, flock bool) <-chan error {
 	errC := make(chan error, 1)
 	if lg != nil {
 		lg.Info("started to purge file",
@@ -73,14 +79,17 @@ func purgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval
 			fnames = newfnames
 			for len(newfnames) > int(max) {
 				f := filepath.Join(dirname, newfnames[0])
-				l, err := TryLockFile(f, os.O_WRONLY, PrivateFileMode)
-				if err != nil {
-					if lg != nil {
-						lg.Warn("failed to lock file", zap.String("path", f), zap.Error(err))
-					} else {
-						plog.Warningf("failed to lock file, path: %s, error: %v", f, err)
+				var l *LockedFile
+				if flock {
+					l, err = TryLockFile(f, os.O_WRONLY, PrivateFileMode)
+					if err != nil {
+						if lg != nil {
+							lg.Warn("failed to lock file", zap.String("path", f), zap.Error(err))
+						} else {
+							plog.Warningf("failed to lock file, path: %s, error: %v", f, err)
+						}
+						break
 					}
-					break
 				}
 				if err = os.Remove(f); err != nil {
 					if lg != nil {
@@ -91,14 +100,16 @@ func purgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval
 					errC <- err
 					return
 				}
-				if err = l.Close(); err != nil {
-					if lg != nil {
-						lg.Error("failed to unlock/close", zap.String("path", l.Name()), zap.Error(err))
-					} else {
-						plog.Errorf("error unlocking %s when purging file (%v)", l.Name(), err)
+				if flock {
+					if err = l.Close(); err != nil {
+						if lg != nil {
+							lg.Error("failed to unlock/close", zap.String("path", l.Name()), zap.Error(err))
+						} else {
+							plog.Errorf("error unlocking %s when purging file (%v)", l.Name(), err)
+						}
+						errC <- err
+						return
 					}
-					errC <- err
-					return
 				}
 				if lg != nil {
 					lg.Info("purged", zap.String("path", f))

diff --git a/pkg/fileutil/purge_test.go b/pkg/fileutil/purge_test.go
@@ -45,7 +45,7 @@ func TestPurgeFile(t *testing.T) {
 	stop, purgec := make(chan struct{}), make(chan string, 10)
 
 	// keep 3 most recent files
-	errch := purgeFile(zap.NewExample(), dir, "test", 3, time.Millisecond, stop, purgec, nil)
+	errch := purgeFile(zap.NewExample(), dir, "test", 3, time.Millisecond, stop, purgec, nil, false)
 	select {
 	case f := <-purgec:
 		t.Errorf("unexpected purge on %q", f)
@@ -116,7 +116,7 @@ func TestPurgeFileHoldingLockFile(t *testing.T) {
 	}
 
 	stop, purgec := make(chan struct{}), make(chan string, 10)
-	errch := purgeFile(zap.NewExample(), dir, "test", 3, time.Millisecond, stop, purgec, nil)
+	errch := purgeFile(zap.NewExample(), dir, "test", 3, time.Millisecond, stop, purgec, nil, true)
 
 	for i := 0; i < 5; i++ {
 		select {