From 6cb4dc0ca8a6ca0fdd368da94dc40a8dfd36556f Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 16:46:31 -0700 Subject: [PATCH 1/7] add comments clarifying existing behaviour of readLine --- tail.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tail.go b/tail.go index babc269..6ec57bc 100644 --- a/tail.go +++ b/tail.go @@ -169,10 +169,18 @@ func (tail *Tail) reopen() error { func (tail *Tail) readLine() ([]byte, error) { line, isPrefix, err := tail.reader.ReadLine() + if err != nil { + return nil, err + } + + // If MaxLineSize is set, we don't have to join the parts (let + // Go's reader split them). if !isPrefix || tail.MaxLineSize > 0 { return line, err } + // Join lines from next read "if the line was too long for the + // buffer". XXX: why not use ReadBytes('\n') or Scanner? buf := append([]byte(nil), line...) for isPrefix && err == nil { line, isPrefix, err = tail.reader.ReadLine() From 6dab63b3c0979112aff22ba71e84adac2a120618 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 16:59:29 -0700 Subject: [PATCH 2/7] XXX --- tail.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tail.go b/tail.go index 6ec57bc..61cf465 100644 --- a/tail.go +++ b/tail.go @@ -170,7 +170,7 @@ func (tail *Tail) reopen() error { func (tail *Tail) readLine() ([]byte, error) { line, isPrefix, err := tail.reader.ReadLine() if err != nil { - return nil, err + return line, err } // If MaxLineSize is set, we don't have to join the parts (let @@ -244,6 +244,7 @@ func (tail *Tail) tailFileSync() { } } case io.EOF: + // XXX: should `line` be returned at this point? if !tail.Follow { return } From c7e3c77befe8e49efcba154ad6caddb71322a6d4 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 17:03:16 -0700 Subject: [PATCH 3/7] refactor newReader --- tail.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tail.go b/tail.go index 61cf465..7202580 100644 --- a/tail.go +++ b/tail.go @@ -214,7 +214,7 @@ func (tail *Tail) tailFileSync() { } } - tail.reader = tail.newReader() + tail.openReader() // Read line by line. for { @@ -295,7 +295,7 @@ func (tail *Tail) waitForChanges() error { return err } tail.Logger.Printf("Successfully reopened %s", tail.Filename) - tail.reader = tail.newReader() + tail.openReader() return nil } else { tail.Logger.Printf("Stopping tail as file no longer exists: %s", tail.Filename) @@ -308,7 +308,7 @@ func (tail *Tail) waitForChanges() error { return err } tail.Logger.Printf("Successfully reopened truncated %s", tail.Filename) - tail.reader = tail.newReader() + tail.openReader() return nil case <-tail.Dying(): return ErrStop @@ -316,12 +316,12 @@ func (tail *Tail) waitForChanges() error { panic("unreachable") } -func (tail *Tail) newReader() *bufio.Reader { +func (tail *Tail) openReader() { if tail.MaxLineSize > 0 { // add 2 to account for newline characters - return bufio.NewReaderSize(tail.file, tail.MaxLineSize+2) + tail.reader = bufio.NewReaderSize(tail.file, tail.MaxLineSize+2) } else { - return bufio.NewReader(tail.file) + tail.reader = bufio.NewReader(tail.file) } } From 929590016a94a9cf87937bbca681be261ff01fe4 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 18:01:10 -0700 Subject: [PATCH 4/7] use ReadString instead of ReadLine ReadLine is a low-level primitive, and we don't do a good job of splitting the lines as they are being read. best to read the entire line and then split it in one go. with this change, there is one test failure that will be resolved next: --- FAIL: TestMaxLineSize (0.10 seconds) tail_test.go:410: tail ended early; expecting more: [he] --- tail.go | 32 ++++++++++++-------------------- tail_test.go | 8 +++++--- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/tail.go b/tail.go index 7202580..f166589 100644 --- a/tail.go +++ b/tail.go @@ -13,6 +13,7 @@ import ( "launchpad.net/tomb" "log" "os" + "strings" "time" ) @@ -167,26 +168,18 @@ func (tail *Tail) reopen() error { return nil } -func (tail *Tail) readLine() ([]byte, error) { - line, isPrefix, err := tail.reader.ReadLine() +func (tail *Tail) readLine() (string, error) { + line, err := tail.reader.ReadString('\n') 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. return line, err } - // If MaxLineSize is set, we don't have to join the parts (let - // Go's reader split them). - if !isPrefix || tail.MaxLineSize > 0 { - return line, err - } + line = strings.TrimRight(line, "\n") - // Join lines from next read "if the line was too long for the - // buffer". XXX: why not use ReadBytes('\n') or Scanner? - buf := append([]byte(nil), line...) - for isPrefix && err == nil { - line, isPrefix, err = tail.reader.ReadLine() - buf = append(buf, line...) - } - return buf, err + return line, err } func (tail *Tail) tailFileSync() { @@ -222,7 +215,7 @@ func (tail *Tail) tailFileSync() { switch err { case nil: - if line != nil { + if true { cooloff := !tail.sendLine(line) if cooloff { // Wait a second before seeking till the end of @@ -337,14 +330,13 @@ func (tail *Tail) seekEnd() error { // sendLine sends the line(s) to Lines channel, splitting longer lines // if necessary. Return false if rate limit is reached. -func (tail *Tail) sendLine(line []byte) bool { +func (tail *Tail) sendLine(line string) bool { now := time.Now() - lines := []string{string(line)} + lines := []string{line} // Split longer lines if tail.MaxLineSize > 0 && len(line) > tail.MaxLineSize { - lines = util.PartitionString( - string(line), tail.MaxLineSize) + lines = util.PartitionString(line, tail.MaxLineSize) } for _, line := range lines { diff --git a/tail_test.go b/tail_test.go index c6ad712..0b9c5f9 100644 --- a/tail_test.go +++ b/tail_test.go @@ -85,7 +85,7 @@ func TestOver4096ByteLine(_t *testing.T) { func TestOver4096ByteLineWithSetMaxLineSize(_t *testing.T) { t := NewTailTest("Over4096ByteLineMaxLineSize", _t) testString := strings.Repeat("a", 4097) - t.CreateFile("test.txt", "test\r\n"+testString+"\r\nhello\r\nworld\r\n") + t.CreateFile("test.txt", "test\n"+testString+"\nhello\nworld\n") tail := t.StartTail("test.txt", Config{Follow: true, Location: nil, MaxLineSize: 4097}) go t.VerifyTailOutput(tail, []string{"test", testString, "hello", "world"}) @@ -93,7 +93,7 @@ func TestOver4096ByteLineWithSetMaxLineSize(_t *testing.T) { // to read all lines. <-time.After(100 * time.Millisecond) t.RemoveFile("test.txt") - tail.Stop() + // tail.Stop() Cleanup() } @@ -415,7 +415,9 @@ func (t TailTest) VerifyTailOutput(tail *Tail, lines []string) { // Note: not checking .Err as the `lines` argument is designed // to match error strings as well. if tailedLine.Text != line { - t.Fatalf("unexpected line/err from tail: expecting ```%s```, but got ```%s```", + t.Fatalf( + "unexpected line/err from tail: "+ + "expecting <<%s>>>, but got <<<%s>>>", line, tailedLine.Text) } } From 941cc3e3017a3a3c28e3b1b53b94dbaa0dd837e5 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 18:05:11 -0700 Subject: [PATCH 5/7] refactor this part of code for upcoming change --- tail.go | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/tail.go b/tail.go index f166589..89a9474 100644 --- a/tail.go +++ b/tail.go @@ -213,31 +213,27 @@ func (tail *Tail) tailFileSync() { for { line, err := tail.readLine() - switch err { - case nil: - if true { - cooloff := !tail.sendLine(line) - if cooloff { - // Wait a second before seeking till the end of - // file when rate limit is reached. - msg := fmt.Sprintf( - "Too much log activity; waiting a second " + - "before resuming tailing") - tail.Lines <- &Line{msg, time.Now(), fmt.Errorf(msg)} - select { - case <-time.After(time.Second): - case <-tail.Dying(): - return - } - err = tail.seekEnd() - if err != nil { - tail.Kill(err) - return - } + if err == nil { + cooloff := !tail.sendLine(line) + if cooloff { + // Wait a second before seeking till the end of + // file when rate limit is reached. + msg := fmt.Sprintf( + "Too much log activity; waiting a second " + + "before resuming tailing") + tail.Lines <- &Line{msg, time.Now(), fmt.Errorf(msg)} + select { + case <-time.After(time.Second): + case <-tail.Dying(): + return + } + err = tail.seekEnd() + if err != nil { + tail.Kill(err) + return } } - case io.EOF: - // XXX: should `line` be returned at this point? + }else if err == io.EOF { if !tail.Follow { return } @@ -251,7 +247,8 @@ func (tail *Tail) tailFileSync() { } return } - default: // non-EOF error + }else { + // non-EOF error tail.Killf("Error reading %s: %s", tail.Filename, err) return } From 136cda4961305ff120af46ab0b62439d1416c599 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 18:06:11 -0700 Subject: [PATCH 6/7] process line even if it doesn't end in newline especially as we use ReadString now --- tail.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tail.go b/tail.go index 89a9474..87debed 100644 --- a/tail.go +++ b/tail.go @@ -213,7 +213,8 @@ func (tail *Tail) tailFileSync() { for { line, err := tail.readLine() - if err == nil { + // Process `line` even if err is EOF. + if err == nil || (err == io.EOF && line != "") { cooloff := !tail.sendLine(line) if cooloff { // Wait a second before seeking till the end of From 4441c2eb1ba29cfa43be1787c96adebfeb762b39 Mon Sep 17 00:00:00 2001 From: Sridhar Ratnakumar Date: Fri, 16 May 2014 18:10:04 -0700 Subject: [PATCH 7/7] add -max param to gotail utility --- cmd/gotail/gotail.go | 3 +++ tail.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/gotail/gotail.go b/cmd/gotail/gotail.go index 4b5b5d3..12c61d3 100644 --- a/cmd/gotail/gotail.go +++ b/cmd/gotail/gotail.go @@ -12,7 +12,9 @@ import ( func args2config() (tail.Config, int64) { config := tail.Config{Follow: true} n := int64(0) + maxlinesize := int(0) flag.Int64Var(&n, "n", 0, "tail from the last Nth location") + flag.IntVar(&maxlinesize, "max", 0, "max line size") flag.BoolVar(&config.Follow, "f", false, "wait for additional data to be appended to the file") flag.BoolVar(&config.ReOpen, "F", false, "follow, and track file rename/rotation") flag.BoolVar(&config.Poll, "p", false, "use polling, instead of inotify") @@ -20,6 +22,7 @@ func args2config() (tail.Config, int64) { if config.ReOpen { config.Follow = true } + config.MaxLineSize = maxlinesize return config, n } diff --git a/tail.go b/tail.go index 87debed..e4b770b 100644 --- a/tail.go +++ b/tail.go @@ -234,7 +234,7 @@ func (tail *Tail) tailFileSync() { return } } - }else if err == io.EOF { + } else if err == io.EOF { if !tail.Follow { return } @@ -248,7 +248,7 @@ func (tail *Tail) tailFileSync() { } return } - }else { + } else { // non-EOF error tail.Killf("Error reading %s: %s", tail.Filename, err) return