From 8695e3d99fc9970adae3809106a92f3776731547 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 13 Jan 2016 17:16:25 -0800 Subject: [PATCH 01/18] Added an impl for log-rotator --- client/driver/logs.go | 71 ++++++++++++++++++++++++++++++++++++++ client/driver/logs_test.go | 40 +++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 client/driver/logs.go create mode 100644 client/driver/logs_test.go diff --git a/client/driver/logs.go b/client/driver/logs.go new file mode 100644 index 000000000..3967fe645 --- /dev/null +++ b/client/driver/logs.go @@ -0,0 +1,71 @@ +package driver + +import ( + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "strconv" + "strings" +) + +type LogRotator struct { + maxFiles int + fileSize int64 + path string + fileName string + logFileIdx int +} + +func NewLogRotor(path string, fileName string, maxFiles int, fileSize int64) (*LogRotator, error) { + files, err := ioutil.ReadDir(path) + if err != nil { + return nil, err + } + + logFileIdx := 0 + for _, f := range files { + if strings.HasPrefix(f.Name(), fileName) { + fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", fileName)) + n, err := strconv.Atoi(fileIdx) + if err != nil { + continue + } + if n > logFileIdx { + logFileIdx = n + } + } + } + + return &LogRotator{ + maxFiles: maxFiles, + fileSize: fileSize, + path: path, + fileName: fileName, + logFileIdx: logFileIdx, + }, nil +} + +func (l *LogRotator) Start(r io.Reader) error { + for { + logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) + f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 066) + if err != nil { + return err + } + remainingSize := l.fileSize + if finfo, err := os.Stat(logFileName); err == nil { + remainingSize = l.fileSize - finfo.Size() + } + if remainingSize < 1 { + l.logFileIdx = l.logFileIdx + 1 + continue + } + if _, err := io.Copy(f, io.LimitReader(r, remainingSize)); err != nil { + return err + } + l.logFileIdx = l.logFileIdx + 1 + } + return nil +} diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go new file mode 100644 index 000000000..cdb22083a --- /dev/null +++ b/client/driver/logs_test.go @@ -0,0 +1,40 @@ +package driver + +import ( + "os" + "path/filepath" + "testing" +) + +func TestLogRotator_IncorrectPath(t *testing.T) { + incorrectPath := "/foo" + + if _, err := NewLogRotor(incorrectPath, "redis.stdout", 10, 10); err == nil { + t.Fatal("expected err") + } +} + +func TestLogRotator_FindCorrectIndex(t *testing.T) { + path := "/tmp/tmplogrator" + if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + + fname := filepath.Join(path, "redis.stdout.1") + if f, err := os.Create(fname); err == nil { + f.Close() + } + + fname = filepath.Join(path, "redis.stdout.2") + if f, err := os.Create(fname); err == nil { + f.Close() + } + + r, err := NewLogRotor(path, "redis.stdout", 10, 10) + if err != nil { + t.Fatalf("test setup err: %v", err) + } + if r.logFileIdx != 2 { + t.Fatalf("Expected log file idx: %v, actual: %v", 2, r.logFileIdx) + } +} From ea280d1b26be4a5e08ab14988bf3e5fc9599475c Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 14 Jan 2016 11:41:31 -0800 Subject: [PATCH 02/18] Added a test for file rotation --- client/driver/logs.go | 2 +- client/driver/logs_test.go | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 3967fe645..377f7c35f 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -24,7 +24,7 @@ func NewLogRotor(path string, fileName string, maxFiles int, fileSize int64) (*L return nil, err } - logFileIdx := 0 + logFileIdx := 1 for _, f := range files { if strings.HasPrefix(f.Name(), fileName) { fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", fileName)) diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index cdb22083a..78588c899 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -1,6 +1,7 @@ package driver import ( + "bytes" "os" "path/filepath" "testing" @@ -19,6 +20,7 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) } + defer os.RemoveAll(path) fname := filepath.Join(path, "redis.stdout.1") if f, err := os.Create(fname); err == nil { @@ -38,3 +40,24 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { t.Fatalf("Expected log file idx: %v, actual: %v", 2, r.logFileIdx) } } + +func TestLogRotator_AppendToCurrentFile(t *testing.T) { + path := "/tmp/tmplogrator" + if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + defer os.RemoveAll(path) + fname := filepath.Join(path, "redis.stdout.1") + if f, err := os.Create(fname); err == nil { + f.Close() + } + + r, err := NewLogRotor(path, "redis.stdout", 10, 10) + if err != nil { + t.Fatalf("test setup err: %v", err) + } + + var buf bytes.Buffer + buf.WriteString("hello") + r.Start(&buf) +} From 7b1e3db4c54013c9598fa28da3eed9b1c857d165 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 14 Jan 2016 18:30:53 -0800 Subject: [PATCH 03/18] Fixed the test for writing only N amount of bytes as much as capacity --- client/driver/logs.go | 29 +++++++++++++++++++++++------ client/driver/logs_test.go | 32 +++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 377f7c35f..6c10235da 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -18,13 +18,13 @@ type LogRotator struct { logFileIdx int } -func NewLogRotor(path string, fileName string, maxFiles int, fileSize int64) (*LogRotator, error) { +func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) (*LogRotator, error) { files, err := ioutil.ReadDir(path) if err != nil { return nil, err } - logFileIdx := 1 + logFileIdx := 0 for _, f := range files { if strings.HasPrefix(f.Name(), fileName) { fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", fileName)) @@ -48,22 +48,39 @@ func NewLogRotor(path string, fileName string, maxFiles int, fileSize int64) (*L } func (l *LogRotator) Start(r io.Reader) error { + buf := make([]byte, 32 * 1024) for { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) - f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 066) + f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0066) if err != nil { return err } remainingSize := l.fileSize if finfo, err := os.Stat(logFileName); err == nil { - remainingSize = l.fileSize - finfo.Size() + remainingSize -= finfo.Size() } if remainingSize < 1 { l.logFileIdx = l.logFileIdx + 1 + f.Close() continue } - if _, err := io.Copy(f, io.LimitReader(r, remainingSize)); err != nil { - return err + + for { + nr, err := io.LimitReader(r, remainingSize).Read(buf) + if err != nil { + f.Close() + return err + } + nw, err := f.Write(buf[:nr]) + if err != nil { + f.Close() + return err + } + if nr != nw { + f.Close() + return fmt.Errorf("Failed to write data R: %d W: %d", nr, nw) + } + remainingSize -= int64(nr) } l.logFileIdx = l.logFileIdx + 1 } diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 78588c899..0d7109fbe 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -1,7 +1,7 @@ package driver import ( - "bytes" + "io" "os" "path/filepath" "testing" @@ -10,7 +10,7 @@ import ( func TestLogRotator_IncorrectPath(t *testing.T) { incorrectPath := "/foo" - if _, err := NewLogRotor(incorrectPath, "redis.stdout", 10, 10); err == nil { + if _, err := NewLogRotator(incorrectPath, "redis.stdout", 10, 10); err == nil { t.Fatal("expected err") } } @@ -32,7 +32,7 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { f.Close() } - r, err := NewLogRotor(path, "redis.stdout", 10, 10) + r, err := NewLogRotator(path, "redis.stdout", 10, 10) if err != nil { t.Fatalf("test setup err: %v", err) } @@ -43,21 +43,35 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { func TestLogRotator_AppendToCurrentFile(t *testing.T) { path := "/tmp/tmplogrator" + defer os.RemoveAll(path) if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) } - defer os.RemoveAll(path) - fname := filepath.Join(path, "redis.stdout.1") + fname := filepath.Join(path, "redis.stdout.0") if f, err := os.Create(fname); err == nil { + f.WriteString("abcde") f.Close() } - r, err := NewLogRotor(path, "redis.stdout", 10, 10) + l, err := NewLogRotator(path, "redis.stdout", 10, 6) if err != nil { t.Fatalf("test setup err: %v", err) } - var buf bytes.Buffer - buf.WriteString("hello") - r.Start(&buf) + r, w := io.Pipe() + go func() { + w.Write([]byte("fg")) + w.Close() + }() + err = l.Start(r) + if err != nil && err != io.EOF{ + t.Fatal(err) + } + finfo, err := os.Stat(fname) + if err != nil { + t.Fatal(err) + } + if finfo.Size() != 6 { + t.Fatalf("Expected size of file: %v, actual: %v", 6, finfo.Size()) + } } From b3c56da9c40d663a6e72993072cf1e85511ca07c Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 14 Jan 2016 22:36:55 -0800 Subject: [PATCH 04/18] Fixed the logic of rotating files --- client/driver/logs.go | 16 ++++++++++--- client/driver/logs_test.go | 49 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 6c10235da..ba2b81cd6 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -48,10 +48,10 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) ( } func (l *LogRotator) Start(r io.Reader) error { - buf := make([]byte, 32 * 1024) + buf := make([]byte, 32*1024) for { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) - f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0066) + f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) if err != nil { return err } @@ -66,7 +66,13 @@ func (l *LogRotator) Start(r io.Reader) error { } for { - nr, err := io.LimitReader(r, remainingSize).Read(buf) + var nr int + var err error + if remainingSize < 32*1024 { + nr, err = r.Read(buf[0:remainingSize]) + } else { + nr, err = r.Read(buf) + } if err != nil { f.Close() return err @@ -81,6 +87,10 @@ func (l *LogRotator) Start(r io.Reader) error { return fmt.Errorf("Failed to write data R: %d W: %d", nr, nw) } remainingSize -= int64(nr) + if remainingSize < 1 { + f.Close() + break + } } l.logFileIdx = l.logFileIdx + 1 } diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 0d7109fbe..413ff3721 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -2,8 +2,10 @@ package driver import ( "io" + "io/ioutil" "os" "path/filepath" + "strings" "testing" ) @@ -54,7 +56,7 @@ func TestLogRotator_AppendToCurrentFile(t *testing.T) { } l, err := NewLogRotator(path, "redis.stdout", 10, 6) - if err != nil { + if err != nil && err != io.EOF { t.Fatalf("test setup err: %v", err) } @@ -64,7 +66,7 @@ func TestLogRotator_AppendToCurrentFile(t *testing.T) { w.Close() }() err = l.Start(r) - if err != nil && err != io.EOF{ + if err != nil && err != io.EOF { t.Fatal(err) } finfo, err := os.Stat(fname) @@ -75,3 +77,46 @@ func TestLogRotator_AppendToCurrentFile(t *testing.T) { t.Fatalf("Expected size of file: %v, actual: %v", 6, finfo.Size()) } } + +func TestLogRotator_RotateFiles(t *testing.T) { + path := "/tmp/tmplogrator" + defer os.RemoveAll(path) + if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + fname := filepath.Join(path, "redis.stdout.0") + if f, err := os.Create(fname); err == nil { + f.WriteString("abcde") + f.Close() + } + + l, err := NewLogRotator(path, "redis.stdout", 10, 6) + if err != nil { + t.Fatalf("test setup err: %v", err) + } + + r, w := io.Pipe() + go func() { + w.Write([]byte("fg")) + w.Close() + }() + err = l.Start(r) + if err != nil && err != io.EOF { + t.Fatalf("Failure in logrotator start %v", err) + } + + files, err := ioutil.ReadDir(path) + if err != nil { + t.Fatal(err) + } + numFiles := 0 + for _, f := range files { + if strings.HasPrefix(f.Name(), "redis.stdout.") { + numFiles += 1 + } + } + + if numFiles != 2 { + t.Fatalf("Expected number of files: %v, actual: %v", 2, numFiles) + } +} From c3a3ea102db147e8e55ae70e04fbe9fc0afc687f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 14 Jan 2016 22:43:52 -0800 Subject: [PATCH 05/18] Added a test for testing if logrotator creates an empty dir when it starts from scratch --- client/driver/logs_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 413ff3721..03140116e 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -120,3 +120,35 @@ func TestLogRotator_RotateFiles(t *testing.T) { t.Fatalf("Expected number of files: %v, actual: %v", 2, numFiles) } } + +func TestLogRotator_StartFromEmptyDir(t *testing.T) { + path := "/tmp/tmplogrator" + defer os.RemoveAll(path) + if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + + l, err := NewLogRotator(path, "redis.stdout", 10, 10) + if err != nil { + t.Fatalf("test setup err: %v", err) + } + + r, w := io.Pipe() + go func() { + w.Write([]byte("abcdefg")) + w.Close() + }() + err = l.Start(r) + if err != nil && err != io.EOF { + t.Fatalf("Failure in logrotator start %v", err) + } + + finfo, err := os.Stat(filepath.Join(path, "redis.stdout.0")) + if err != nil { + t.Fatal(err) + } + if finfo.Size() != 7 { + t.Fatalf("expected size of file: %v, actual: %v", 7, finfo.Size()) + } + +} From 579178720cde4cb55fcedaee933ea9a35e1d9347 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 14 Jan 2016 23:05:45 -0800 Subject: [PATCH 06/18] excluding directories before opening file for writing --- client/driver/logs.go | 10 ++++++++ client/driver/logs_test.go | 47 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/client/driver/logs.go b/client/driver/logs.go index ba2b81cd6..ecaf38bfc 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -27,6 +27,10 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) ( logFileIdx := 0 for _, f := range files { if strings.HasPrefix(f.Name(), fileName) { + if f.IsDir() { + logFileIdx += 1 + continue + } fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", fileName)) n, err := strconv.Atoi(fileIdx) if err != nil { @@ -51,6 +55,12 @@ func (l *LogRotator) Start(r io.Reader) error { buf := make([]byte, 32*1024) for { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) + if f, err := os.Stat(logFileName); err == nil { + if f.IsDir() { + l.logFileIdx += 1 + continue + } + } f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) if err != nil { return err diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 03140116e..f8e50533c 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -152,3 +152,50 @@ func TestLogRotator_StartFromEmptyDir(t *testing.T) { } } + +func TestLogRotator_SetPathAsFile(t *testing.T) { + path := "/tmp/tmplogrator" + defer os.RemoveAll(path) + if _, err := os.Create(path); err != nil { + t.Fatalf("test setup problem: %v", err) + } + + _, err := NewLogRotator(path, "redis.stdout", 10, 10) + if err == nil { + t.Fatal("expected error") + } +} + +func TestLogRotator_ExcludeDirs(t *testing.T) { + path := "/tmp/tmplogrator" + defer os.RemoveAll(path) + if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + if err := os.Mkdir(filepath.Join(path, "redis.stdout.0"), os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + + l, err := NewLogRotator(path, "redis.stdout", 10, 6) + if err != nil { + t.Fatalf("test setup err: %v", err) + } + + r, w := io.Pipe() + go func() { + w.Write([]byte("fg")) + w.Close() + }() + err = l.Start(r) + if err != nil && err != io.EOF { + t.Fatalf("Failure in logrotator start %v", err) + } + + finfo, err := os.Stat("/tmp/tmplogrator/redis.stdout.1") + if err != nil { + t.Fatal("expected rotator to create redis.stdout.1") + } + if finfo.Size() != 2 { + t.Fatalf("expected size: %v, actual: %v", 2, finfo.Size()) + } +} From 57068a28efb07ece82c8bd9241c97bf54811266d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 14 Jan 2016 23:16:30 -0800 Subject: [PATCH 07/18] add a logger --- client/driver/logs.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index ecaf38bfc..9e8ccaf0a 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -4,18 +4,25 @@ import ( "fmt" "io" "io/ioutil" + "log" "os" "path/filepath" "strconv" "strings" ) +const ( + bufSize = 32 * 1024 +) + type LogRotator struct { maxFiles int fileSize int64 path string fileName string logFileIdx int + + logger *log.Logger } func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) (*LogRotator, error) { @@ -52,7 +59,7 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) ( } func (l *LogRotator) Start(r io.Reader) error { - buf := make([]byte, 32*1024) + buf := make([]byte, bufSize) for { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) if f, err := os.Stat(logFileName); err == nil { @@ -65,6 +72,7 @@ func (l *LogRotator) Start(r io.Reader) error { if err != nil { return err } + l.logger.Println("[INFO] logrotator: opened a new file: %s", logFileName) remainingSize := l.fileSize if finfo, err := os.Stat(logFileName); err == nil { remainingSize -= finfo.Size() @@ -78,7 +86,7 @@ func (l *LogRotator) Start(r io.Reader) error { for { var nr int var err error - if remainingSize < 32*1024 { + if remainingSize < bufSize { nr, err = r.Read(buf[0:remainingSize]) } else { nr, err = r.Read(buf) From c39cb6833d39c9d8cf0e8e2971e0d045f2cb2fd0 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Fri, 15 Jan 2016 11:18:02 -0800 Subject: [PATCH 08/18] Fixing tests --- client/driver/logs.go | 7 ++----- client/driver/logs_test.go | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 9e8ccaf0a..262686566 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -25,7 +25,7 @@ type LogRotator struct { logger *log.Logger } -func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) (*LogRotator, error) { +func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64, logger *log.Logger) (*LogRotator, error) { files, err := ioutil.ReadDir(path) if err != nil { return nil, err @@ -34,10 +34,6 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) ( logFileIdx := 0 for _, f := range files { if strings.HasPrefix(f.Name(), fileName) { - if f.IsDir() { - logFileIdx += 1 - continue - } fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", fileName)) n, err := strconv.Atoi(fileIdx) if err != nil { @@ -55,6 +51,7 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64) ( path: path, fileName: fileName, logFileIdx: logFileIdx, + logger: logger, }, nil } diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index f8e50533c..436159434 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -3,16 +3,21 @@ package driver import ( "io" "io/ioutil" + "log" "os" "path/filepath" "strings" "testing" ) +var ( + logger = log.New(os.Stdout, "", log.LstdFlags) +) + func TestLogRotator_IncorrectPath(t *testing.T) { incorrectPath := "/foo" - if _, err := NewLogRotator(incorrectPath, "redis.stdout", 10, 10); err == nil { + if _, err := NewLogRotator(incorrectPath, "redis.stdout", 10, 10, logger); err == nil { t.Fatal("expected err") } } @@ -34,7 +39,7 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { f.Close() } - r, err := NewLogRotator(path, "redis.stdout", 10, 10) + r, err := NewLogRotator(path, "redis.stdout", 10, 10, logger) if err != nil { t.Fatalf("test setup err: %v", err) } @@ -55,7 +60,7 @@ func TestLogRotator_AppendToCurrentFile(t *testing.T) { f.Close() } - l, err := NewLogRotator(path, "redis.stdout", 10, 6) + l, err := NewLogRotator(path, "redis.stdout", 10, 6, logger) if err != nil && err != io.EOF { t.Fatalf("test setup err: %v", err) } @@ -90,7 +95,7 @@ func TestLogRotator_RotateFiles(t *testing.T) { f.Close() } - l, err := NewLogRotator(path, "redis.stdout", 10, 6) + l, err := NewLogRotator(path, "redis.stdout", 10, 6, logger) if err != nil { t.Fatalf("test setup err: %v", err) } @@ -128,7 +133,7 @@ func TestLogRotator_StartFromEmptyDir(t *testing.T) { t.Fatalf("test setup err: %v", err) } - l, err := NewLogRotator(path, "redis.stdout", 10, 10) + l, err := NewLogRotator(path, "redis.stdout", 10, 10, logger) if err != nil { t.Fatalf("test setup err: %v", err) } @@ -160,7 +165,7 @@ func TestLogRotator_SetPathAsFile(t *testing.T) { t.Fatalf("test setup problem: %v", err) } - _, err := NewLogRotator(path, "redis.stdout", 10, 10) + _, err := NewLogRotator(path, "redis.stdout", 10, 10, logger) if err == nil { t.Fatal("expected error") } @@ -176,7 +181,7 @@ func TestLogRotator_ExcludeDirs(t *testing.T) { t.Fatalf("test setup err: %v", err) } - l, err := NewLogRotator(path, "redis.stdout", 10, 6) + l, err := NewLogRotator(path, "redis.stdout", 10, 6, logger) if err != nil { t.Fatalf("test setup err: %v", err) } From ca07a914edb714bcc49fcb6e48746eabe2ea1b1a Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Sat, 16 Jan 2016 19:19:52 -0800 Subject: [PATCH 09/18] Implemented a method to purge files --- client/driver/logs.go | 35 ++++++++++++++++++++++++++++++++ client/driver/logs_test.go | 41 +++++++++++++++++++++++++++++++------- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 262686566..4e6b059c2 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -7,6 +7,7 @@ import ( "log" "os" "path/filepath" + "sort" "strconv" "strings" ) @@ -15,6 +16,8 @@ const ( bufSize = 32 * 1024 ) +// LogRotator rotates files for a buffer and retains only the last N rotated +// files type LogRotator struct { maxFiles int fileSize int64 @@ -25,6 +28,7 @@ type LogRotator struct { logger *log.Logger } +// NewLogRotator configures and returns a new LogRotator func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64, logger *log.Logger) (*LogRotator, error) { files, err := ioutil.ReadDir(path) if err != nil { @@ -55,6 +59,8 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64, l }, nil } +// Start reads from a Reader and writes them to files and rotates them when the +// size of the file becomes equal to the max size configured func (l *LogRotator) Start(r io.Reader) error { buf := make([]byte, bufSize) for { @@ -111,3 +117,32 @@ func (l *LogRotator) Start(r io.Reader) error { } return nil } + +// PurgeOldFiles removes older files and keeps only the last N files rotated for +// a file +func (l *LogRotator) PurgeOldFiles() { + fIndexes := make([]int, l.maxFiles) + files, err := ioutil.ReadDir(l.path) + if err != nil { + return + } + count := 0 + for _, f := range files { + if strings.HasPrefix(f.Name(), l.fileName) { + fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", l.fileName)) + n, err := strconv.Atoi(fileIdx) + if err != nil { + continue + } + if count == l.maxFiles { + sort.Sort(sort.Reverse(sort.IntSlice(fIndexes))) + fname := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, fIndexes[count-1])) + l.logger.Printf("[INFO] removing file: %v", fname) + os.RemoveAll(fname) + count -= 1 + } + fIndexes[count] = n + count += 1 + } + } +} diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 436159434..348663fd1 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -12,6 +12,7 @@ import ( var ( logger = log.New(os.Stdout, "", log.LstdFlags) + path = "/tmp/logrotator" ) func TestLogRotator_IncorrectPath(t *testing.T) { @@ -23,7 +24,6 @@ func TestLogRotator_IncorrectPath(t *testing.T) { } func TestLogRotator_FindCorrectIndex(t *testing.T) { - path := "/tmp/tmplogrator" if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) } @@ -49,7 +49,6 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { } func TestLogRotator_AppendToCurrentFile(t *testing.T) { - path := "/tmp/tmplogrator" defer os.RemoveAll(path) if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) @@ -84,7 +83,6 @@ func TestLogRotator_AppendToCurrentFile(t *testing.T) { } func TestLogRotator_RotateFiles(t *testing.T) { - path := "/tmp/tmplogrator" defer os.RemoveAll(path) if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) @@ -127,7 +125,6 @@ func TestLogRotator_RotateFiles(t *testing.T) { } func TestLogRotator_StartFromEmptyDir(t *testing.T) { - path := "/tmp/tmplogrator" defer os.RemoveAll(path) if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) @@ -159,7 +156,6 @@ func TestLogRotator_StartFromEmptyDir(t *testing.T) { } func TestLogRotator_SetPathAsFile(t *testing.T) { - path := "/tmp/tmplogrator" defer os.RemoveAll(path) if _, err := os.Create(path); err != nil { t.Fatalf("test setup problem: %v", err) @@ -172,7 +168,6 @@ func TestLogRotator_SetPathAsFile(t *testing.T) { } func TestLogRotator_ExcludeDirs(t *testing.T) { - path := "/tmp/tmplogrator" defer os.RemoveAll(path) if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { t.Fatalf("test setup err: %v", err) @@ -196,7 +191,7 @@ func TestLogRotator_ExcludeDirs(t *testing.T) { t.Fatalf("Failure in logrotator start %v", err) } - finfo, err := os.Stat("/tmp/tmplogrator/redis.stdout.1") + finfo, err := os.Stat(filepath.Join(path, "redis.stdout.1")) if err != nil { t.Fatal("expected rotator to create redis.stdout.1") } @@ -204,3 +199,35 @@ func TestLogRotator_ExcludeDirs(t *testing.T) { t.Fatalf("expected size: %v, actual: %v", 2, finfo.Size()) } } + +func TestLogRotator_PurgeDirs(t *testing.T) { + defer os.RemoveAll(path) + if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + t.Fatalf("test setup err: %v", err) + } + + l, err := NewLogRotator(path, "redis.stdout", 2, 4, logger) + if err != nil { + t.Fatalf("test setup err: %v", err) + } + + r, w := io.Pipe() + go func() { + w.Write([]byte("abcdefghijklmno")) + w.Close() + }() + + err = l.Start(r) + if err != nil && err != io.EOF { + t.Fatalf("failure in logrotator start: %v", err) + } + l.PurgeOldFiles() + + files, err := ioutil.ReadDir(path) + if err != nil { + t.Fatalf("err: %v", err) + } + if len(files) != 2 { + t.Fatalf("expected number of files: %v, actual: %v", 2, len(files)) + } +} From e142dc0e4dd9e46436bbae3de8fc77393b0b8060 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Sat, 16 Jan 2016 19:21:16 -0800 Subject: [PATCH 10/18] Added a comment for bufsize --- client/driver/logs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 4e6b059c2..0e6758a29 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -13,7 +13,7 @@ import ( ) const ( - bufSize = 32 * 1024 + bufSize = 32 * 1024 // Max number of bytes read from a buffer ) // LogRotator rotates files for a buffer and retains only the last N rotated From e45b89d1184c94d57c4746a341a6608c926c2d06 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Sat, 16 Jan 2016 20:09:24 -0800 Subject: [PATCH 11/18] stating an existing file only once --- client/driver/logs.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 0e6758a29..7673637a2 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -65,21 +65,19 @@ func (l *LogRotator) Start(r io.Reader) error { buf := make([]byte, bufSize) for { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) + remainingSize := l.fileSize if f, err := os.Stat(logFileName); err == nil { if f.IsDir() { l.logFileIdx += 1 continue } + remainingSize = l.fileSize - f.Size() } f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) if err != nil { return err } l.logger.Println("[INFO] logrotator: opened a new file: %s", logFileName) - remainingSize := l.fileSize - if finfo, err := os.Stat(logFileName); err == nil { - remainingSize -= finfo.Size() - } if remainingSize < 1 { l.logFileIdx = l.logFileIdx + 1 f.Close() From 306d5d281f6b0e599793a8d6bcbc6687ac81c35f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 14:14:37 -0800 Subject: [PATCH 12/18] Added some comments --- client/driver/logs.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 7673637a2..4213fffa2 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -16,14 +16,14 @@ const ( bufSize = 32 * 1024 // Max number of bytes read from a buffer ) -// LogRotator rotates files for a buffer and retains only the last N rotated -// files +// LogRotator ingests data and writes out to a rotated set of files type LogRotator struct { - maxFiles int - fileSize int64 - path string - fileName string - logFileIdx int + maxFiles int // maximum number of rotated files retained by the log rotator + fileSize int64 // maximum file size of a rotated file + path string // path where the rotated files are created + fileName string // base file name of the rotated files + + logFileIdx int // index to the current file logger *log.Logger } @@ -35,6 +35,7 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64, l return nil, err } + // finding out the log file with the largest index logFileIdx := 0 for _, f := range files { if strings.HasPrefix(f.Name(), fileName) { @@ -67,23 +68,28 @@ func (l *LogRotator) Start(r io.Reader) error { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) remainingSize := l.fileSize if f, err := os.Stat(logFileName); err == nil { + // skipping the current file if it happens to be a directory if f.IsDir() { l.logFileIdx += 1 continue } + // calculating the remaining capacity of the log file remainingSize = l.fileSize - f.Size() } f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) if err != nil { return err } - l.logger.Println("[INFO] logrotator: opened a new file: %s", logFileName) + l.logger.Println("[INFO] client.logrotator: opened a new file: %s", logFileName) + // closing the current log file if it doesn't have any more capacity if remainingSize < 1 { l.logFileIdx = l.logFileIdx + 1 f.Close() continue } + // reading from the reader and writing into the current log file as long + // as it has capacity or the reader closes for { var nr int var err error @@ -103,7 +109,7 @@ func (l *LogRotator) Start(r io.Reader) error { } if nr != nw { f.Close() - return fmt.Errorf("Failed to write data R: %d W: %d", nr, nw) + return fmt.Errorf("failed to write data read from the reader into file, R: %d W: %d", nr, nw) } remainingSize -= int64(nr) if remainingSize < 1 { @@ -135,7 +141,7 @@ func (l *LogRotator) PurgeOldFiles() { if count == l.maxFiles { sort.Sort(sort.Reverse(sort.IntSlice(fIndexes))) fname := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, fIndexes[count-1])) - l.logger.Printf("[INFO] removing file: %v", fname) + l.logger.Printf("[DEBUG] client.logrator: removing file: %v", fname) os.RemoveAll(fname) count -= 1 } From 87ccaab00bc96827f6646c397d7f9566d7a6b5cd Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 15:52:52 -0800 Subject: [PATCH 13/18] Changing the logic of purging old rotated files --- client/driver/logs.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 4213fffa2..2da6fe2a9 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -80,7 +80,7 @@ func (l *LogRotator) Start(r io.Reader) error { if err != nil { return err } - l.logger.Println("[INFO] client.logrotator: opened a new file: %s", logFileName) + l.logger.Printf("[DEBUG] client.logrotator: opened a new file: %s", logFileName) // closing the current log file if it doesn't have any more capacity if remainingSize < 1 { l.logFileIdx = l.logFileIdx + 1 @@ -125,12 +125,12 @@ func (l *LogRotator) Start(r io.Reader) error { // PurgeOldFiles removes older files and keeps only the last N files rotated for // a file func (l *LogRotator) PurgeOldFiles() { - fIndexes := make([]int, l.maxFiles) + var fIndexes []int files, err := ioutil.ReadDir(l.path) if err != nil { return } - count := 0 + // Inserting all the rotated files in a slice for _, f := range files { if strings.HasPrefix(f.Name(), l.fileName) { fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", l.fileName)) @@ -138,15 +138,16 @@ func (l *LogRotator) PurgeOldFiles() { if err != nil { continue } - if count == l.maxFiles { - sort.Sort(sort.Reverse(sort.IntSlice(fIndexes))) - fname := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, fIndexes[count-1])) - l.logger.Printf("[DEBUG] client.logrator: removing file: %v", fname) - os.RemoveAll(fname) - count -= 1 - } - fIndexes[count] = n - count += 1 + fIndexes = append(fIndexes, n) } } + + // sorting the file indexes so that we can purge the older files and keep + // only the number of files as configured by the user + sort.Sort(sort.IntSlice(fIndexes)) + toDelete := fIndexes[l.maxFiles-1 : len(fIndexes)-1] + for _, fIndex := range toDelete { + fname := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, fIndex)) + os.RemoveAll(fname) + } } From 8e091a52e3697491e1ad7737e7873fb663a87cf5 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 15:55:41 -0800 Subject: [PATCH 14/18] Renaming a test --- client/driver/logs_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 348663fd1..72af1f086 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -15,10 +15,10 @@ var ( path = "/tmp/logrotator" ) -func TestLogRotator_IncorrectPath(t *testing.T) { - incorrectPath := "/foo" +func TestLogRotator_InvalidPath(t *testing.T) { + invalidPath := "/foo" - if _, err := NewLogRotator(incorrectPath, "redis.stdout", 10, 10, logger); err == nil { + if _, err := NewLogRotator(invalidPath, "redis.stdout", 10, 10, logger); err == nil { t.Fatal("expected err") } } From aa4d9a33172b283ff671ed93e1d146c04d82d8df Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 16:17:49 -0800 Subject: [PATCH 15/18] Using tmpdir and tmpfile in tests --- client/driver/logs.go | 3 ++- client/driver/logs_test.go | 38 +++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 2da6fe2a9..45351f7a4 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -81,8 +81,9 @@ func (l *LogRotator) Start(r io.Reader) error { return err } l.logger.Printf("[DEBUG] client.logrotator: opened a new file: %s", logFileName) + // closing the current log file if it doesn't have any more capacity - if remainingSize < 1 { + if remainingSize <= 0 { l.logFileIdx = l.logFileIdx + 1 f.Close() continue diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 72af1f086..6b7fd6ffc 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -11,8 +11,8 @@ import ( ) var ( - logger = log.New(os.Stdout, "", log.LstdFlags) - path = "/tmp/logrotator" + logger = log.New(os.Stdout, "", log.LstdFlags) + pathPrefix = "logrotator" ) func TestLogRotator_InvalidPath(t *testing.T) { @@ -24,7 +24,9 @@ func TestLogRotator_InvalidPath(t *testing.T) { } func TestLogRotator_FindCorrectIndex(t *testing.T) { - if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + var path string + var err error + if path, err = ioutil.TempDir("", pathPrefix); err != nil { t.Fatalf("test setup err: %v", err) } defer os.RemoveAll(path) @@ -49,8 +51,10 @@ func TestLogRotator_FindCorrectIndex(t *testing.T) { } func TestLogRotator_AppendToCurrentFile(t *testing.T) { + var path string + var err error defer os.RemoveAll(path) - if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + if path, err = ioutil.TempDir("", pathPrefix); err != nil { t.Fatalf("test setup err: %v", err) } fname := filepath.Join(path, "redis.stdout.0") @@ -83,8 +87,10 @@ func TestLogRotator_AppendToCurrentFile(t *testing.T) { } func TestLogRotator_RotateFiles(t *testing.T) { + var path string + var err error defer os.RemoveAll(path) - if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + if path, err = ioutil.TempDir("", pathPrefix); err != nil { t.Fatalf("test setup err: %v", err) } fname := filepath.Join(path, "redis.stdout.0") @@ -125,8 +131,10 @@ func TestLogRotator_RotateFiles(t *testing.T) { } func TestLogRotator_StartFromEmptyDir(t *testing.T) { + var path string + var err error defer os.RemoveAll(path) - if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + if path, err = ioutil.TempDir("", pathPrefix); err != nil { t.Fatalf("test setup err: %v", err) } @@ -156,20 +164,26 @@ func TestLogRotator_StartFromEmptyDir(t *testing.T) { } func TestLogRotator_SetPathAsFile(t *testing.T) { + var f *os.File + var err error + var path string defer os.RemoveAll(path) - if _, err := os.Create(path); err != nil { + if f, err = ioutil.TempFile("", pathPrefix); err != nil { t.Fatalf("test setup problem: %v", err) } - _, err := NewLogRotator(path, "redis.stdout", 10, 10, logger) - if err == nil { + path = f.Name() + + if _, err = NewLogRotator(f.Name(), "redis.stdout", 10, 10, logger); err == nil { t.Fatal("expected error") } } func TestLogRotator_ExcludeDirs(t *testing.T) { + var path string + var err error defer os.RemoveAll(path) - if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + if path, err = ioutil.TempDir("", pathPrefix); err != nil { t.Fatalf("test setup err: %v", err) } if err := os.Mkdir(filepath.Join(path, "redis.stdout.0"), os.ModeDir|os.ModePerm); err != nil { @@ -201,8 +215,10 @@ func TestLogRotator_ExcludeDirs(t *testing.T) { } func TestLogRotator_PurgeDirs(t *testing.T) { + var path string + var err error defer os.RemoveAll(path) - if err := os.Mkdir(path, os.ModeDir|os.ModePerm); err != nil { + if path, err = ioutil.TempDir("", pathPrefix); err != nil { t.Fatalf("test setup err: %v", err) } From dca7a53d50c9f57d001d84a20556e92437e43c4f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 16:25:51 -0800 Subject: [PATCH 16/18] Simplifying a test --- client/driver/logs_test.go | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index 6b7fd6ffc..a357226dd 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -6,7 +6,6 @@ import ( "log" "os" "path/filepath" - "strings" "testing" ) @@ -114,19 +113,20 @@ func TestLogRotator_RotateFiles(t *testing.T) { t.Fatalf("Failure in logrotator start %v", err) } - files, err := ioutil.ReadDir(path) - if err != nil { - t.Fatal(err) - } - numFiles := 0 - for _, f := range files { - if strings.HasPrefix(f.Name(), "redis.stdout.") { - numFiles += 1 + if finfo, err := os.Stat(filepath.Join(path, "redis.stdout.1")); err == nil { + if finfo.Size() != 1 { + t.Fatalf("expected number of bytes: %v, actual: %v", 1, finfo.Size()) } + } else { + t.Fatal("expected file redis.stdout.1") } - if numFiles != 2 { - t.Fatalf("Expected number of files: %v, actual: %v", 2, numFiles) + if finfo, err := os.Stat(filepath.Join(path, "redis.stdout.0")); err == nil { + if finfo.Size() != 6 { + t.Fatalf("expected number of bytes: %v, actual: %v", 1, finfo.Size()) + } + } else { + t.Fatal("expected file redis.stdout.0") } } @@ -171,9 +171,7 @@ func TestLogRotator_SetPathAsFile(t *testing.T) { if f, err = ioutil.TempFile("", pathPrefix); err != nil { t.Fatalf("test setup problem: %v", err) } - path = f.Name() - if _, err = NewLogRotator(f.Name(), "redis.stdout", 10, 10, logger); err == nil { t.Fatal("expected error") } From 890333b2e6fa32d7f1416cdb24f1160b2825d677 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 16:30:55 -0800 Subject: [PATCH 17/18] Added a comment in the test --- client/driver/logs_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/driver/logs_test.go b/client/driver/logs_test.go index a357226dd..d45ae5fdd 100644 --- a/client/driver/logs_test.go +++ b/client/driver/logs_test.go @@ -105,6 +105,7 @@ func TestLogRotator_RotateFiles(t *testing.T) { r, w := io.Pipe() go func() { + // This should make the current log file rotate w.Write([]byte("fg")) w.Close() }() From a284d27daaedc1301102ae751cb8a521325e92e8 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 20 Jan 2016 16:50:31 -0800 Subject: [PATCH 18/18] Simplied the logic of detecting the currently rotate log file --- client/driver/logs.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/client/driver/logs.go b/client/driver/logs.go index 45351f7a4..7297382cd 100644 --- a/client/driver/logs.go +++ b/client/driver/logs.go @@ -35,11 +35,12 @@ func NewLogRotator(path string, fileName string, maxFiles int, fileSize int64, l return nil, err } - // finding out the log file with the largest index + // Finding out the log file with the largest index logFileIdx := 0 + prefix := fmt.Sprintf("%s.", fileName) for _, f := range files { - if strings.HasPrefix(f.Name(), fileName) { - fileIdx := strings.TrimPrefix(f.Name(), fmt.Sprintf("%s.", fileName)) + if strings.HasPrefix(f.Name(), prefix) { + fileIdx := strings.TrimPrefix(f.Name(), prefix) n, err := strconv.Atoi(fileIdx) if err != nil { continue @@ -68,12 +69,12 @@ func (l *LogRotator) Start(r io.Reader) error { logFileName := filepath.Join(l.path, fmt.Sprintf("%s.%d", l.fileName, l.logFileIdx)) remainingSize := l.fileSize if f, err := os.Stat(logFileName); err == nil { - // skipping the current file if it happens to be a directory + // Skipping the current file if it happens to be a directory if f.IsDir() { l.logFileIdx += 1 continue } - // calculating the remaining capacity of the log file + // Calculating the remaining capacity of the log file remainingSize = l.fileSize - f.Size() } f, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) @@ -82,14 +83,14 @@ func (l *LogRotator) Start(r io.Reader) error { } l.logger.Printf("[DEBUG] client.logrotator: opened a new file: %s", logFileName) - // closing the current log file if it doesn't have any more capacity + // Closing the current log file if it doesn't have any more capacity if remainingSize <= 0 { l.logFileIdx = l.logFileIdx + 1 f.Close() continue } - // reading from the reader and writing into the current log file as long + // Reading from the reader and writing into the current log file as long // as it has capacity or the reader closes for { var nr int @@ -143,7 +144,7 @@ func (l *LogRotator) PurgeOldFiles() { } } - // sorting the file indexes so that we can purge the older files and keep + // Sorting the file indexes so that we can purge the older files and keep // only the number of files as configured by the user sort.Sort(sort.IntSlice(fIndexes)) toDelete := fIndexes[l.maxFiles-1 : len(fIndexes)-1]