From 73d9a9f160117255f3155bb273df839218095cc1 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Wed, 31 Mar 2021 12:48:24 +0200 Subject: [PATCH 1/7] first draft of CompleteLines --- tail.go | 33 +++++++++++++++++++++++-------- tail_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/tail.go b/tail.go index 37ea441..35b3d95 100644 --- a/tail.go +++ b/tail.go @@ -77,8 +77,9 @@ type Config struct { Pipe bool // The file is a named pipe (mkfifo) // Generic IO - Follow bool // Continue looking for new lines (tail -f) - MaxLineSize int // If non-zero, split longer lines into multiple lines + Follow bool // Continue looking for new lines (tail -f) + MaxLineSize int // If non-zero, split longer lines into multiple lines + CompleteLines bool // Only return complete lines (that end with "\n") // Optionally, use a ratelimiter (e.g. created by the ratelimiter/NewLeakyBucket function) RateLimiter *ratelimiter.LeakyBucket @@ -97,6 +98,8 @@ type Tail struct { reader *bufio.Reader lineNum int + lineBuf *strings.Builder + watcher watch.FileWatcher changes *watch.FileChanges @@ -202,6 +205,7 @@ func (tail *Tail) closeFile() { } func (tail *Tail) reopen() error { + // TODO(PR): should we reset the lineBuf (if it exists)? tail.closeFile() tail.lineNum = 0 for { @@ -229,16 +233,29 @@ func (tail *Tail) readLine() (string, error) { tail.lk.Lock() line, err := tail.reader.ReadString('\n') tail.lk.Unlock() - if err != nil { - // Note ReadString "returns the data read before the error" in - // case of an error, including EOF, so we return it as is. The - // caller is expected to process it if err is EOF. + + if tail.lineBuf == nil { + tail.lineBuf = new(strings.Builder) // TODO(PR): should this be in the constructor or some place? + } + + if _, err := tail.lineBuf.WriteString(line); err != nil { return line, err } - line = strings.TrimRight(line, "\n") + line = tail.lineBuf.String() + newlineEnding := strings.HasSuffix(line, "\n") + if newlineEnding { + tail.lineBuf.Reset() + } + + if tail.Config.CompleteLines && !newlineEnding { + return "", io.EOF + } - return line, err + // Note ReadString "returns the data read before the error" in + // case of an error, including EOF, so we return it as is. The + // caller is expected to process it if err is EOF. + return strings.TrimRight(line, "\n"), err } func (tail *Tail) tailFileSync() { diff --git a/tail_test.go b/tail_test.go index 8a34118..cdf9c54 100644 --- a/tail_test.go +++ b/tail_test.go @@ -505,6 +505,61 @@ func TestInotify_WaitForCreateThenMove(t *testing.T) { tailTest.Cleanup(tail, false) } +func TestIncompleteLines(t *testing.T) { + tailTest := NewTailTest("incomplete-lines", t) + filename := "test.txt" + config := Config{ + Follow: true, + CompleteLines: true, + } + tail := tailTest.StartTail(filename, config) + go func() { + time.Sleep(100 * time.Millisecond) + tailTest.CreateFile(filename, "hello world\n") + time.Sleep(100 * time.Millisecond) + // here we intentially write a partial line to see if `Tail` contains + // information that it's incomplete + tailTest.AppendFile(filename, "hello") + time.Sleep(100 * time.Millisecond) + tailTest.AppendFile(filename, " again\n") + }() + + lines := []string{"hello world", "hello again"} + + tailTest.ReadLines(tail, lines, false) + + tailTest.RemoveFile(filename) + tail.Stop() + tail.Cleanup() +} + +func TestIncompleteLongLines(t *testing.T) { + tailTest := NewTailTest("incomplete-lines", t) + filename := "test.txt" + config := Config{ + Follow: true, + MaxLineSize: 3, + CompleteLines: true, + } + tail := tailTest.StartTail(filename, config) + go func() { + time.Sleep(100 * time.Millisecond) + tailTest.CreateFile(filename, "hello world\n") + time.Sleep(100 * time.Millisecond) + tailTest.AppendFile(filename, "hello") + time.Sleep(100 * time.Millisecond) + tailTest.AppendFile(filename, "again\n") + }() + + lines := []string{"hel", "lo ", "wor", "ld", "hel", "loa", "gai", "n"} + + tailTest.ReadLines(tail, lines, false) + + tailTest.RemoveFile(filename) + tail.Stop() + tail.Cleanup() +} + func reSeek(t *testing.T, poll bool) { var name string if poll { From 3cb5cffcc99d173bf5459dcf0c50f1a304cc5557 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Wed, 31 Mar 2021 13:27:31 +0200 Subject: [PATCH 2/7] tests --- tail.go | 4 +++- tail_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/tail.go b/tail.go index 35b3d95..1bcd53d 100644 --- a/tail.go +++ b/tail.go @@ -205,7 +205,9 @@ func (tail *Tail) closeFile() { } func (tail *Tail) reopen() error { - // TODO(PR): should we reset the lineBuf (if it exists)? + if tail.lineBuf != nil { + tail.lineBuf = nil + } tail.closeFile() tail.lineNum = 0 for { diff --git a/tail_test.go b/tail_test.go index cdf9c54..dd671eb 100644 --- a/tail_test.go +++ b/tail_test.go @@ -560,6 +560,31 @@ func TestIncompleteLongLines(t *testing.T) { tail.Cleanup() } +func TestIncompleteLinesWithReopens(t *testing.T) { + tailTest := NewTailTest("incomplete-lines", t) + filename := "test.txt" + config := Config{ + Follow: true, + CompleteLines: true, + } + tail := tailTest.StartTail(filename, config) + go func() { + // time.Sleep(100 * time.Millisecond) + tailTest.CreateFile(filename, "hello world\nhi") + time.Sleep(100 * time.Millisecond) + tailTest.TruncateFile(filename, "rewriting\n") + }() + + // not that the "hi" gets lost, because it was never a complete line + lines := []string{"hello world", "rewriting"} + + tailTest.ReadLines(tail, lines, false) + + tailTest.RemoveFile(filename) + tail.Stop() + tail.Cleanup() +} + func reSeek(t *testing.T, poll bool) { var name string if poll { From 76e3c426b2e523b56f459ff9fd337e5419f357f7 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Wed, 31 Mar 2021 13:35:25 +0200 Subject: [PATCH 3/7] refactor --- tail.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/tail.go b/tail.go index 1bcd53d..521cb29 100644 --- a/tail.go +++ b/tail.go @@ -129,6 +129,7 @@ func TailFile(filename string, config Config) (*Tail, error) { Filename: filename, Lines: make(chan *Line), Config: config, + lineBuf: new(strings.Builder), } // when Logger was not specified in config, use default logger @@ -205,9 +206,7 @@ func (tail *Tail) closeFile() { } func (tail *Tail) reopen() error { - if tail.lineBuf != nil { - tail.lineBuf = nil - } + tail.lineBuf.Reset() tail.closeFile() tail.lineNum = 0 for { @@ -236,28 +235,28 @@ func (tail *Tail) readLine() (string, error) { line, err := tail.reader.ReadString('\n') tail.lk.Unlock() - if tail.lineBuf == nil { - tail.lineBuf = new(strings.Builder) // TODO(PR): should this be in the constructor or some place? + newlineEnding := strings.HasSuffix(line, "\n") + line = strings.TrimRight(line, "\n") + + // if we don't have to handle incomplete lines, we can return the line as-is + if !tail.Config.CompleteLines { + // Note ReadString "returns the data read before the error" in + // case of an error, including EOF, so we return it as is. The + // caller is expected to process it if err is EOF. + return line, err } if _, err := tail.lineBuf.WriteString(line); err != nil { return line, err } - line = tail.lineBuf.String() - newlineEnding := strings.HasSuffix(line, "\n") if newlineEnding { + line = tail.lineBuf.String() tail.lineBuf.Reset() - } - - if tail.Config.CompleteLines && !newlineEnding { + return line, nil + } else { return "", io.EOF } - - // Note ReadString "returns the data read before the error" in - // case of an error, including EOF, so we return it as is. The - // caller is expected to process it if err is EOF. - return strings.TrimRight(line, "\n"), err } func (tail *Tail) tailFileSync() { From af593d5b71405a9f45dcb18edb2c0ced16333cdd Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Wed, 31 Mar 2021 13:57:09 +0200 Subject: [PATCH 4/7] CompleteLines should return the last line if we don't follow --- tail.go | 7 +++++-- tail_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/tail.go b/tail.go index 521cb29..41c205a 100644 --- a/tail.go +++ b/tail.go @@ -79,7 +79,7 @@ type Config struct { // Generic IO Follow bool // Continue looking for new lines (tail -f) MaxLineSize int // If non-zero, split longer lines into multiple lines - CompleteLines bool // Only return complete lines (that end with "\n") + CompleteLines bool // Only return complete lines (that end with "\n" or EOF when Follow is false) // Optionally, use a ratelimiter (e.g. created by the ratelimiter/NewLeakyBucket function) RateLimiter *ratelimiter.LeakyBucket @@ -255,7 +255,10 @@ func (tail *Tail) readLine() (string, error) { tail.lineBuf.Reset() return line, nil } else { - return "", io.EOF + if tail.Config.Follow { + line = "" + } + return line, io.EOF } } diff --git a/tail_test.go b/tail_test.go index dd671eb..12ac5cf 100644 --- a/tail_test.go +++ b/tail_test.go @@ -569,7 +569,7 @@ func TestIncompleteLinesWithReopens(t *testing.T) { } tail := tailTest.StartTail(filename, config) go func() { - // time.Sleep(100 * time.Millisecond) + time.Sleep(100 * time.Millisecond) tailTest.CreateFile(filename, "hello world\nhi") time.Sleep(100 * time.Millisecond) tailTest.TruncateFile(filename, "rewriting\n") @@ -585,6 +585,29 @@ func TestIncompleteLinesWithReopens(t *testing.T) { tail.Cleanup() } +func TestIncompleteLinesWithoutFollow(t *testing.T) { + tailTest := NewTailTest("incomplete-lines", t) + filename := "test.txt" + config := Config{ + Follow: false, + CompleteLines: true, + } + tail := tailTest.StartTail(filename, config) + go func() { + time.Sleep(100 * time.Millisecond) + // intentionally missing a newline at the end + tailTest.CreateFile(filename, "foo\nbar\nbaz") + }() + + lines := []string{"foo", "bar", "baz"} + + tailTest.VerifyTailOutput(tail, lines, true) + + tailTest.RemoveFile(filename) + tail.Stop() + tail.Cleanup() +} + func reSeek(t *testing.T, poll bool) { var name string if poll { From 9cbef38d759609f942ce15dbcc86fcdd115a4cee Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Sun, 11 Apr 2021 16:25:36 +0200 Subject: [PATCH 5/7] rename temp dirs to avoid races --- tail_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tail_test.go b/tail_test.go index 12ac5cf..9b89364 100644 --- a/tail_test.go +++ b/tail_test.go @@ -534,7 +534,7 @@ func TestIncompleteLines(t *testing.T) { } func TestIncompleteLongLines(t *testing.T) { - tailTest := NewTailTest("incomplete-lines", t) + tailTest := NewTailTest("incomplete-lines-long", t) filename := "test.txt" config := Config{ Follow: true, @@ -561,7 +561,7 @@ func TestIncompleteLongLines(t *testing.T) { } func TestIncompleteLinesWithReopens(t *testing.T) { - tailTest := NewTailTest("incomplete-lines", t) + tailTest := NewTailTest("incomplete-lines-reopens", t) filename := "test.txt" config := Config{ Follow: true, @@ -586,7 +586,7 @@ func TestIncompleteLinesWithReopens(t *testing.T) { } func TestIncompleteLinesWithoutFollow(t *testing.T) { - tailTest := NewTailTest("incomplete-lines", t) + tailTest := NewTailTest("incomplete-lines-no-follow", t) filename := "test.txt" config := Config{ Follow: false, From 224135092b774b93c11480ca8f503f171ea90d85 Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Tue, 2 Nov 2021 13:04:01 +0100 Subject: [PATCH 6/7] Conditional allocation of strings.Builder --- tail.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tail.go b/tail.go index 41c205a..c962599 100644 --- a/tail.go +++ b/tail.go @@ -129,7 +129,10 @@ func TailFile(filename string, config Config) (*Tail, error) { Filename: filename, Lines: make(chan *Line), Config: config, - lineBuf: new(strings.Builder), + } + + if config.CompleteLines { + t.lineBuf = new(strings.Builder) } // when Logger was not specified in config, use default logger @@ -206,7 +209,9 @@ func (tail *Tail) closeFile() { } func (tail *Tail) reopen() error { - tail.lineBuf.Reset() + if tail.lineBuf != nil { + tail.lineBuf.Reset() + } tail.closeFile() tail.lineNum = 0 for { From 526b058439008bba8edefc4e9edd4260e795f7ec Mon Sep 17 00:00:00 2001 From: Ondrej Kokes Date: Tue, 2 Nov 2021 13:10:41 +0100 Subject: [PATCH 7/7] Fix tests by using the new cleanup() --- tail_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tail_test.go b/tail_test.go index 0568cb7..7b9319e 100644 --- a/tail_test.go +++ b/tail_test.go @@ -516,7 +516,8 @@ func TestInotify_WaitForCreateThenMove(t *testing.T) { } func TestIncompleteLines(t *testing.T) { - tailTest := NewTailTest("incomplete-lines", t) + tailTest, cleanup := NewTailTest("incomplete-lines", t) + defer cleanup() filename := "test.txt" config := Config{ Follow: true, @@ -544,7 +545,8 @@ func TestIncompleteLines(t *testing.T) { } func TestIncompleteLongLines(t *testing.T) { - tailTest := NewTailTest("incomplete-lines-long", t) + tailTest, cleanup := NewTailTest("incomplete-lines-long", t) + defer cleanup() filename := "test.txt" config := Config{ Follow: true, @@ -571,7 +573,8 @@ func TestIncompleteLongLines(t *testing.T) { } func TestIncompleteLinesWithReopens(t *testing.T) { - tailTest := NewTailTest("incomplete-lines-reopens", t) + tailTest, cleanup := NewTailTest("incomplete-lines-reopens", t) + defer cleanup() filename := "test.txt" config := Config{ Follow: true, @@ -596,7 +599,8 @@ func TestIncompleteLinesWithReopens(t *testing.T) { } func TestIncompleteLinesWithoutFollow(t *testing.T) { - tailTest := NewTailTest("incomplete-lines-no-follow", t) + tailTest, cleanup := NewTailTest("incomplete-lines-no-follow", t) + defer cleanup() filename := "test.txt" config := Config{ Follow: false, @@ -660,7 +664,7 @@ type TailTest struct { } func NewTailTest(name string, t *testing.T) (TailTest, func()) { - testdir, err := ioutil.TempDir("", "tail-test-" + name) + testdir, err := ioutil.TempDir("", "tail-test-"+name) if err != nil { t.Fatal(err) }