From 7e04f9e7d9870e3f1734998ca45ad4cdab14fd18 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Thu, 2 Jun 2022 16:40:58 +0000 Subject: [PATCH 1/2] Add SyncOpen config option --- tail.go | 24 +++++++++++++++++++----- tail_test.go | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/tail.go b/tail.go index c962599..f6f3f22 100644 --- a/tail.go +++ b/tail.go @@ -73,6 +73,7 @@ type Config struct { Location *SeekInfo // Tail from this location. If nil, start at the beginning of the file ReOpen bool // Reopen recreated files (tail -F) MustExist bool // Fail early if the file does not exist + SyncOpen bool // Do any seek operation before returning from TailFile. Poll bool // Poll for file changes instead of using the default inotify Pipe bool // The file is a named pipe (mkfifo) @@ -146,12 +147,25 @@ func TailFile(filename string, config Config) (*Tail, error) { t.watcher = watch.NewInotifyFileWatcher(filename) } - if t.MustExist { + if t.MustExist || t.SyncOpen { var err error t.file, err = OpenFile(t.Filename) - if err != nil { - return nil, err - } + if t.MustExist { + if err != nil { + return nil, err + } + } + if t.SyncOpen && err == nil && t.Location != nil { + // Do any required seek operation before returning from TailFile. + // + // If the file didn't already exist, it's reasonable to assume that + // we shouldn't seek later when it opens, because its size when we + // first tried to open it was zero, so not seeking at all is fine. + _, err = t.file.Seek(t.Location.Offset, t.Location.Whence) + if err != nil { + return nil, err + } + } } go t.tailFileSync() @@ -283,7 +297,7 @@ func (tail *Tail) tailFileSync() { } // Seek to requested location on first open of the file. - if tail.Location != nil { + if tail.Location != nil && !tail.SyncOpen { _, err := tail.file.Seek(tail.Location.Offset, tail.Location.Whence) if err != nil { tail.Killf("Seek error on %s: %s", tail.Filename, err) diff --git a/tail_test.go b/tail_test.go index 7b9319e..a9499fc 100644 --- a/tail_test.go +++ b/tail_test.go @@ -66,6 +66,23 @@ func TestMustExist(t *testing.T) { tail.Cleanup() } +// This test is not very robust because its purpose is to test that a race +// condition doesn't exist. As such, the failure pattern of this test would +// be flaky failure, not necessarily easily reproduced failure. +// +// Running this same test *without* SyncOpen: true, alongside +// stress --hdd 8 (to make the filesystem very busy), will fail most +// of the time. (It fails significantly even without stress.) +func TestSeeksSynchronously(t *testing.T) { + tailTest, cleanup := NewTailTest("seeks-synchronously", t) + defer cleanup() + tailTest.CreateFile("test.txt", "hello\nworld\n") + tail := tailTest.StartTail("test.txt", Config{SyncOpen: true, Location: &SeekInfo{0, io.SeekEnd}}) + go tailTest.VerifyTailOutput(tail, []string{"more","data"}, false) + tailTest.AppendToFile("test.txt", "more\ndata\n") + tailTest.Cleanup(tail, true) +} + func TestWaitsForFileToExist(t *testing.T) { tailTest, cleanup := NewTailTest("waits-for-file-to-exist", t) defer cleanup() From 25c3059f45aee14e43a43780c8f72417590e250b Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 6 Jun 2022 14:35:51 +0000 Subject: [PATCH 2/2] gofmt --- tail.go | 32 ++++++++++++++++---------------- tail_test.go | 14 +++++++------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/tail.go b/tail.go index f6f3f22..73b4208 100644 --- a/tail.go +++ b/tail.go @@ -73,7 +73,7 @@ type Config struct { Location *SeekInfo // Tail from this location. If nil, start at the beginning of the file ReOpen bool // Reopen recreated files (tail -F) MustExist bool // Fail early if the file does not exist - SyncOpen bool // Do any seek operation before returning from TailFile. + SyncOpen bool // Do any seek operation before returning from TailFile. Poll bool // Poll for file changes instead of using the default inotify Pipe bool // The file is a named pipe (mkfifo) @@ -151,21 +151,21 @@ func TailFile(filename string, config Config) (*Tail, error) { var err error t.file, err = OpenFile(t.Filename) if t.MustExist { - if err != nil { - return nil, err - } - } - if t.SyncOpen && err == nil && t.Location != nil { - // Do any required seek operation before returning from TailFile. - // - // If the file didn't already exist, it's reasonable to assume that - // we shouldn't seek later when it opens, because its size when we - // first tried to open it was zero, so not seeking at all is fine. - _, err = t.file.Seek(t.Location.Offset, t.Location.Whence) - if err != nil { - return nil, err - } - } + if err != nil { + return nil, err + } + } + if t.SyncOpen && err == nil && t.Location != nil { + // Do any required seek operation before returning from TailFile. + // + // If the file didn't already exist, it's reasonable to assume that + // we shouldn't seek later when it opens, because its size when we + // first tried to open it was zero, so not seeking at all is fine. + _, err = t.file.Seek(t.Location.Offset, t.Location.Whence) + if err != nil { + return nil, err + } + } } go t.tailFileSync() diff --git a/tail_test.go b/tail_test.go index a9499fc..51a62a4 100644 --- a/tail_test.go +++ b/tail_test.go @@ -74,13 +74,13 @@ func TestMustExist(t *testing.T) { // stress --hdd 8 (to make the filesystem very busy), will fail most // of the time. (It fails significantly even without stress.) func TestSeeksSynchronously(t *testing.T) { - tailTest, cleanup := NewTailTest("seeks-synchronously", t) - defer cleanup() - tailTest.CreateFile("test.txt", "hello\nworld\n") - tail := tailTest.StartTail("test.txt", Config{SyncOpen: true, Location: &SeekInfo{0, io.SeekEnd}}) - go tailTest.VerifyTailOutput(tail, []string{"more","data"}, false) - tailTest.AppendToFile("test.txt", "more\ndata\n") - tailTest.Cleanup(tail, true) + tailTest, cleanup := NewTailTest("seeks-synchronously", t) + defer cleanup() + tailTest.CreateFile("test.txt", "hello\nworld\n") + tail := tailTest.StartTail("test.txt", Config{SyncOpen: true, Location: &SeekInfo{0, io.SeekEnd}}) + go tailTest.VerifyTailOutput(tail, []string{"more", "data"}, false) + tailTest.AppendToFile("test.txt", "more\ndata\n") + tailTest.Cleanup(tail, true) } func TestWaitsForFileToExist(t *testing.T) {