From 3923679469bb78120f0d34ef95d6a409c3673f72 Mon Sep 17 00:00:00 2001 From: Inhere Date: Sat, 8 Jul 2023 22:29:36 +0800 Subject: [PATCH] :necktie: up: update rotate writer async clean logic, fix test bugs --- .github/workflows/go.yml | 2 +- handler/handler_test.go | 1 + rotatefile/cleanup.go | 5 +- rotatefile/cleanup_test.go | 13 ++--- rotatefile/config.go | 2 +- rotatefile/util.go | 50 ------------------ rotatefile/writer.go | 102 +++++++++++++++++++++++-------------- rotatefile/writer_test.go | 30 +++++++---- 8 files changed, 97 insertions(+), 108 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 142c220..1ab68cc 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -59,7 +59,7 @@ jobs: - name: Run unit tests # run: go test -v -cover ./... - run: go test -v -coverprofile="profile.cov" ./... + run: go test -coverprofile="profile.cov" ./... - name: Send coverage uses: shogo82148/actions-goveralls@v1 diff --git a/handler/handler_test.go b/handler/handler_test.go index 9e5a687..b806d8b 100644 --- a/handler/handler_test.go +++ b/handler/handler_test.go @@ -149,6 +149,7 @@ func TestLockWrapper_Lock(t *testing.T) { func TestNewSysLogHandler(t *testing.T) { if sysutil.IsWin() { t.Skip("skip test on windows") + return } h, err := handler.NewSysLogHandler(syslog.LOG_INFO, "slog") diff --git a/rotatefile/cleanup.go b/rotatefile/cleanup.go index c070d7f..49fea85 100644 --- a/rotatefile/cleanup.go +++ b/rotatefile/cleanup.go @@ -91,7 +91,8 @@ func NewCConfig() *CConfig { } } -// FilesClear multi files by time. TODO +// FilesClear multi files by time. +// // use for rotate and clear other program produce log files type FilesClear struct { // mu sync.Mutex @@ -226,7 +227,7 @@ func (r *FilesClear) cleanByPattern(filePattern string) (err error) { return err } - // not handle subdir + // not handle subdir TODO: support subdir if stat.IsDir() { return nil } diff --git a/rotatefile/cleanup_test.go b/rotatefile/cleanup_test.go index 1b2868e..ed4651e 100644 --- a/rotatefile/cleanup_test.go +++ b/rotatefile/cleanup_test.go @@ -15,6 +15,13 @@ import ( ) func TestFilesClear_Clean(t *testing.T) { + // make files for clean + makeNum := 5 + makeWaitCleanFiles("file_clean.log", makeNum) + _, err := fsutil.PutContents("testdata/subdir/some.txt", "test data") + assert.NoErr(t, err) + + // create clear fc := rotatefile.NewFilesClear() fc.WithConfig(rotatefile.NewCConfig()) fc.WithConfigFn(func(c *rotatefile.CConfig) { @@ -28,12 +35,6 @@ func TestFilesClear_Clean(t *testing.T) { assert.Eq(t, uint(1), cfg.BackupNum) dump.P(cfg) - // make files for clean - makeNum := 5 - makeWaitCleanFiles("file_clean.log", makeNum) - _, err := fsutil.PutContents("testdata/subdir/some.txt", "test data") - assert.NoErr(t, err) - // do clean assert.NoErr(t, fc.Clean()) diff --git a/rotatefile/config.go b/rotatefile/config.go index ec64450..5ca62ff 100644 --- a/rotatefile/config.go +++ b/rotatefile/config.go @@ -188,7 +188,7 @@ type Config struct { // default see DefaultFilenameFn RenameFunc func(filePath string, rotateNum uint) string - // TimeClock for rotate + // TimeClock for rotate file by time. TimeClock Clocker } diff --git a/rotatefile/util.go b/rotatefile/util.go index 2dc187a..e942cf6 100644 --- a/rotatefile/util.go +++ b/rotatefile/util.go @@ -41,56 +41,6 @@ func compressFile(srcPath, dstPath string) error { return zw.Close() } -type filterFunc func(fPath string, fi os.FileInfo) bool -type handleFunc func(fPath string, fi os.FileInfo) error - -// from the go pkg: path/filepath.glob() -// TODO replace use fsutil.FindInDir() -func findFilesInDir(dir string, handleFn handleFunc, filters ...filterFunc) (err error) { - fi, err := os.Stat(dir) - if err != nil { - return // ignore I/O error - } - if !fi.IsDir() { - return // ignore I/O error - } - - d, err := os.Open(dir) - if err != nil { - return // ignore I/O error - } - defer d.Close() - - // names, _ := d.Readdirnames(-1) - // sort.Strings(names) - - stats, _ := d.Readdir(-1) - for _, fi := range stats { - baseName := fi.Name() - filePath := dir + baseName - - // call filters - if len(filters) > 0 { - var filtered = false - for _, filter := range filters { - if !filter(filePath, fi) { - filtered = true - break - } - } - - if filtered { - continue - } - } - - if err := handleFn(filePath, fi); err != nil { - return err - } - } - return nil -} - // TODO replace to fsutil.FileInfo type fileInfo struct { fs.FileInfo diff --git a/rotatefile/writer.go b/rotatefile/writer.go index 28ef932..be3efe2 100644 --- a/rotatefile/writer.go +++ b/rotatefile/writer.go @@ -2,9 +2,9 @@ package rotatefile import ( "fmt" + "io/fs" "os" "path" - "path/filepath" "sort" "strings" "sync" @@ -24,11 +24,14 @@ type Writer struct { // current opened logfile file *os.File path string - // file dir path for the Config.Filepath + // logfile dir path for the Config.Filepath fileDir string - // file max backup time. equals Config.BackupTime * time.Hour + + // logfile max backup time. equals Config.BackupTime * time.Hour backupDur time.Duration // oldFiles []string + cleanCh chan struct{} + stopCh chan struct{} // context use for rotating file by size written uint64 // written size @@ -101,10 +104,15 @@ func (d *Writer) Sync() error { // Close the writer. // will sync data to disk, then close the file handle func (d *Writer) Close() error { - err := d.file.Sync() - if err != nil { + if err := d.file.Sync(); err != nil { return err } + + // stop the async clean backups + if d.stopCh != nil { + close(d.stopCh) + d.stopCh = nil + } return d.file.Close() } @@ -140,12 +148,10 @@ func (d *Writer) Write(p []byte) (n int, err error) { return } -// Rotate the file by config. -func (d *Writer) Rotate() (err error) { - return d.doRotate() -} +// Rotate the file by config and async clean backups +func (d *Writer) Rotate() error { return d.doRotate() } -// Rotate the file by config. +// do rotate the logfile by config and async clean backups func (d *Writer) doRotate() (err error) { // do rotate file by size if d.cfg.MaxSize > 0 && d.written >= d.cfg.MaxSize { @@ -160,12 +166,12 @@ func (d *Writer) doRotate() (err error) { err = d.rotatingByTime() } - // clean backup files - d.asyncCleanBackups() + // async clean backup files + d.asyncClean() return } -// TIP: only called on d.checkInterval > 0 +// TIP: should only call on d.checkInterval > 0 func (d *Writer) rotatingByTime() error { now := d.cfg.TimeClock.Now() if d.nextRotatingAt > now.Unix() { @@ -232,14 +238,6 @@ func (d *Writer) rotatingFile(bakFile string, rename bool) error { return nil } -// ReopenFile the log file -func (d *Writer) ReopenFile() error { - if d.file != nil { - d.file.Close() - } - return d.openFile(d.path) -} - // open the log file. and set the d.file, d.path func (d *Writer) openFile(logfile string) error { file, err := fsutil.OpenFile(logfile, DefaultFileFlags, d.cfg.FilePerm) @@ -258,15 +256,37 @@ func (d *Writer) openFile(logfile string) error { // --------------------------------------------------------------------------- // -// async clean old files by config -func (d *Writer) asyncCleanBackups() { +// async clean old files by config. should be in lock. +func (d *Writer) asyncClean() { if d.cfg.BackupNum == 0 && d.cfg.BackupTime == 0 { return } - // TODO pref: only start once + // if already running, send a signal + if d.cleanCh != nil { + select { + case d.cleanCh <- struct{}{}: + // case <-d.stopCh: + // return // stop clean + default: // skip on blocking + } + return + } + + // init clean channel + d.cleanCh = make(chan struct{}, 1) + d.stopCh = make(chan struct{}) + + // start a goroutine to clean backups go func() { - printErrln("rotatefile: clean backup files error:", d.Clean()) + // consume the signal until stop + select { + case <-d.cleanCh: + printErrln("rotatefile: clean backup files error:", d.Clean()) + case <-d.stopCh: + d.cleanCh = nil + return // stop clean + } }() } @@ -276,25 +296,28 @@ func (d *Writer) Clean() (err error) { return errorx.Err("clean: backupNum and backupTime are both 0") } - // oldFiles: old xx.log.xx files, no gz file + // oldFiles: xx.log.yy files, no gz file var oldFiles, gzFiles []fileInfo fileDir, fileName := path.Split(d.cfg.Filepath) // find and clean old files - err = findFilesInDir(fileDir, func(fPath string, fi os.FileInfo) error { - if strings.HasSuffix(fi.Name(), compressSuffix) { + err = fsutil.FindInDir(fileDir, func(fPath string, ent fs.DirEntry) error { + fi, err := ent.Info() + if err != nil { + return err + } + + if strings.HasSuffix(ent.Name(), compressSuffix) { gzFiles = append(gzFiles, newFileInfo(fPath, fi)) } else { oldFiles = append(oldFiles, newFileInfo(fPath, fi)) } - return nil }, d.buildFilterFns(fileName)...) gzNum := len(gzFiles) oldNum := len(oldFiles) - maxNum := int(d.cfg.BackupNum) - remNum := gzNum + oldNum - maxNum + remNum := gzNum + oldNum - int(d.cfg.BackupNum) if remNum > 0 { // remove old gz files @@ -347,17 +370,17 @@ func (d *Writer) Clean() (err error) { return } -func (d *Writer) buildFilterFns(fileName string) []filterFunc { - filterFns := []filterFunc{ +func (d *Writer) buildFilterFns(fileName string) []fsutil.FilterFunc { + filterFns := []fsutil.FilterFunc{ + fsutil.OnlyFindFile, // filter by name. should match like error.log.* // eg: error.log.xx, error.log.xx.gz - func(fPath string, fi os.FileInfo) bool { - ok, err := filepath.Match(fileName+".*", fi.Name()) + func(fPath string, ent fs.DirEntry) bool { + ok, err := path.Match(fileName+".*", ent.Name()) if err != nil { printErrln("rotatefile: match old file error:", err) return false // skip, not handle } - return ok }, } @@ -365,7 +388,12 @@ func (d *Writer) buildFilterFns(fileName string) []filterFunc { // filter by mod-time, clear expired files if d.cfg.BackupTime > 0 { cutTime := d.cfg.TimeClock.Now().Add(-d.backupDur) - filterFns = append(filterFns, func(fPath string, fi os.FileInfo) bool { + filterFns = append(filterFns, func(fPath string, ent fs.DirEntry) bool { + fi, err := ent.Info() + if err != nil { + return false // skip, not handle + } + // collect un-expired if fi.ModTime().After(cutTime) { return true diff --git a/rotatefile/writer_test.go b/rotatefile/writer_test.go index 84a5bca..17677fb 100644 --- a/rotatefile/writer_test.go +++ b/rotatefile/writer_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/gookit/goutil" + "github.com/gookit/goutil/dump" "github.com/gookit/goutil/fsutil" "github.com/gookit/goutil/mathutil" "github.com/gookit/goutil/testutil/assert" @@ -86,9 +87,7 @@ func TestWriter_Clean(t *testing.T) { logfile := "testdata/writer_clean.log" c := rotatefile.NewConfig(logfile) - c.MaxSize = 128 - c.BackupNum = 0 - c.BackupTime = 0 + c.MaxSize = 128 // will rotate by size wr, err := c.Create() assert.NoErr(t, err) @@ -99,15 +98,24 @@ func TestWriter_Clean(t *testing.T) { } assert.True(t, fsutil.IsFile(logfile)) - _, err = wr.WriteString("hi\n") assert.NoErr(t, err) - assert.Err(t, wr.Clean()) - - // test clean and backup - c.BackupNum = 2 - c.Compress = true - err = wr.Clean() - assert.NoErr(t, err) + files := fsutil.Glob("testdata/writer_clean.log.*") + dump.P(files) + + // test clean error + t.Run("clean error", func(t *testing.T) { + c.BackupNum = 0 + c.BackupTime = 0 + assert.Err(t, wr.Clean()) + }) + + // test clean and compress backup + t.Run("clean and compress", func(t *testing.T) { + c.BackupNum = 2 + c.Compress = true + err = wr.Clean() + assert.NoErr(t, err) + }) }