Merge pull request etcd-io#17199 from serathius/dont-flock

Don't flock snapshot files
ahrtr · Jan 8, 2024 · a2eb17c · a2eb17c
2 parents 3a8e90c + 7f8346b
commit a2eb17c
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 15 deletions.
diff --git a/client/pkg/fileutil/purge.go b/client/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 {
 	if lg == nil {
 		lg = zap.NewNop()
 	}
@@ -67,20 +73,25 @@ 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 {
-					lg.Warn("failed to lock file", zap.String("path", f), zap.Error(err))
-					break
+				var l *LockedFile
+				if flock {
+					l, err = TryLockFile(f, os.O_WRONLY, PrivateFileMode)
+					if err != nil {
+						lg.Warn("failed to lock file", zap.String("path", f), zap.Error(err))
+						break
+					}
 				}
 				if err = os.Remove(f); err != nil {
 					lg.Error("failed to remove file", zap.String("path", f), zap.Error(err))
 					errC <- err
 					return
 				}
-				if err = l.Close(); err != nil {
-					lg.Error("failed to unlock/close", zap.String("path", l.Name()), zap.Error(err))
-					errC <- err
-					return
+				if flock {
+					if err = l.Close(); err != nil {
+						lg.Error("failed to unlock/close", zap.String("path", l.Name()), zap.Error(err))
+						errC <- err
+						return
+					}
 				}
 				lg.Info("purged", zap.String("path", f))
 				newfnames = newfnames[1:]

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

diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go
@@ -598,8 +598,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(lg, s.Cfg.SnapDir(), "snap.db", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
-		sdonec, serrc = fileutil.PurgeFileWithDoneNotify(lg, s.Cfg.SnapDir(), "snap", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
+		dbdonec, dberrc = fileutil.PurgeFileWithoutFlock(lg, s.Cfg.SnapDir(), "snap.db", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
+		sdonec, serrc = fileutil.PurgeFileWithoutFlock(lg, s.Cfg.SnapDir(), "snap", s.Cfg.MaxSnapFiles, purgeFileInterval, s.stopping)
 	}
 	if s.Cfg.MaxWALFiles > 0 {
 		wdonec, werrc = fileutil.PurgeFileWithDoneNotify(lg, s.Cfg.WALDir(), "wal", s.Cfg.MaxWALFiles, purgeFileInterval, s.stopping)