From 26582509ce721280aa656633f7b1f191add9d4bb Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Wed, 24 Apr 2024 13:09:00 -0500 Subject: [PATCH 01/10] feat(agent): output buffer persistence --- config/config.go | 5 + go.mod | 3 +- go.sum | 6 +- models/buffer.go | 238 ++++-------------- models/buffer_disk.go | 108 ++++++++ models/buffer_disk_test.go | 38 +++ models/buffer_mem.go | 191 ++++++++++++++ models/{buffer_test.go => buffer_mem_test.go} | 91 +++---- models/running_output.go | 7 +- 9 files changed, 453 insertions(+), 234 deletions(-) create mode 100644 models/buffer_disk.go create mode 100644 models/buffer_disk_test.go create mode 100644 models/buffer_mem.go rename models/{buffer_test.go => buffer_mem_test.go} (85%) diff --git a/config/config.go b/config/config.go index f0ccb0a80f507..fb8015a187d11 100644 --- a/config/config.go +++ b/config/config.go @@ -278,6 +278,9 @@ type AgentConfig struct { // Number of attempts to obtain a remote configuration via a URL during // startup. Set to -1 for unlimited attempts. ConfigURLRetryAttempts int `toml:"config_url_retry_attempts"` + + BufferStrategy string `toml:"buffer_strategy"` + BufferFileDirectory string `toml:"buffer_file_directory"` } // InputNames returns a list of strings of the configured inputs. @@ -1521,6 +1524,8 @@ func (c *Config) buildOutput(name string, tbl *ast.Table) (*models.OutputConfig, c.getFieldString(tbl, "name_suffix", &oc.NameSuffix) c.getFieldString(tbl, "name_prefix", &oc.NamePrefix) c.getFieldString(tbl, "startup_error_behavior", &oc.StartupErrorBehavior) + c.getFieldString(tbl, "buffer_strategy", &oc.BufferStrategy) + c.getFieldString(tbl, "buffer_file_directory", &oc.BufferFileDirectory) if c.hasErrs() { return nil, c.firstErr() diff --git a/go.mod b/go.mod index cc8c3a717145c..db7a47a766011 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( github.com/Masterminds/sprig/v3 v3.2.3 github.com/Mellanox/rdmamap v1.1.0 github.com/PaesslerAG/gval v1.2.2 + github.com/aarthikrao/wal v0.0.8 github.com/aerospike/aerospike-client-go/v5 v5.11.0 github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 github.com/alitto/pond v1.8.3 @@ -483,7 +484,7 @@ require ( go.opentelemetry.io/proto/otlp v1.0.0 // indirect go.uber.org/atomic v1.11.0 // indirect go.uber.org/multierr v1.11.0 // indirect - go.uber.org/zap v1.24.0 // indirect + go.uber.org/zap v1.25.0 // indirect golang.org/x/exp v0.0.0-20240529005216-23cca8864a10 // indirect golang.org/x/time v0.5.0 // indirect golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect diff --git a/go.sum b/go.sum index 3c46d8117dfa3..ec997d191b885 100644 --- a/go.sum +++ b/go.sum @@ -753,6 +753,8 @@ github.com/PaesslerAG/gval v1.2.2 h1:Y7iBzhgE09IGTt5QgGQ2IdaYYYOU134YGHBThD+wm9E github.com/PaesslerAG/gval v1.2.2/go.mod h1:XRFLwvmkTEdYziLdaCeCa5ImcGVrfQbeNUbVR+C6xac= github.com/PaesslerAG/jsonpath v0.1.0 h1:gADYeifvlqK3R3i2cR5B4DGgxLXIPb3TRTH1mGi0jPI= github.com/PaesslerAG/jsonpath v0.1.0/go.mod h1:4BzmtoM/PI8fPO4aQGIusjGxGir2BzcV0grWtFzq1Y8= +github.com/aarthikrao/wal v0.0.8 h1:ZC1QcS1NOxEFlTLEF/ZLAhhKVbRLxJJgUaORJ1YzPK4= +github.com/aarthikrao/wal v0.0.8/go.mod h1:C4QnBrOmMhCNsGnw1XuUh/7Y3aFRMLlh/JLYHOtpsPM= github.com/aerospike/aerospike-client-go/v5 v5.11.0 h1:z3ZmDSm3I10VMXXIIrsFCFq3IenwFqTCnLNyvnFVzrk= github.com/aerospike/aerospike-client-go/v5 v5.11.0/go.mod h1:e/zYeIoBg9We63fLKa+h+198+fT1GdoLfKa+Pu4QSpg= github.com/ajstarks/deck v0.0.0-20200831202436-30c9fc6549a9/go.mod h1:JynElWSGnm/4RlzPXRlREEwqTHAN3T56Bv2ITsFT3gY= @@ -2392,8 +2394,8 @@ go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee/go.mod h1:vJERXedbb3MVM5f9E go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.13.0/go.mod h1:zwrFLgMcdUuIBviXEYEH1YKNaOBnKXsx2IPda5bBwHM= -go.uber.org/zap v1.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60= -go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= +go.uber.org/zap v1.25.0 h1:4Hvk6GtkucQ790dqmj7l1eEnRdKm3k3ZUrUMS2d5+5c= +go.uber.org/zap v1.25.0/go.mod h1:JIAUzQIH94IC4fOJQm7gMmBJP5k7wQfdcnYdPoEXJYk= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20181029021203-45a5f77698d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= diff --git a/models/buffer.go b/models/buffer.go index 3f97a43a6482b..d14a85672d912 100644 --- a/models/buffer.go +++ b/models/buffer.go @@ -1,8 +1,6 @@ package models import ( - "sync" - "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/selfstat" ) @@ -12,18 +10,36 @@ var ( AgentMetricsDropped = selfstat.Register("agent", "metrics_dropped", map[string]string{}) ) -// Buffer stores metrics in a circular buffer. -type Buffer struct { - sync.Mutex - buf []telegraf.Metric - first int // index of the first/oldest metric - last int // one after the index of the last/newest metric - size int // number of metrics currently in the buffer - cap int // the capacity of the buffer +const ( + StrategyMemory = "memory" + StrategyDisk = "disk" + StrategyOverflow = "overflow" +) + +type Buffer interface { + + // Len returns the number of metrics currently in the buffer. + Len() int - batchFirst int // index of the first metric in the batch - batchSize int // number of metrics currently in the batch + // Add adds metrics to the buffer and returns number of dropped metrics. + Add(metrics ...telegraf.Metric) int + + // Batch returns a slice containing up to batchSize of the oldest metrics not + // yet dropped. Metrics are ordered from oldest to newest in the batch. The + // batch must not be modified by the client. + Batch(batchSize int) []telegraf.Metric + + // Accept marks the batch, acquired from Batch(), as successfully written. + Accept(metrics []telegraf.Metric) + + // Reject returns the batch, acquired from Batch(), to the buffer and marks it + // as unsent. + Reject([]telegraf.Metric) +} +// BufferMetrics holds common metrics used for buffer implementations. +// Implementations of Buffer should embed this struct in them. +type BufferMetrics struct { MetricsAdded selfstat.Stat MetricsWritten selfstat.Stat MetricsDropped selfstat.Stat @@ -32,19 +48,29 @@ type Buffer struct { } // NewBuffer returns a new empty Buffer with the given capacity. -func NewBuffer(name string, alias string, capacity int) *Buffer { +func NewBuffer(name string, alias string, capacity int, strategy string, path string) Buffer { + bm := NewBufferMetrics(name, alias, capacity) + + if strategy == StrategyDisk { + return NewDiskBuffer(name, capacity, path, bm) + } else if strategy == StrategyOverflow { + // todo implementme + // todo log currently unimplemented + return NewMemoryBuffer(capacity, bm) + } else if strategy == StrategyMemory || strategy == "" { + return NewMemoryBuffer(capacity, bm) + } + // todo log invalid buffer strategy configuration provided, falling back to memory + return NewMemoryBuffer(capacity, bm) +} + +func NewBufferMetrics(name string, alias string, capacity int) BufferMetrics { tags := map[string]string{"output": name} if alias != "" { tags["alias"] = alias } - b := &Buffer{ - buf: make([]telegraf.Metric, capacity), - first: 0, - last: 0, - size: 0, - cap: capacity, - + bm := BufferMetrics{ MetricsAdded: selfstat.Register( "write", "metrics_added", @@ -71,183 +97,23 @@ func NewBuffer(name string, alias string, capacity int) *Buffer { tags, ), } - b.BufferSize.Set(int64(0)) - b.BufferLimit.Set(int64(capacity)) - return b -} - -// Len returns the number of metrics currently in the buffer. -func (b *Buffer) Len() int { - b.Lock() - defer b.Unlock() - - return b.length() -} - -func (b *Buffer) length() int { - return min(b.size+b.batchSize, b.cap) + bm.BufferSize.Set(int64(0)) + bm.BufferLimit.Set(int64(capacity)) + return bm } -func (b *Buffer) metricAdded() { +func (b *BufferMetrics) metricAdded() { b.MetricsAdded.Incr(1) } -func (b *Buffer) metricWritten(metric telegraf.Metric) { +func (b *BufferMetrics) metricWritten(metric telegraf.Metric) { AgentMetricsWritten.Incr(1) b.MetricsWritten.Incr(1) metric.Accept() } -func (b *Buffer) metricDropped(metric telegraf.Metric) { +func (b *BufferMetrics) metricDropped(metric telegraf.Metric) { AgentMetricsDropped.Incr(1) b.MetricsDropped.Incr(1) metric.Reject() } - -func (b *Buffer) addMetric(m telegraf.Metric) int { - dropped := 0 - // Check if Buffer is full - if b.size == b.cap { - b.metricDropped(b.buf[b.last]) - dropped++ - - if b.batchSize > 0 { - b.batchSize-- - b.batchFirst = b.next(b.batchFirst) - } - } - - b.metricAdded() - - b.buf[b.last] = m - b.last = b.next(b.last) - - if b.size == b.cap { - b.first = b.next(b.first) - } - - b.size = min(b.size+1, b.cap) - return dropped -} - -// Add adds metrics to the buffer and returns number of dropped metrics. -func (b *Buffer) Add(metrics ...telegraf.Metric) int { - b.Lock() - defer b.Unlock() - - dropped := 0 - for i := range metrics { - if n := b.addMetric(metrics[i]); n != 0 { - dropped += n - } - } - - b.BufferSize.Set(int64(b.length())) - return dropped -} - -// Batch returns a slice containing up to batchSize of the oldest metrics not -// yet dropped. Metrics are ordered from oldest to newest in the batch. The -// batch must not be modified by the client. -func (b *Buffer) Batch(batchSize int) []telegraf.Metric { - b.Lock() - defer b.Unlock() - - outLen := min(b.size, batchSize) - out := make([]telegraf.Metric, outLen) - if outLen == 0 { - return out - } - - b.batchFirst = b.first - b.batchSize = outLen - - batchIndex := b.batchFirst - for i := range out { - out[i] = b.buf[batchIndex] - b.buf[batchIndex] = nil - batchIndex = b.next(batchIndex) - } - - b.first = b.nextby(b.first, b.batchSize) - b.size -= outLen - return out -} - -// Accept marks the batch, acquired from Batch(), as successfully written. -func (b *Buffer) Accept(batch []telegraf.Metric) { - b.Lock() - defer b.Unlock() - - for _, m := range batch { - b.metricWritten(m) - } - - b.resetBatch() - b.BufferSize.Set(int64(b.length())) -} - -// Reject returns the batch, acquired from Batch(), to the buffer and marks it -// as unsent. -func (b *Buffer) Reject(batch []telegraf.Metric) { - b.Lock() - defer b.Unlock() - - if len(batch) == 0 { - return - } - - free := b.cap - b.size - restore := min(len(batch), free) - skip := len(batch) - restore - - b.first = b.prevby(b.first, restore) - b.size = min(b.size+restore, b.cap) - - re := b.first - - // Copy metrics from the batch back into the buffer - for i := range batch { - if i < skip { - b.metricDropped(batch[i]) - } else { - b.buf[re] = batch[i] - re = b.next(re) - } - } - - b.resetBatch() - b.BufferSize.Set(int64(b.length())) -} - -// next returns the next index with wrapping. -func (b *Buffer) next(index int) int { - index++ - if index == b.cap { - return 0 - } - return index -} - -// nextby returns the index that is count newer with wrapping. -func (b *Buffer) nextby(index, count int) int { - index += count - index %= b.cap - return index -} - -// prevby returns the index that is count older with wrapping. -func (b *Buffer) prevby(index, count int) int { - index -= count - for index < 0 { - index += b.cap - } - - index %= b.cap - return index -} - -func (b *Buffer) resetBatch() { - b.batchFirst = 0 - b.batchSize = 0 -} diff --git a/models/buffer_disk.go b/models/buffer_disk.go new file mode 100644 index 0000000000000..dd293b913c866 --- /dev/null +++ b/models/buffer_disk.go @@ -0,0 +1,108 @@ +package models + +import ( + "bytes" + "encoding/gob" + "errors" + "fmt" + "time" + + "github.com/aarthikrao/wal" + "github.com/influxdata/telegraf" + "go.uber.org/zap" +) + +type DiskBuffer struct { + BufferMetrics + + walFile *wal.WriteAheadLog +} + +func NewDiskBuffer(name string, capacity int, path string, metrics BufferMetrics) *DiskBuffer { + log, err := zap.NewProduction() + if err != nil { + return nil // todo error handling + } + walFile, err := wal.NewWriteAheadLog(&wal.WALOptions{ + LogDir: path + "/" + name, + MaxLogSize: int64(capacity), + MaxSegments: 2, + Log: log, + MaxWaitBeforeSync: 1 * time.Second, + SyncMaxBytes: 1000, + }) + if err != nil { + return nil // todo error handling + } + return &DiskBuffer{ + BufferMetrics: metrics, + walFile: walFile, + } +} + +func (b *DiskBuffer) Len() int { + return -1 // todo +} + +func (b *DiskBuffer) Add(metrics ...telegraf.Metric) int { + written := 0 + for _, m := range metrics { + data := b.metricToBytes(m) + m.Accept() // accept here, since the metric object is no longer retained from here + _, err := b.walFile.Write(data) + if err == nil { + written++ + b.metricAdded() + } + } + return written +} + +func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { + metrics := make([]telegraf.Metric, batchSize) + index := 0 + _ = b.walFile.Replay(0, func(bytes []byte) error { + if index == batchSize { + return errors.New("stop parsing WAL file error") + } + metrics[index] = b.bytesToMetric(bytes) + index++ + return nil + }) + return metrics +} + +func (b *DiskBuffer) Accept(batch []telegraf.Metric) { + for _, m := range batch { + b.metricWritten(m) + } +} + +func (b *DiskBuffer) Reject(batch []telegraf.Metric) { + for _, m := range batch { + b.metricDropped(m) + } +} + +func (b *DiskBuffer) metricToBytes(metric telegraf.Metric) []byte { + var buf bytes.Buffer + encoder := gob.NewEncoder(&buf) + if err := encoder.Encode(metric); err != nil { + // todo error handle + fmt.Println("Error encoding:", err) + panic(1) + } + return buf.Bytes() +} + +func (b *DiskBuffer) bytesToMetric(data []byte) telegraf.Metric { + buf := bytes.NewBuffer(data) + decoder := gob.NewDecoder(buf) + var m telegraf.Metric + if err := decoder.Decode(&m); err != nil { + // todo error handle + fmt.Println("Error decoding:", err) + panic(1) + } + return m +} diff --git a/models/buffer_disk_test.go b/models/buffer_disk_test.go new file mode 100644 index 0000000000000..56f7cbf7eb351 --- /dev/null +++ b/models/buffer_disk_test.go @@ -0,0 +1,38 @@ +package models + +import ( + "os" + "testing" + + "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/testutil" +) + +func newTestDiskBuffer(name string, alias string, capacity int) (*DiskBuffer, string) { + bm := NewBufferMetrics(name, alias, capacity) + path, _ := os.MkdirTemp("", "*-disk_buffer-"+name) + return NewDiskBuffer(name, capacity, path, bm), path +} + +func setupDisk(b *DiskBuffer) *DiskBuffer { + b.MetricsAdded.Set(0) + b.MetricsWritten.Set(0) + b.MetricsDropped.Set(0) + return b +} + +func TestDiskBuffer(t *testing.T) { + db, path := newTestDiskBuffer("test", "", 5000) + defer os.RemoveAll(path) + db = setupDisk(db) + + db.Add(MetricTime(1)) + db.Add(MetricTime(2)) + db.Add(MetricTime(3)) + batch := db.Batch(2) + testutil.RequireMetricsEqual(t, + []telegraf.Metric{ + MetricTime(1), + MetricTime(2), + }, batch) +} diff --git a/models/buffer_mem.go b/models/buffer_mem.go new file mode 100644 index 0000000000000..f319f568c6180 --- /dev/null +++ b/models/buffer_mem.go @@ -0,0 +1,191 @@ +package models + +import ( + "sync" + + "github.com/influxdata/telegraf" +) + +// MemoryBuffer stores metrics in a circular buffer. +type MemoryBuffer struct { + sync.Mutex + BufferMetrics + + buf []telegraf.Metric + first int // index of the first/oldest metric + last int // one after the index of the last/newest metric + size int // number of metrics currently in the buffer + cap int // the capacity of the buffer + + batchFirst int // index of the first metric in the batch + batchSize int // number of metrics currently in the batch +} + +func NewMemoryBuffer(capacity int, metrics BufferMetrics) *MemoryBuffer { + return &MemoryBuffer{ + BufferMetrics: metrics, + buf: make([]telegraf.Metric, capacity), + first: 0, + last: 0, + size: 0, + cap: capacity, + } +} + +func (b *MemoryBuffer) Len() int { + b.Lock() + defer b.Unlock() + return b.length() +} + +func (b *MemoryBuffer) length() int { + return min(b.size+b.batchSize, b.cap) +} + +func (b *MemoryBuffer) addMetric(m telegraf.Metric) int { + dropped := 0 + // Check if Buffer is full + if b.size == b.cap { + b.metricDropped(b.buf[b.last]) + dropped++ + + if b.batchSize > 0 { + b.batchSize-- + b.batchFirst = b.next(b.batchFirst) + } + } + + b.metricAdded() + + b.buf[b.last] = m + b.last = b.next(b.last) + + if b.size == b.cap { + b.first = b.next(b.first) + } + + b.size = min(b.size+1, b.cap) + return dropped +} + +func (b *MemoryBuffer) Add(metrics ...telegraf.Metric) int { + b.Lock() + defer b.Unlock() + + dropped := 0 + for i := range metrics { + if n := b.addMetric(metrics[i]); n != 0 { + dropped += n + } + } + + b.BufferSize.Set(int64(b.length())) + return dropped +} + +func (b *MemoryBuffer) Batch(batchSize int) []telegraf.Metric { + b.Lock() + defer b.Unlock() + + outLen := min(b.size, batchSize) + out := make([]telegraf.Metric, outLen) + if outLen == 0 { + return out + } + + b.batchFirst = b.first + b.batchSize = outLen + + batchIndex := b.batchFirst + for i := range out { + out[i] = b.buf[batchIndex] + b.buf[batchIndex] = nil + batchIndex = b.next(batchIndex) + } + + b.first = b.nextby(b.first, b.batchSize) + b.size -= outLen + return out +} + +func (b *MemoryBuffer) Accept(batch []telegraf.Metric) { + b.Lock() + defer b.Unlock() + + for _, m := range batch { + b.metricWritten(m) + } + + b.resetBatch() + b.BufferSize.Set(int64(b.length())) +} + +func (b *MemoryBuffer) Reject(batch []telegraf.Metric) { + b.Lock() + defer b.Unlock() + + if len(batch) == 0 { + return + } + + free := b.cap - b.size + restore := min(len(batch), free) + skip := len(batch) - restore + + b.first = b.prevby(b.first, restore) + b.size = min(b.size+restore, b.cap) + + re := b.first + + // Copy metrics from the batch back into the buffer + for i := range batch { + if i < skip { + b.metricDropped(batch[i]) + } else { + b.buf[re] = batch[i] + re = b.next(re) + } + } + + b.resetBatch() + b.BufferSize.Set(int64(b.length())) +} + +// next returns the next index with wrapping. +func (b *MemoryBuffer) next(index int) int { + index++ + if index == b.cap { + return 0 + } + return index +} + +// nextby returns the index that is count newer with wrapping. +func (b *MemoryBuffer) nextby(index, count int) int { + index += count + index %= b.cap + return index +} + +// prevby returns the index that is count older with wrapping. +func (b *MemoryBuffer) prevby(index, count int) int { + index -= count + for index < 0 { + index += b.cap + } + + index %= b.cap + return index +} + +func (b *MemoryBuffer) resetBatch() { + b.batchFirst = 0 + b.batchSize = 0 +} + +func min(a, b int) int { + if b < a { + return b + } + return a +} diff --git a/models/buffer_test.go b/models/buffer_mem_test.go similarity index 85% rename from models/buffer_test.go rename to models/buffer_mem_test.go index 276b5c47cabbb..d434e87ee3490 100644 --- a/models/buffer_test.go +++ b/models/buffer_mem_test.go @@ -47,14 +47,19 @@ func MetricTime(sec int64) telegraf.Metric { } func BenchmarkAddMetrics(b *testing.B) { - buf := NewBuffer("test", "", 10000) + buf := newTestMemoryBuffer("test", "", 10000) m := Metric() for n := 0; n < b.N; n++ { buf.Add(m) } } -func setup(b *Buffer) *Buffer { +func newTestMemoryBuffer(name string, alias string, capacity int) *MemoryBuffer { + bm := NewBufferMetrics(name, alias, capacity) + return NewMemoryBuffer(capacity, bm) +} + +func setup(b *MemoryBuffer) *MemoryBuffer { b.MetricsAdded.Set(0) b.MetricsWritten.Set(0) b.MetricsDropped.Set(0) @@ -62,14 +67,14 @@ func setup(b *Buffer) *Buffer { } func TestBuffer_LenEmpty(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) require.Equal(t, 0, b.Len()) } func TestBuffer_LenOne(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m) require.Equal(t, 1, b.Len()) @@ -77,7 +82,7 @@ func TestBuffer_LenOne(t *testing.T) { func TestBuffer_LenFull(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) require.Equal(t, 5, b.Len()) @@ -85,7 +90,7 @@ func TestBuffer_LenFull(t *testing.T) { func TestBuffer_LenOverfill(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) setup(b) b.Add(m, m, m, m, m, m) @@ -93,14 +98,14 @@ func TestBuffer_LenOverfill(t *testing.T) { } func TestBuffer_BatchLenZero(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) batch := b.Batch(0) require.Empty(t, batch) } func TestBuffer_BatchLenBufferEmpty(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) batch := b.Batch(2) require.Empty(t, batch) @@ -108,7 +113,7 @@ func TestBuffer_BatchLenBufferEmpty(t *testing.T) { func TestBuffer_BatchLenUnderfill(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m) batch := b.Batch(2) @@ -117,7 +122,7 @@ func TestBuffer_BatchLenUnderfill(t *testing.T) { func TestBuffer_BatchLenFill(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m) batch := b.Batch(2) require.Len(t, batch, 2) @@ -125,7 +130,7 @@ func TestBuffer_BatchLenFill(t *testing.T) { func TestBuffer_BatchLenExact(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m) batch := b.Batch(2) require.Len(t, batch, 2) @@ -133,7 +138,7 @@ func TestBuffer_BatchLenExact(t *testing.T) { func TestBuffer_BatchLenLargerThanBuffer(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(6) require.Len(t, batch, 5) @@ -141,7 +146,7 @@ func TestBuffer_BatchLenLargerThanBuffer(t *testing.T) { func TestBuffer_BatchWrap(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(2) b.Accept(batch) @@ -151,7 +156,7 @@ func TestBuffer_BatchWrap(t *testing.T) { } func TestBuffer_BatchLatest(t *testing.T) { - b := setup(NewBuffer("test", "", 4)) + b := setup(newTestMemoryBuffer("test", "", 4)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -165,7 +170,7 @@ func TestBuffer_BatchLatest(t *testing.T) { } func TestBuffer_BatchLatestWrap(t *testing.T) { - b := setup(NewBuffer("test", "", 4)) + b := setup(newTestMemoryBuffer("test", "", 4)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -181,7 +186,7 @@ func TestBuffer_BatchLatestWrap(t *testing.T) { } func TestBuffer_MultipleBatch(t *testing.T) { - b := setup(NewBuffer("test", "", 10)) + b := setup(newTestMemoryBuffer("test", "", 10)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -207,7 +212,7 @@ func TestBuffer_MultipleBatch(t *testing.T) { } func TestBuffer_RejectWithRoom(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -230,7 +235,7 @@ func TestBuffer_RejectWithRoom(t *testing.T) { } func TestBuffer_RejectNothingNewFull(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -253,7 +258,7 @@ func TestBuffer_RejectNothingNewFull(t *testing.T) { } func TestBuffer_RejectNoRoom(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) @@ -282,7 +287,7 @@ func TestBuffer_RejectNoRoom(t *testing.T) { } func TestBuffer_RejectRoomExact(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) batch := b.Batch(2) @@ -306,7 +311,7 @@ func TestBuffer_RejectRoomExact(t *testing.T) { } func TestBuffer_RejectRoomOverwriteOld(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -331,7 +336,7 @@ func TestBuffer_RejectRoomOverwriteOld(t *testing.T) { } func TestBuffer_RejectPartialRoom(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) @@ -358,7 +363,7 @@ func TestBuffer_RejectPartialRoom(t *testing.T) { } func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -401,7 +406,7 @@ func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { } func TestBuffer_RejectWrapped(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -432,7 +437,7 @@ func TestBuffer_RejectWrapped(t *testing.T) { } func TestBuffer_RejectAdjustFirst(t *testing.T) { - b := setup(NewBuffer("test", "", 10)) + b := setup(newTestMemoryBuffer("test", "", 10)) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -480,7 +485,7 @@ func TestBuffer_RejectAdjustFirst(t *testing.T) { func TestBuffer_AddDropsOverwrittenMetrics(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) b.Add(m, m, m, m, m) @@ -491,7 +496,7 @@ func TestBuffer_AddDropsOverwrittenMetrics(t *testing.T) { func TestBuffer_AcceptRemovesBatch(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m) batch := b.Batch(2) b.Accept(batch) @@ -500,7 +505,7 @@ func TestBuffer_AcceptRemovesBatch(t *testing.T) { func TestBuffer_RejectLeavesBatch(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m) batch := b.Batch(2) b.Reject(batch) @@ -509,7 +514,7 @@ func TestBuffer_RejectLeavesBatch(t *testing.T) { func TestBuffer_AcceptWritesOverwrittenBatch(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(5) @@ -522,7 +527,7 @@ func TestBuffer_AcceptWritesOverwrittenBatch(t *testing.T) { func TestBuffer_BatchRejectDropsOverwrittenBatch(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(5) @@ -535,7 +540,7 @@ func TestBuffer_BatchRejectDropsOverwrittenBatch(t *testing.T) { func TestBuffer_MetricsOverwriteBatchAccept(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(3) @@ -547,7 +552,7 @@ func TestBuffer_MetricsOverwriteBatchAccept(t *testing.T) { func TestBuffer_MetricsOverwriteBatchReject(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(3) @@ -559,7 +564,7 @@ func TestBuffer_MetricsOverwriteBatchReject(t *testing.T) { func TestBuffer_MetricsBatchAcceptRemoved(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(3) @@ -571,7 +576,7 @@ func TestBuffer_MetricsBatchAcceptRemoved(t *testing.T) { func TestBuffer_WrapWithBatch(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m) b.Batch(3) @@ -582,7 +587,7 @@ func TestBuffer_WrapWithBatch(t *testing.T) { func TestBuffer_BatchNotRemoved(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) b.Batch(2) require.Equal(t, 5, b.Len()) @@ -590,7 +595,7 @@ func TestBuffer_BatchNotRemoved(t *testing.T) { func TestBuffer_BatchRejectAcceptNoop(t *testing.T) { m := Metric() - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(m, m, m, m, m) batch := b.Batch(2) b.Reject(batch) @@ -606,7 +611,7 @@ func TestBuffer_AcceptCallsMetricAccept(t *testing.T) { accept++ }, } - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(mm, mm, mm) batch := b.Batch(2) b.Accept(batch) @@ -621,7 +626,7 @@ func TestBuffer_AddCallsMetricRejectWhenNoBatch(t *testing.T) { reject++ }, } - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) setup(b) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm) @@ -636,7 +641,7 @@ func TestBuffer_AddCallsMetricRejectWhenNotInBatch(t *testing.T) { reject++ }, } - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) setup(b) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(2) @@ -654,7 +659,7 @@ func TestBuffer_RejectCallsMetricRejectWithOverwritten(t *testing.T) { reject++ }, } - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(5) b.Add(mm, mm) @@ -671,7 +676,7 @@ func TestBuffer_AddOverwriteAndReject(t *testing.T) { reject++ }, } - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(5) b.Add(mm, mm, mm, mm, mm) @@ -695,7 +700,7 @@ func TestBuffer_AddOverwriteAndRejectOffset(t *testing.T) { accept++ }, } - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) b.Add(mm, mm, mm) b.Add(mm, mm, mm, mm) require.Equal(t, 2, reject) @@ -714,7 +719,7 @@ func TestBuffer_AddOverwriteAndRejectOffset(t *testing.T) { } func TestBuffer_RejectEmptyBatch(t *testing.T) { - b := setup(NewBuffer("test", "", 5)) + b := setup(newTestMemoryBuffer("test", "", 5)) batch := b.Batch(2) b.Add(MetricTime(1)) b.Reject(batch) diff --git a/models/running_output.go b/models/running_output.go index ada1f745f2c6d..91123b2cde749 100644 --- a/models/running_output.go +++ b/models/running_output.go @@ -37,6 +37,9 @@ type OutputConfig struct { NameOverride string NamePrefix string NameSuffix string + + BufferStrategy string + BufferFileDirectory string } // RunningOutput contains the output configuration @@ -56,7 +59,7 @@ type RunningOutput struct { BatchReady chan time.Time - buffer *Buffer + buffer Buffer log telegraf.Logger started bool @@ -97,7 +100,7 @@ func NewRunningOutput( } ro := &RunningOutput{ - buffer: NewBuffer(config.Name, config.Alias, bufferLimit), + buffer: NewBuffer(config.Name, config.Alias, bufferLimit, config.BufferStrategy, config.BufferFileDirectory), BatchReady: make(chan time.Time, 1), Output: output, Config: config, From 38cdb70fbfe3b249a5ade4597d917d74db0a0fe6 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:55:21 -0500 Subject: [PATCH 02/10] move to tidwall/wal library --- go.mod | 3 +- go.sum | 7 ++- models/buffer_disk.go | 101 ++++++++++++++++++++++++++++++------------ 3 files changed, 79 insertions(+), 32 deletions(-) diff --git a/go.mod b/go.mod index db7a47a766011..7dd11326796f4 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,6 @@ require ( github.com/Masterminds/sprig/v3 v3.2.3 github.com/Mellanox/rdmamap v1.1.0 github.com/PaesslerAG/gval v1.2.2 - github.com/aarthikrao/wal v0.0.8 github.com/aerospike/aerospike-client-go/v5 v5.11.0 github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 github.com/alitto/pond v1.8.3 @@ -186,6 +185,7 @@ require ( github.com/testcontainers/testcontainers-go/modules/kafka v0.30.0 github.com/thomasklein94/packer-plugin-libvirt v0.5.0 github.com/tidwall/gjson v1.17.0 + github.com/tidwall/wal v1.1.7 github.com/tinylib/msgp v1.1.9 github.com/urfave/cli/v2 v2.27.1 github.com/vapourismo/knx-go v0.0.0-20240217175130-922a0d50c241 @@ -452,6 +452,7 @@ require ( github.com/stretchr/objx v0.5.2 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.0 // indirect + github.com/tidwall/tinylru v1.1.0 // indirect github.com/tklauser/go-sysconf v0.3.13 // indirect github.com/tklauser/numcpus v0.7.0 // indirect github.com/twmb/murmur3 v1.1.7 // indirect diff --git a/go.sum b/go.sum index ec997d191b885..0ee20e01374a4 100644 --- a/go.sum +++ b/go.sum @@ -753,8 +753,6 @@ github.com/PaesslerAG/gval v1.2.2 h1:Y7iBzhgE09IGTt5QgGQ2IdaYYYOU134YGHBThD+wm9E github.com/PaesslerAG/gval v1.2.2/go.mod h1:XRFLwvmkTEdYziLdaCeCa5ImcGVrfQbeNUbVR+C6xac= github.com/PaesslerAG/jsonpath v0.1.0 h1:gADYeifvlqK3R3i2cR5B4DGgxLXIPb3TRTH1mGi0jPI= github.com/PaesslerAG/jsonpath v0.1.0/go.mod h1:4BzmtoM/PI8fPO4aQGIusjGxGir2BzcV0grWtFzq1Y8= -github.com/aarthikrao/wal v0.0.8 h1:ZC1QcS1NOxEFlTLEF/ZLAhhKVbRLxJJgUaORJ1YzPK4= -github.com/aarthikrao/wal v0.0.8/go.mod h1:C4QnBrOmMhCNsGnw1XuUh/7Y3aFRMLlh/JLYHOtpsPM= github.com/aerospike/aerospike-client-go/v5 v5.11.0 h1:z3ZmDSm3I10VMXXIIrsFCFq3IenwFqTCnLNyvnFVzrk= github.com/aerospike/aerospike-client-go/v5 v5.11.0/go.mod h1:e/zYeIoBg9We63fLKa+h+198+fT1GdoLfKa+Pu4QSpg= github.com/ajstarks/deck v0.0.0-20200831202436-30c9fc6549a9/go.mod h1:JynElWSGnm/4RlzPXRlREEwqTHAN3T56Bv2ITsFT3gY= @@ -2224,12 +2222,17 @@ github.com/testcontainers/testcontainers-go/modules/kafka v0.30.0 h1:lQx20102vAH github.com/testcontainers/testcontainers-go/modules/kafka v0.30.0/go.mod h1:n3m3SH0ivwFZbehY8fgTLADfwSPK2ZC5za4r9nYYm4Q= github.com/thomasklein94/packer-plugin-libvirt v0.5.0 h1:aj2HLHZZM/ClGLIwVp9rrgh+2TOU/w4EiaZHAwCpOgs= github.com/thomasklein94/packer-plugin-libvirt v0.5.0/go.mod h1:GwN82FQ6KxCNKtS8LNUgLbwTZs90GGhBzCmTNkrTCrY= +github.com/tidwall/gjson v1.10.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/gjson v1.17.0 h1:/Jocvlh98kcTfpN2+JzGQWQcqrPQwDrVEMApx/M5ZwM= github.com/tidwall/gjson v1.17.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/tinylru v1.1.0 h1:XY6IUfzVTU9rpwdhKUF6nQdChgCdGjkMfLzbWyiau6I= +github.com/tidwall/tinylru v1.1.0/go.mod h1:3+bX+TJ2baOLMWTnlyNWHh4QMnFyARg2TLTQ6OFbzw8= +github.com/tidwall/wal v1.1.7 h1:emc1TRjIVsdKKSnpwGBAcsAGg0767SvUk8+ygx7Bb+4= +github.com/tidwall/wal v1.1.7/go.mod h1:r6lR1j27W9EPalgHiB7zLJDYu3mzW5BQP5KrzBpYY/E= github.com/tinylib/msgp v1.1.9 h1:SHf3yoO2sGA0veCJeCBYLHuttAVFHGm2RHgNodW7wQU= github.com/tinylib/msgp v1.1.9/go.mod h1:BCXGB54lDD8qUEPmiG0cQQUANC4IUQyB2ItS2UDlO/k= github.com/tj/assert v0.0.0-20171129193455-018094318fb0/go.mod h1:mZ9/Rh9oLWpLLDRpvE+3b7gP/C2YyLFYxNmcLnPTMe0= diff --git a/models/buffer_disk.go b/models/buffer_disk.go index dd293b913c866..e06358dbd7730 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -3,34 +3,21 @@ package models import ( "bytes" "encoding/gob" - "errors" "fmt" - "time" - "github.com/aarthikrao/wal" "github.com/influxdata/telegraf" - "go.uber.org/zap" + "github.com/tidwall/wal" ) type DiskBuffer struct { BufferMetrics - walFile *wal.WriteAheadLog + walFile *wal.Log } func NewDiskBuffer(name string, capacity int, path string, metrics BufferMetrics) *DiskBuffer { - log, err := zap.NewProduction() - if err != nil { - return nil // todo error handling - } - walFile, err := wal.NewWriteAheadLog(&wal.WALOptions{ - LogDir: path + "/" + name, - MaxLogSize: int64(capacity), - MaxSegments: 2, - Log: log, - MaxWaitBeforeSync: 1 * time.Second, - SyncMaxBytes: 1000, - }) + // todo capacity + walFile, err := wal.Open(path+"/"+name, nil) if err != nil { return nil // todo error handling } @@ -41,34 +28,82 @@ func NewDiskBuffer(name string, capacity int, path string, metrics BufferMetrics } func (b *DiskBuffer) Len() int { - return -1 // todo + return int(b.writeIndex() - b.readIndex()) +} + +// readIndex is the first index to start reading metrics from, or the head of the buffer +func (b *DiskBuffer) readIndex() uint64 { + index, err := b.walFile.FirstIndex() + if err != nil { + panic(err) // can only occur with a corrupt wal file + } + return index +} + +// writeIndex is the first index to start writing metrics to, or the tail of the buffer +func (b *DiskBuffer) writeIndex() uint64 { + index, err := b.walFile.LastIndex() + if err != nil { + panic(err) // can only occur with a corrupt wal file + } + return index } func (b *DiskBuffer) Add(metrics ...telegraf.Metric) int { + // one metric to write, can write directly + if len(metrics) == 1 { + if b.addSingle(metrics[0]) { + return 1 + } + return 0 + } + + // multiple metrics to write, batch them + return b.addBatch(metrics) +} + +func (b *DiskBuffer) addSingle(metric telegraf.Metric) bool { + err := b.walFile.Write(b.writeIndex(), b.metricToBytes(metric)) + metric.Accept() + if err == nil { + b.metricAdded() + return true + } + return false +} + +func (b *DiskBuffer) addBatch(metrics []telegraf.Metric) int { written := 0 + batch := new(wal.Batch) for _, m := range metrics { data := b.metricToBytes(m) m.Accept() // accept here, since the metric object is no longer retained from here - _, err := b.walFile.Write(data) - if err == nil { - written++ - b.metricAdded() - } + batch.Write(b.writeIndex(), data) + b.metricAdded() + written++ + } + err := b.walFile.WriteBatch(batch) + if err != nil { + return 0 // todo error handle, test if a partial write occur } return written } func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { + if b.Len() == 0 { + // no metrics in the wal file, so return an empty array + return make([]telegraf.Metric, 0) + } metrics := make([]telegraf.Metric, batchSize) index := 0 - _ = b.walFile.Replay(0, func(bytes []byte) error { - if index == batchSize { - return errors.New("stop parsing WAL file error") + for i := b.readIndex(); i < b.readIndex()+uint64(batchSize); i++ { + data, err := b.walFile.Read(i) + if err != nil { + // todo error handle } - metrics[index] = b.bytesToMetric(bytes) + metrics[index] = b.bytesToMetric(data) index++ - return nil - }) + } return metrics } @@ -76,12 +111,20 @@ func (b *DiskBuffer) Accept(batch []telegraf.Metric) { for _, m := range batch { b.metricWritten(m) } + err := b.walFile.TruncateFront(b.readIndex() + uint64(len(batch))) + if err != nil { + panic(err) // can only occur with a corrupt wal file + } } func (b *DiskBuffer) Reject(batch []telegraf.Metric) { for _, m := range batch { b.metricDropped(m) } + err := b.walFile.TruncateFront(b.readIndex() + uint64(len(batch))) + if err != nil { + panic(err) // can only occur with a corrupt wal file + } } func (b *DiskBuffer) metricToBytes(metric telegraf.Metric) []byte { From 08a1c93b3b001386958e9101fad8e30ce737f4a9 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Tue, 30 Apr 2024 11:01:24 -0500 Subject: [PATCH 03/10] address some review comments --- config/config.go | 6 +++--- models/buffer.go | 30 +++++++++++++----------------- models/buffer_disk.go | 8 ++++---- models/buffer_mem.go | 16 ++++++++-------- models/running_output.go | 6 +++--- 5 files changed, 31 insertions(+), 35 deletions(-) diff --git a/config/config.go b/config/config.go index fb8015a187d11..87605890c5586 100644 --- a/config/config.go +++ b/config/config.go @@ -279,8 +279,8 @@ type AgentConfig struct { // startup. Set to -1 for unlimited attempts. ConfigURLRetryAttempts int `toml:"config_url_retry_attempts"` - BufferStrategy string `toml:"buffer_strategy"` - BufferFileDirectory string `toml:"buffer_file_directory"` + BufferStrategy string `toml:"buffer_strategy"` + BufferDirectory string `toml:"buffer_directory"` } // InputNames returns a list of strings of the configured inputs. @@ -1525,7 +1525,7 @@ func (c *Config) buildOutput(name string, tbl *ast.Table) (*models.OutputConfig, c.getFieldString(tbl, "name_prefix", &oc.NamePrefix) c.getFieldString(tbl, "startup_error_behavior", &oc.StartupErrorBehavior) c.getFieldString(tbl, "buffer_strategy", &oc.BufferStrategy) - c.getFieldString(tbl, "buffer_file_directory", &oc.BufferFileDirectory) + c.getFieldString(tbl, "buffer_directory", &oc.BufferDirectory) if c.hasErrs() { return nil, c.firstErr() diff --git a/models/buffer.go b/models/buffer.go index d14a85672d912..dd87c00dbd475 100644 --- a/models/buffer.go +++ b/models/buffer.go @@ -10,12 +10,6 @@ var ( AgentMetricsDropped = selfstat.Register("agent", "metrics_dropped", map[string]string{}) ) -const ( - StrategyMemory = "memory" - StrategyDisk = "disk" - StrategyOverflow = "overflow" -) - type Buffer interface { // Len returns the number of metrics currently in the buffer. @@ -37,9 +31,9 @@ type Buffer interface { Reject([]telegraf.Metric) } -// BufferMetrics holds common metrics used for buffer implementations. +// BufferStats holds common metrics used for buffer implementations. // Implementations of Buffer should embed this struct in them. -type BufferMetrics struct { +type BufferStats struct { MetricsAdded selfstat.Stat MetricsWritten selfstat.Stat MetricsDropped selfstat.Stat @@ -51,26 +45,28 @@ type BufferMetrics struct { func NewBuffer(name string, alias string, capacity int, strategy string, path string) Buffer { bm := NewBufferMetrics(name, alias, capacity) - if strategy == StrategyDisk { + switch strategy { + case "", "memory": + return NewMemoryBuffer(capacity, bm) + case "disk": return NewDiskBuffer(name, capacity, path, bm) - } else if strategy == StrategyOverflow { + case "overflow": // todo implementme // todo log currently unimplemented return NewMemoryBuffer(capacity, bm) - } else if strategy == StrategyMemory || strategy == "" { - return NewMemoryBuffer(capacity, bm) } + // todo log invalid buffer strategy configuration provided, falling back to memory return NewMemoryBuffer(capacity, bm) } -func NewBufferMetrics(name string, alias string, capacity int) BufferMetrics { +func NewBufferMetrics(name string, alias string, capacity int) BufferStats { tags := map[string]string{"output": name} if alias != "" { tags["alias"] = alias } - bm := BufferMetrics{ + bm := BufferStats{ MetricsAdded: selfstat.Register( "write", "metrics_added", @@ -102,17 +98,17 @@ func NewBufferMetrics(name string, alias string, capacity int) BufferMetrics { return bm } -func (b *BufferMetrics) metricAdded() { +func (b *BufferStats) metricAdded() { b.MetricsAdded.Incr(1) } -func (b *BufferMetrics) metricWritten(metric telegraf.Metric) { +func (b *BufferStats) metricWritten(metric telegraf.Metric) { AgentMetricsWritten.Incr(1) b.MetricsWritten.Incr(1) metric.Accept() } -func (b *BufferMetrics) metricDropped(metric telegraf.Metric) { +func (b *BufferStats) metricDropped(metric telegraf.Metric) { AgentMetricsDropped.Incr(1) b.MetricsDropped.Incr(1) metric.Reject() diff --git a/models/buffer_disk.go b/models/buffer_disk.go index e06358dbd7730..5e0a3fda7e00d 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -10,20 +10,20 @@ import ( ) type DiskBuffer struct { - BufferMetrics + BufferStats walFile *wal.Log } -func NewDiskBuffer(name string, capacity int, path string, metrics BufferMetrics) *DiskBuffer { +func NewDiskBuffer(name string, capacity int, path string, stats BufferStats) *DiskBuffer { // todo capacity walFile, err := wal.Open(path+"/"+name, nil) if err != nil { return nil // todo error handling } return &DiskBuffer{ - BufferMetrics: metrics, - walFile: walFile, + BufferStats: stats, + walFile: walFile, } } diff --git a/models/buffer_mem.go b/models/buffer_mem.go index f319f568c6180..a58cef61b9474 100644 --- a/models/buffer_mem.go +++ b/models/buffer_mem.go @@ -9,7 +9,7 @@ import ( // MemoryBuffer stores metrics in a circular buffer. type MemoryBuffer struct { sync.Mutex - BufferMetrics + BufferStats buf []telegraf.Metric first int // index of the first/oldest metric @@ -21,14 +21,14 @@ type MemoryBuffer struct { batchSize int // number of metrics currently in the batch } -func NewMemoryBuffer(capacity int, metrics BufferMetrics) *MemoryBuffer { +func NewMemoryBuffer(capacity int, stats BufferStats) *MemoryBuffer { return &MemoryBuffer{ - BufferMetrics: metrics, - buf: make([]telegraf.Metric, capacity), - first: 0, - last: 0, - size: 0, - cap: capacity, + BufferStats: stats, + buf: make([]telegraf.Metric, capacity), + first: 0, + last: 0, + size: 0, + cap: capacity, } } diff --git a/models/running_output.go b/models/running_output.go index 91123b2cde749..82ab88b1ceb5d 100644 --- a/models/running_output.go +++ b/models/running_output.go @@ -38,8 +38,8 @@ type OutputConfig struct { NamePrefix string NameSuffix string - BufferStrategy string - BufferFileDirectory string + BufferStrategy string + BufferDirectory string } // RunningOutput contains the output configuration @@ -100,7 +100,7 @@ func NewRunningOutput( } ro := &RunningOutput{ - buffer: NewBuffer(config.Name, config.Alias, bufferLimit, config.BufferStrategy, config.BufferFileDirectory), + buffer: NewBuffer(config.Name, config.Alias, bufferLimit, config.BufferStrategy, config.BufferDirectory), BatchReady: make(chan time.Time, 1), Output: output, Config: config, From 83f410c07e757e4ebd754a8db8ca07eaf54026b7 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Mon, 13 May 2024 12:03:05 -0500 Subject: [PATCH 04/10] suite tests, error handling, work on new encoding --- metric.go | 6 + metric/metric.go | 20 + models/buffer.go | 4 +- models/buffer_disk.go | 54 +-- models/buffer_disk_test.go | 38 -- models/buffer_mem.go | 11 +- ...uffer_mem_test.go => buffer_suite_test.go} | 387 +++++++++--------- models/mock/metric.go | 22 + models/running_output.go | 6 +- 9 files changed, 281 insertions(+), 267 deletions(-) delete mode 100644 models/buffer_disk_test.go rename models/{buffer_mem_test.go => buffer_suite_test.go} (50%) create mode 100644 models/mock/metric.go diff --git a/metric.go b/metric.go index 2cf3d6a06072b..971a291bb947b 100644 --- a/metric.go +++ b/metric.go @@ -125,6 +125,12 @@ type Metric interface { // Drop marks the metric as processed successfully without being written // to any output. Drop() + + // ToBytes converts the metric a byte array using the gob encoder. + ToBytes() ([]byte, error) + + // FromBytes populates a metrics data using a binary byte array. + FromBytes([]byte) error } // TemplateMetric is an interface to use in templates (e.g text/template) diff --git a/metric/metric.go b/metric/metric.go index f07f54c2672e6..a5d7e4565adb1 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -1,6 +1,8 @@ package metric import ( + "bytes" + "encoding/gob" "fmt" "hash/fnv" "sort" @@ -384,3 +386,21 @@ func convertField(v interface{}) interface{} { } return nil } + +func (m *metric) ToBytes() ([]byte, error) { + var buf bytes.Buffer + encoder := gob.NewEncoder(&buf) + if err := encoder.Encode(m); err != nil { + return nil, fmt.Errorf("failed to encode metric to bytes: %w", err) + } + return buf.Bytes(), nil +} + +func (m *metric) FromBytes(b []byte) error { + buf := bytes.NewBuffer(b) + decoder := gob.NewDecoder(buf) + if err := decoder.Decode(&m); err != nil { + return fmt.Errorf("failed to decode metric from bytes: %w", err) + } + return nil +} diff --git a/models/buffer.go b/models/buffer.go index dd87c00dbd475..13e053f2e8dbc 100644 --- a/models/buffer.go +++ b/models/buffer.go @@ -29,6 +29,8 @@ type Buffer interface { // Reject returns the batch, acquired from Batch(), to the buffer and marks it // as unsent. Reject([]telegraf.Metric) + + Stats() BufferStats } // BufferStats holds common metrics used for buffer implementations. @@ -42,7 +44,7 @@ type BufferStats struct { } // NewBuffer returns a new empty Buffer with the given capacity. -func NewBuffer(name string, alias string, capacity int, strategy string, path string) Buffer { +func NewBuffer(name string, alias string, capacity int, strategy string, path string) (Buffer, error) { bm := NewBufferMetrics(name, alias, capacity) switch strategy { diff --git a/models/buffer_disk.go b/models/buffer_disk.go index 5e0a3fda7e00d..bb12348199f1c 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -1,30 +1,26 @@ package models import ( - "bytes" - "encoding/gob" "fmt" - "github.com/influxdata/telegraf" "github.com/tidwall/wal" ) type DiskBuffer struct { BufferStats - walFile *wal.Log } -func NewDiskBuffer(name string, capacity int, path string, stats BufferStats) *DiskBuffer { +func NewDiskBuffer(name string, capacity int, path string, stats BufferStats) (*DiskBuffer, error) { // todo capacity walFile, err := wal.Open(path+"/"+name, nil) if err != nil { - return nil // todo error handling + return nil, fmt.Errorf("failed to open wal file: %w", err) } return &DiskBuffer{ BufferStats: stats, walFile: walFile, - } + }, nil } func (b *DiskBuffer) Len() int { @@ -62,9 +58,13 @@ func (b *DiskBuffer) Add(metrics ...telegraf.Metric) int { return b.addBatch(metrics) } -func (b *DiskBuffer) addSingle(metric telegraf.Metric) bool { - err := b.walFile.Write(b.writeIndex(), b.metricToBytes(metric)) - metric.Accept() +func (b *DiskBuffer) addSingle(m telegraf.Metric) bool { + data, err := m.ToBytes() + if err != nil { + panic(err) + } + err = b.walFile.Write(b.writeIndex(), data) + m.Accept() if err == nil { b.metricAdded() return true @@ -76,7 +76,10 @@ func (b *DiskBuffer) addBatch(metrics []telegraf.Metric) int { written := 0 batch := new(wal.Batch) for _, m := range metrics { - data := b.metricToBytes(m) + data, err := m.ToBytes() + if err != nil { + panic(err) + } m.Accept() // accept here, since the metric object is no longer retained from here batch.Write(b.writeIndex(), data) b.metricAdded() @@ -101,7 +104,11 @@ func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { if err != nil { // todo error handle } - metrics[index] = b.bytesToMetric(data) + var m telegraf.Metric + if err = m.FromBytes(data); err != nil { + panic(err) + } + metrics[index] = m index++ } return metrics @@ -127,25 +134,6 @@ func (b *DiskBuffer) Reject(batch []telegraf.Metric) { } } -func (b *DiskBuffer) metricToBytes(metric telegraf.Metric) []byte { - var buf bytes.Buffer - encoder := gob.NewEncoder(&buf) - if err := encoder.Encode(metric); err != nil { - // todo error handle - fmt.Println("Error encoding:", err) - panic(1) - } - return buf.Bytes() -} - -func (b *DiskBuffer) bytesToMetric(data []byte) telegraf.Metric { - buf := bytes.NewBuffer(data) - decoder := gob.NewDecoder(buf) - var m telegraf.Metric - if err := decoder.Decode(&m); err != nil { - // todo error handle - fmt.Println("Error decoding:", err) - panic(1) - } - return m +func (b *DiskBuffer) Stats() BufferStats { + return b.BufferStats } diff --git a/models/buffer_disk_test.go b/models/buffer_disk_test.go deleted file mode 100644 index 56f7cbf7eb351..0000000000000 --- a/models/buffer_disk_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package models - -import ( - "os" - "testing" - - "github.com/influxdata/telegraf" - "github.com/influxdata/telegraf/testutil" -) - -func newTestDiskBuffer(name string, alias string, capacity int) (*DiskBuffer, string) { - bm := NewBufferMetrics(name, alias, capacity) - path, _ := os.MkdirTemp("", "*-disk_buffer-"+name) - return NewDiskBuffer(name, capacity, path, bm), path -} - -func setupDisk(b *DiskBuffer) *DiskBuffer { - b.MetricsAdded.Set(0) - b.MetricsWritten.Set(0) - b.MetricsDropped.Set(0) - return b -} - -func TestDiskBuffer(t *testing.T) { - db, path := newTestDiskBuffer("test", "", 5000) - defer os.RemoveAll(path) - db = setupDisk(db) - - db.Add(MetricTime(1)) - db.Add(MetricTime(2)) - db.Add(MetricTime(3)) - batch := db.Batch(2) - testutil.RequireMetricsEqual(t, - []telegraf.Metric{ - MetricTime(1), - MetricTime(2), - }, batch) -} diff --git a/models/buffer_mem.go b/models/buffer_mem.go index a58cef61b9474..667759ebbd523 100644 --- a/models/buffer_mem.go +++ b/models/buffer_mem.go @@ -21,15 +21,12 @@ type MemoryBuffer struct { batchSize int // number of metrics currently in the batch } -func NewMemoryBuffer(capacity int, stats BufferStats) *MemoryBuffer { +func NewMemoryBuffer(capacity int, stats BufferStats) (*MemoryBuffer, error) { return &MemoryBuffer{ BufferStats: stats, buf: make([]telegraf.Metric, capacity), - first: 0, - last: 0, - size: 0, cap: capacity, - } + }, nil } func (b *MemoryBuffer) Len() int { @@ -151,6 +148,10 @@ func (b *MemoryBuffer) Reject(batch []telegraf.Metric) { b.BufferSize.Set(int64(b.length())) } +func (b *MemoryBuffer) Stats() BufferStats { + return b.BufferStats +} + // next returns the next index with wrapping. func (b *MemoryBuffer) next(index int) int { index++ diff --git a/models/buffer_mem_test.go b/models/buffer_suite_test.go similarity index 50% rename from models/buffer_mem_test.go rename to models/buffer_suite_test.go index d434e87ee3490..25c0e48522b1a 100644 --- a/models/buffer_mem_test.go +++ b/models/buffer_suite_test.go @@ -1,33 +1,45 @@ package models import ( + "os" "testing" "time" - "github.com/stretchr/testify/require" - "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/metric" + models "github.com/influxdata/telegraf/models/mock" "github.com/influxdata/telegraf/testutil" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" ) -type MockMetric struct { - telegraf.Metric - AcceptF func() - RejectF func() - DropF func() +type BufferSuiteTest struct { + suite.Suite + bufferType string + bufferPath string } -func (m *MockMetric) Accept() { - m.AcceptF() +func (suite *BufferSuiteTest) SetupTest() { + if suite.bufferType == "disk" { + path, err := os.MkdirTemp("", "*-buffer-test") + suite.NoError(err) + suite.bufferPath = path + } } -func (m *MockMetric) Reject() { - m.RejectF() +func (suite *BufferSuiteTest) TearDownTest() { + if suite.bufferPath != "" { + _ = os.RemoveAll(suite.bufferPath) + suite.bufferPath = "" + } } -func (m *MockMetric) Drop() { - m.DropF() +func TestMemoryBufferSuite(t *testing.T) { + suite.Run(t, &BufferSuiteTest{bufferType: "memory"}) +} + +func TestDiskBufferSuite(t *testing.T) { + suite.Run(t, &BufferSuiteTest{bufferType: "disk"}) } func Metric() telegraf.Metric { @@ -46,131 +58,120 @@ func MetricTime(sec int64) telegraf.Metric { return m } -func BenchmarkAddMetrics(b *testing.B) { - buf := newTestMemoryBuffer("test", "", 10000) - m := Metric() - for n := 0; n < b.N; n++ { - buf.Add(m) - } +func (suite *BufferSuiteTest) newTestBuffer(capacity int) Buffer { + suite.T().Helper() + buf, err := NewBuffer("test", "", capacity, suite.bufferType, suite.bufferPath) + require.NoError(suite.T(), err) + buf.Stats().MetricsAdded.Set(0) + buf.Stats().MetricsWritten.Set(0) + buf.Stats().MetricsDropped.Set(0) + return buf } -func newTestMemoryBuffer(name string, alias string, capacity int) *MemoryBuffer { - bm := NewBufferMetrics(name, alias, capacity) - return NewMemoryBuffer(capacity, bm) -} +func (suite *BufferSuiteTest) TestBuffer_LenEmpty() { + b := suite.newTestBuffer(5) -func setup(b *MemoryBuffer) *MemoryBuffer { - b.MetricsAdded.Set(0) - b.MetricsWritten.Set(0) - b.MetricsDropped.Set(0) - return b + suite.Equal(0, b.Len()) } -func TestBuffer_LenEmpty(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) - - require.Equal(t, 0, b.Len()) -} - -func TestBuffer_LenOne(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_LenOne() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m) - require.Equal(t, 1, b.Len()) + suite.Equal(1, b.Len()) } -func TestBuffer_LenFull(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_LenFull() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) - require.Equal(t, 5, b.Len()) + suite.Equal(5, b.Len()) } -func TestBuffer_LenOverfill(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_LenOverfill() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) - setup(b) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m, m) - require.Equal(t, 5, b.Len()) + suite.Equal(5, b.Len()) } -func TestBuffer_BatchLenZero(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_BatchLenZero() { + b := suite.newTestBuffer(5) batch := b.Batch(0) - require.Empty(t, batch) + suite.Empty(batch) } -func TestBuffer_BatchLenBufferEmpty(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_BatchLenBufferEmpty() { + b := suite.newTestBuffer(5) batch := b.Batch(2) - require.Empty(t, batch) + suite.Empty(batch) } -func TestBuffer_BatchLenUnderfill(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchLenUnderfill() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m) batch := b.Batch(2) - require.Len(t, batch, 1) + suite.Len(batch, 1) } -func TestBuffer_BatchLenFill(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchLenFill() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m) batch := b.Batch(2) - require.Len(t, batch, 2) + suite.Len(batch, 2) } -func TestBuffer_BatchLenExact(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchLenExact() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m) batch := b.Batch(2) - require.Len(t, batch, 2) + suite.Len(batch, 2) } -func TestBuffer_BatchLenLargerThanBuffer(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchLenLargerThanBuffer() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(6) - require.Len(t, batch, 5) + suite.Len(batch, 5) } -func TestBuffer_BatchWrap(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchWrap() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(2) b.Accept(batch) b.Add(m, m) batch = b.Batch(5) - require.Len(t, batch, 5) + suite.Len(batch, 5) } -func TestBuffer_BatchLatest(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 4)) +func (suite *BufferSuiteTest) TestBuffer_BatchLatest() { + b := suite.newTestBuffer(4) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) batch := b.Batch(2) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), }, batch) } -func TestBuffer_BatchLatestWrap(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 4)) +func (suite *BufferSuiteTest) TestBuffer_BatchLatestWrap() { + b := suite.newTestBuffer(4) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -178,15 +179,15 @@ func TestBuffer_BatchLatestWrap(t *testing.T) { b.Add(MetricTime(5)) batch := b.Batch(2) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(2), MetricTime(3), }, batch) } -func TestBuffer_MultipleBatch(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 10)) +func (suite *BufferSuiteTest) TestBuffer_MultipleBatch() { + b := suite.newTestBuffer(10) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -194,7 +195,7 @@ func TestBuffer_MultipleBatch(t *testing.T) { b.Add(MetricTime(5)) b.Add(MetricTime(6)) batch := b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -204,15 +205,15 @@ func TestBuffer_MultipleBatch(t *testing.T) { }, batch) b.Accept(batch) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(6), }, batch) b.Accept(batch) } -func TestBuffer_RejectWithRoom(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectWithRoom() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -221,10 +222,10 @@ func TestBuffer_RejectWithRoom(t *testing.T) { b.Add(MetricTime(5)) b.Reject(batch) - require.Equal(t, int64(0), b.MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -234,8 +235,8 @@ func TestBuffer_RejectWithRoom(t *testing.T) { }, batch) } -func TestBuffer_RejectNothingNewFull(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectNothingNewFull() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -244,10 +245,10 @@ func TestBuffer_RejectNothingNewFull(t *testing.T) { batch := b.Batch(2) b.Reject(batch) - require.Equal(t, int64(0), b.MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -257,8 +258,8 @@ func TestBuffer_RejectNothingNewFull(t *testing.T) { }, batch) } -func TestBuffer_RejectNoRoom(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectNoRoom() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) @@ -273,10 +274,10 @@ func TestBuffer_RejectNoRoom(t *testing.T) { b.Reject(batch) - require.Equal(t, int64(3), b.MetricsDropped.Get()) + suite.Equal(int64(3), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(4), MetricTime(5), @@ -286,8 +287,8 @@ func TestBuffer_RejectNoRoom(t *testing.T) { }, batch) } -func TestBuffer_RejectRoomExact(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectRoomExact() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) batch := b.Batch(2) @@ -297,10 +298,10 @@ func TestBuffer_RejectRoomExact(t *testing.T) { b.Reject(batch) - require.Equal(t, int64(0), b.MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -310,8 +311,8 @@ func TestBuffer_RejectRoomExact(t *testing.T) { }, batch) } -func TestBuffer_RejectRoomOverwriteOld(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectRoomOverwriteOld() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -322,10 +323,10 @@ func TestBuffer_RejectRoomOverwriteOld(t *testing.T) { b.Reject(batch) - require.Equal(t, int64(1), b.MetricsDropped.Get()) + suite.Equal(int64(1), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(2), MetricTime(3), @@ -335,8 +336,8 @@ func TestBuffer_RejectRoomOverwriteOld(t *testing.T) { }, batch) } -func TestBuffer_RejectPartialRoom(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectPartialRoom() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) @@ -349,10 +350,10 @@ func TestBuffer_RejectPartialRoom(t *testing.T) { b.Add(MetricTime(7)) b.Reject(batch) - require.Equal(t, int64(2), b.MetricsDropped.Get()) + suite.Equal(int64(2), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(3), MetricTime(4), @@ -362,8 +363,8 @@ func TestBuffer_RejectPartialRoom(t *testing.T) { }, batch) } -func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -372,7 +373,7 @@ func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { b.Add(MetricTime(5)) // buffer: 1, 4, 5; batch: 2, 3 - require.Equal(t, int64(0), b.MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) b.Add(MetricTime(6)) b.Add(MetricTime(7)) @@ -381,7 +382,7 @@ func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { b.Add(MetricTime(10)) // buffer: 8, 9, 10, 6, 7; batch: 2, 3 - require.Equal(t, int64(3), b.MetricsDropped.Get()) + suite.Equal(int64(3), b.Stats().MetricsDropped.Get()) b.Add(MetricTime(11)) b.Add(MetricTime(12)) @@ -389,13 +390,13 @@ func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { b.Add(MetricTime(14)) b.Add(MetricTime(15)) // buffer: 13, 14, 15, 11, 12; batch: 2, 3 - require.Equal(t, int64(8), b.MetricsDropped.Get()) + suite.Equal(int64(8), b.Stats().MetricsDropped.Get()) b.Reject(batch) - require.Equal(t, int64(10), b.MetricsDropped.Get()) + suite.Equal(int64(10), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(11), MetricTime(12), @@ -405,8 +406,8 @@ func TestBuffer_RejectNewMetricsWrapped(t *testing.T) { }, batch) } -func TestBuffer_RejectWrapped(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectWrapped() { + b := suite.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -426,7 +427,7 @@ func TestBuffer_RejectWrapped(t *testing.T) { b.Reject(batch) batch = b.Batch(5) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(8), MetricTime(9), @@ -436,8 +437,8 @@ func TestBuffer_RejectWrapped(t *testing.T) { }, batch) } -func TestBuffer_RejectAdjustFirst(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 10)) +func (suite *BufferSuiteTest) TestBuffer_RejectAdjustFirst() { + b := suite.newTestBuffer(10) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -468,7 +469,7 @@ func TestBuffer_RejectAdjustFirst(t *testing.T) { b.Add(MetricTime(19)) batch = b.Batch(10) - testutil.RequireMetricsEqual(t, + testutil.RequireMetricsEqual(suite.T(), []telegraf.Metric{ MetricTime(10), MetricTime(11), @@ -483,215 +484,213 @@ func TestBuffer_RejectAdjustFirst(t *testing.T) { }, batch) } -func TestBuffer_AddDropsOverwrittenMetrics(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AddDropsOverwrittenMetrics() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) b.Add(m, m, m, m, m) - require.Equal(t, int64(5), b.MetricsDropped.Get()) - require.Equal(t, int64(0), b.MetricsWritten.Get()) + suite.Equal(int64(5), b.Stats().MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsWritten.Get()) } -func TestBuffer_AcceptRemovesBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AcceptRemovesBatch() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m) batch := b.Batch(2) b.Accept(batch) - require.Equal(t, 1, b.Len()) + suite.Equal(1, b.Len()) } -func TestBuffer_RejectLeavesBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_RejectLeavesBatch() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m) batch := b.Batch(2) b.Reject(batch) - require.Equal(t, 3, b.Len()) + suite.Equal(3, b.Len()) } -func TestBuffer_AcceptWritesOverwrittenBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AcceptWritesOverwrittenBatch() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(5) b.Add(m, m, m, m, m) b.Accept(batch) - require.Equal(t, int64(0), b.MetricsDropped.Get()) - require.Equal(t, int64(5), b.MetricsWritten.Get()) + suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) + suite.Equal(int64(5), b.Stats().MetricsWritten.Get()) } -func TestBuffer_BatchRejectDropsOverwrittenBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchRejectDropsOverwrittenBatch() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(5) b.Add(m, m, m, m, m) b.Reject(batch) - require.Equal(t, int64(5), b.MetricsDropped.Get()) - require.Equal(t, int64(0), b.MetricsWritten.Get()) + suite.Equal(int64(5), b.Stats().MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsWritten.Get()) } -func TestBuffer_MetricsOverwriteBatchAccept(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_MetricsOverwriteBatchAccept() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(3) b.Add(m, m, m) b.Accept(batch) - require.Equal(t, int64(0), b.MetricsDropped.Get(), "dropped") - require.Equal(t, int64(3), b.MetricsWritten.Get(), "written") + suite.Equal(int64(0), b.Stats().MetricsDropped.Get(), "dropped") + suite.Equal(int64(3), b.Stats().MetricsWritten.Get(), "written") } -func TestBuffer_MetricsOverwriteBatchReject(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_MetricsOverwriteBatchReject() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(3) b.Add(m, m, m) b.Reject(batch) - require.Equal(t, int64(3), b.MetricsDropped.Get()) - require.Equal(t, int64(0), b.MetricsWritten.Get()) + suite.Equal(int64(3), b.Stats().MetricsDropped.Get()) + suite.Equal(int64(0), b.Stats().MetricsWritten.Get()) } -func TestBuffer_MetricsBatchAcceptRemoved(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_MetricsBatchAcceptRemoved() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(3) b.Add(m, m, m, m, m) b.Accept(batch) - require.Equal(t, int64(2), b.MetricsDropped.Get()) - require.Equal(t, int64(3), b.MetricsWritten.Get()) + suite.Equal(int64(2), b.Stats().MetricsDropped.Get()) + suite.Equal(int64(3), b.Stats().MetricsWritten.Get()) } -func TestBuffer_WrapWithBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_WrapWithBatch() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m) b.Batch(3) b.Add(m, m, m, m, m, m) - require.Equal(t, int64(1), b.MetricsDropped.Get()) + suite.Equal(int64(1), b.Stats().MetricsDropped.Get()) } -func TestBuffer_BatchNotRemoved(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchNotRemoved() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) b.Batch(2) - require.Equal(t, 5, b.Len()) + suite.Equal(5, b.Len()) } -func TestBuffer_BatchRejectAcceptNoop(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_BatchRejectAcceptNoop() { m := Metric() - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(2) b.Reject(batch) b.Accept(batch) - require.Equal(t, 5, b.Len()) + suite.Equal(5, b.Len()) } -func TestBuffer_AcceptCallsMetricAccept(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AcceptCallsMetricAccept() { var accept int - mm := &MockMetric{ + mm := &models.MockMetric{ Metric: Metric(), AcceptF: func() { accept++ }, } - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(mm, mm, mm) batch := b.Batch(2) b.Accept(batch) - require.Equal(t, 2, accept) + suite.Equal(2, accept) } -func TestBuffer_AddCallsMetricRejectWhenNoBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNoBatch() { var reject int - mm := &MockMetric{ + mm := &models.MockMetric{ Metric: Metric(), RejectF: func() { reject++ }, } - b := setup(newTestMemoryBuffer("test", "", 5)) - setup(b) + b := suite.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm) - require.Equal(t, 2, reject) + suite.Equal(2, reject) } -func TestBuffer_AddCallsMetricRejectWhenNotInBatch(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNotInBatch() { var reject int - mm := &MockMetric{ + mm := &models.MockMetric{ Metric: Metric(), RejectF: func() { reject++ }, } - b := setup(newTestMemoryBuffer("test", "", 5)) - setup(b) + b := suite.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(2) b.Add(mm, mm, mm, mm) - require.Equal(t, 2, reject) + suite.Equal(2, reject) b.Reject(batch) - require.Equal(t, 4, reject) + suite.Equal(4, reject) } -func TestBuffer_RejectCallsMetricRejectWithOverwritten(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_RejectCallsMetricRejectWithOverwritten() { var reject int - mm := &MockMetric{ + mm := &models.MockMetric{ Metric: Metric(), RejectF: func() { reject++ }, } - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(5) b.Add(mm, mm) - require.Equal(t, 0, reject) + suite.Equal(0, reject) b.Reject(batch) - require.Equal(t, 2, reject) + suite.Equal(2, reject) } -func TestBuffer_AddOverwriteAndReject(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AddOverwriteAndReject() { var reject int - mm := &MockMetric{ + mm := &models.MockMetric{ Metric: Metric(), RejectF: func() { reject++ }, } - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(5) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm, mm, mm, mm) - require.Equal(t, 15, reject) + suite.Equal(15, reject) b.Reject(batch) - require.Equal(t, 20, reject) + suite.Equal(20, reject) } -func TestBuffer_AddOverwriteAndRejectOffset(t *testing.T) { +func (suite *BufferSuiteTest) TestBuffer_AddOverwriteAndRejectOffset() { var reject int var accept int - mm := &MockMetric{ + mm := &models.MockMetric{ Metric: Metric(), RejectF: func() { reject++ @@ -700,32 +699,42 @@ func TestBuffer_AddOverwriteAndRejectOffset(t *testing.T) { accept++ }, } - b := setup(newTestMemoryBuffer("test", "", 5)) + b := suite.newTestBuffer(5) b.Add(mm, mm, mm) b.Add(mm, mm, mm, mm) - require.Equal(t, 2, reject) + suite.Equal(2, reject) batch := b.Batch(5) b.Add(mm, mm, mm, mm) - require.Equal(t, 2, reject) + suite.Equal(2, reject) b.Add(mm, mm, mm, mm) - require.Equal(t, 5, reject) + suite.Equal(5, reject) b.Add(mm, mm, mm, mm) - require.Equal(t, 9, reject) + suite.Equal(9, reject) b.Add(mm, mm, mm, mm) - require.Equal(t, 13, reject) + suite.Equal(13, reject) b.Accept(batch) - require.Equal(t, 13, reject) - require.Equal(t, 5, accept) + suite.Equal(13, reject) + suite.Equal(5, accept) } -func TestBuffer_RejectEmptyBatch(t *testing.T) { - b := setup(newTestMemoryBuffer("test", "", 5)) +func (suite *BufferSuiteTest) TestBuffer_RejectEmptyBatch() { + b := suite.newTestBuffer(5) batch := b.Batch(2) b.Add(MetricTime(1)) b.Reject(batch) b.Add(MetricTime(2)) batch = b.Batch(2) for _, m := range batch { - require.NotNil(t, m) + suite.NotNil(m) + } +} + +// Benchmark test outside the suite +func BenchmarkMemoryAddMetrics(b *testing.B) { + buf, err := NewBuffer("test", "", 10000, "memory", "") + require.NoError(b, err) + m := Metric() + for n := 0; n < b.N; n++ { + buf.Add(m) } } diff --git a/models/mock/metric.go b/models/mock/metric.go new file mode 100644 index 0000000000000..a4de92ac5704e --- /dev/null +++ b/models/mock/metric.go @@ -0,0 +1,22 @@ +package models + +import "github.com/influxdata/telegraf" + +type MockMetric struct { + telegraf.Metric + AcceptF func() + RejectF func() + DropF func() +} + +func (m *MockMetric) Accept() { + m.AcceptF() +} + +func (m *MockMetric) Reject() { + m.RejectF() +} + +func (m *MockMetric) Drop() { + m.DropF() +} diff --git a/models/running_output.go b/models/running_output.go index 82ab88b1ceb5d..17f4e9de0c9d9 100644 --- a/models/running_output.go +++ b/models/running_output.go @@ -99,8 +99,12 @@ func NewRunningOutput( batchSize = DefaultMetricBatchSize } + b, err := NewBuffer(config.Name, config.Alias, bufferLimit, config.BufferStrategy, config.BufferDirectory) + if err != nil { + panic(err) // todo be more graceful here? + } ro := &RunningOutput{ - buffer: NewBuffer(config.Name, config.Alias, bufferLimit, config.BufferStrategy, config.BufferDirectory), + buffer: b, BatchReady: make(chan time.Time, 1), Output: output, Config: config, From f6d1548f8f760b299fa0e624a09361c1bc88eada Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Mon, 13 May 2024 12:05:32 -0500 Subject: [PATCH 05/10] remove shadowed builtin min function --- models/buffer_mem.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/models/buffer_mem.go b/models/buffer_mem.go index 667759ebbd523..55eb1c321db36 100644 --- a/models/buffer_mem.go +++ b/models/buffer_mem.go @@ -183,10 +183,3 @@ func (b *MemoryBuffer) resetBatch() { b.batchFirst = 0 b.batchSize = 0 } - -func min(a, b int) int { - if b < a { - return b - } - return a -} From 45fd10123e14cce6a0e5d15773bd0d4cfc447de0 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Tue, 4 Jun 2024 13:12:39 -0500 Subject: [PATCH 06/10] fix many issues and get all tests passing --- metric.go | 3 - metric/metric.go | 7 +- models/buffer.go | 2 +- models/buffer_disk.go | 122 ++++++++--- models/buffer_disk_test.go | 37 ++++ models/buffer_mem_test.go | 58 +++++ models/buffer_suite_test.go | 414 +++++++++++++++++++----------------- 7 files changed, 409 insertions(+), 234 deletions(-) create mode 100644 models/buffer_disk_test.go create mode 100644 models/buffer_mem_test.go diff --git a/metric.go b/metric.go index 971a291bb947b..93d46858b0db6 100644 --- a/metric.go +++ b/metric.go @@ -128,9 +128,6 @@ type Metric interface { // ToBytes converts the metric a byte array using the gob encoder. ToBytes() ([]byte, error) - - // FromBytes populates a metrics data using a binary byte array. - FromBytes([]byte) error } // TemplateMetric is an interface to use in templates (e.g text/template) diff --git a/metric/metric.go b/metric/metric.go index a5d7e4565adb1..a087a7a98d85b 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -396,11 +396,12 @@ func (m *metric) ToBytes() ([]byte, error) { return buf.Bytes(), nil } -func (m *metric) FromBytes(b []byte) error { +func FromBytes(b []byte) (telegraf.Metric, error) { buf := bytes.NewBuffer(b) decoder := gob.NewDecoder(buf) + var m *metric if err := decoder.Decode(&m); err != nil { - return fmt.Errorf("failed to decode metric from bytes: %w", err) + return nil, fmt.Errorf("failed to decode metric from bytes: %w", err) } - return nil + return m, nil } diff --git a/models/buffer.go b/models/buffer.go index 13e053f2e8dbc..9684ce4440a17 100644 --- a/models/buffer.go +++ b/models/buffer.go @@ -51,7 +51,7 @@ func NewBuffer(name string, alias string, capacity int, strategy string, path st case "", "memory": return NewMemoryBuffer(capacity, bm) case "disk": - return NewDiskBuffer(name, capacity, path, bm) + return NewDiskBuffer(name, path, bm) case "overflow": // todo implementme // todo log currently unimplemented diff --git a/models/buffer_disk.go b/models/buffer_disk.go index bb12348199f1c..ee5aae59bd915 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -2,28 +2,49 @@ package models import ( "fmt" + "os" + "sync" + "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/metric" "github.com/tidwall/wal" ) type DiskBuffer struct { BufferStats - walFile *wal.Log + sync.Mutex + + walFile *wal.Log + walFilePath string + + batchFirst uint64 // index of the first metric in the batch + batchSize uint64 // number of metrics currently in the batch } -func NewDiskBuffer(name string, capacity int, path string, stats BufferStats) (*DiskBuffer, error) { - // todo capacity - walFile, err := wal.Open(path+"/"+name, nil) +func NewDiskBuffer(name string, path string, stats BufferStats) (*DiskBuffer, error) { + filePath := path + "/" + name + walFile, err := wal.Open(filePath, nil) if err != nil { return nil, fmt.Errorf("failed to open wal file: %w", err) } return &DiskBuffer{ BufferStats: stats, walFile: walFile, + walFilePath: filePath, }, nil } func (b *DiskBuffer) Len() int { + b.Lock() + defer b.Unlock() + return b.length() +} + +func (b *DiskBuffer) length() int { + // Special case for when the read index is zero, it must be empty (otherwise it would be >= 1) + if b.readIndex() == 0 { + return 0 + } return int(b.writeIndex() - b.readIndex()) } @@ -42,20 +63,22 @@ func (b *DiskBuffer) writeIndex() uint64 { if err != nil { panic(err) // can only occur with a corrupt wal file } - return index + return index + 1 } func (b *DiskBuffer) Add(metrics ...telegraf.Metric) int { - // one metric to write, can write directly - if len(metrics) == 1 { - if b.addSingle(metrics[0]) { - return 1 + b.Lock() + defer b.Unlock() + + dropped := 0 + for _, m := range metrics { + if !b.addSingle(m) { + dropped++ } - return 0 } - - // multiple metrics to write, batch them - return b.addBatch(metrics) + b.BufferSize.Set(int64(b.length())) + return dropped + // todo implement batched writes } func (b *DiskBuffer) addSingle(m telegraf.Metric) bool { @@ -72,6 +95,7 @@ func (b *DiskBuffer) addSingle(m telegraf.Metric) bool { return false } +//nolint:unused // to be implemented in the future func (b *DiskBuffer) addBatch(metrics []telegraf.Metric) int { written := 0 batch := new(wal.Batch) @@ -93,47 +117,77 @@ func (b *DiskBuffer) addBatch(metrics []telegraf.Metric) int { } func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { - if b.Len() == 0 { + b.Lock() + defer b.Unlock() + + if b.length() == 0 { // no metrics in the wal file, so return an empty array return make([]telegraf.Metric, 0) } - metrics := make([]telegraf.Metric, batchSize) - index := 0 - for i := b.readIndex(); i < b.readIndex()+uint64(batchSize); i++ { - data, err := b.walFile.Read(i) + b.batchSize = uint64(min(b.length(), batchSize)) + b.batchFirst = b.readIndex() + metrics := make([]telegraf.Metric, b.batchSize) + + for i := 0; i < int(b.batchSize); i++ { + data, err := b.walFile.Read(b.batchFirst + uint64(i)) if err != nil { - // todo error handle + panic(err) } - var m telegraf.Metric - if err = m.FromBytes(data); err != nil { + m, err := metric.FromBytes(data) + if err != nil { panic(err) } - metrics[index] = m - index++ + metrics[i] = m } return metrics } func (b *DiskBuffer) Accept(batch []telegraf.Metric) { + b.Lock() + defer b.Unlock() + + if b.batchSize == 0 || len(batch) == 0 { + // nothing to accept + return + } for _, m := range batch { b.metricWritten(m) } - err := b.walFile.TruncateFront(b.readIndex() + uint64(len(batch))) - if err != nil { - panic(err) // can only occur with a corrupt wal file + if b.length() == len(batch) { + b.resetWalFile() + } else { + err := b.walFile.TruncateFront(b.batchFirst + uint64(len(batch))) + if err != nil { + panic(err) + } } + b.resetBatch() + b.BufferSize.Set(int64(b.length())) } -func (b *DiskBuffer) Reject(batch []telegraf.Metric) { - for _, m := range batch { - b.metricDropped(m) - } - err := b.walFile.TruncateFront(b.readIndex() + uint64(len(batch))) - if err != nil { - panic(err) // can only occur with a corrupt wal file - } +func (b *DiskBuffer) Reject(_ []telegraf.Metric) { + // very little to do here as the disk buffer retains metrics in + // the wal file until a call to accept + b.Lock() + defer b.Unlock() + b.resetBatch() } func (b *DiskBuffer) Stats() BufferStats { return b.BufferStats } + +func (b *DiskBuffer) resetBatch() { + b.batchFirst = 0 + b.batchSize = 0 +} + +// todo This is very messy and not ideal, but serves as the only way I can find currently +// todo to actually clear the walfile completely if needed, since Truncate() calls require +// todo at least one entry remains in them otherwise they return an error. +func (b *DiskBuffer) resetWalFile() { + b.walFile.Close() + os.Remove(b.walFilePath) + walFile, _ := wal.Open(b.walFilePath, nil) + b.walFile = walFile +} diff --git a/models/buffer_disk_test.go b/models/buffer_disk_test.go new file mode 100644 index 0000000000000..e9f675b1c0403 --- /dev/null +++ b/models/buffer_disk_test.go @@ -0,0 +1,37 @@ +package models + +import ( + "os" + "testing" + + models "github.com/influxdata/telegraf/models/mock" + "github.com/stretchr/testify/require" +) + +func newTestDiskBuffer(t testing.TB) Buffer { + t.Helper() + path, err := os.MkdirTemp("", "*-buffer-test") + require.NoError(t, err) + buf, err := NewBuffer("test", "", 0, "disk", path) + require.NoError(t, err) + buf.Stats().MetricsAdded.Set(0) + buf.Stats().MetricsWritten.Set(0) + buf.Stats().MetricsDropped.Set(0) + return buf +} + +func TestBuffer_AddCallsMetricAccept(t *testing.T) { + var accept int + mm := &models.MockMetric{ + Metric: Metric(), + AcceptF: func() { + accept++ + }, + } + b := newTestDiskBuffer(t) + b.Add(mm, mm, mm) + batch := b.Batch(2) + b.Accept(batch) + // all 3 metrics should be accepted as metric Accept() is called on buffer Add() + require.Equal(t, 3, accept) +} diff --git a/models/buffer_mem_test.go b/models/buffer_mem_test.go new file mode 100644 index 0000000000000..2c470685f8da5 --- /dev/null +++ b/models/buffer_mem_test.go @@ -0,0 +1,58 @@ +package models + +import ( + "testing" + + models "github.com/influxdata/telegraf/models/mock" + "github.com/stretchr/testify/require" +) + +func newTestMemoryBuffer(t testing.TB, capacity int) Buffer { + t.Helper() + buf, err := NewBuffer("test", "", capacity, "memory", "") + require.NoError(t, err) + buf.Stats().MetricsAdded.Set(0) + buf.Stats().MetricsWritten.Set(0) + buf.Stats().MetricsDropped.Set(0) + return buf +} + +func TestBuffer_AcceptCallsMetricAccept(t *testing.T) { + var accept int + mm := &models.MockMetric{ + Metric: Metric(), + AcceptF: func() { + accept++ + }, + } + b := newTestMemoryBuffer(t, 5) + b.Add(mm, mm, mm) + batch := b.Batch(2) + b.Accept(batch) + require.Equal(t, 2, accept) +} + +func TestBuffer_RejectCallsMetricRejectWithOverwritten(t *testing.T) { + var reject int + mm := &models.MockMetric{ + Metric: Metric(), + RejectF: func() { + reject++ + }, + } + b := newTestMemoryBuffer(t, 5) + b.Add(mm, mm, mm, mm, mm) + batch := b.Batch(5) + b.Add(mm, mm) + require.Equal(t, 0, reject) + b.Reject(batch) + require.Equal(t, 2, reject) +} + +func BenchmarkMemoryAddMetrics(b *testing.B) { + buf := newTestMemoryBuffer(b, 10000) + m := Metric() + for n := 0; n < b.N; n++ { + buf.Add(m) + } +} diff --git a/models/buffer_suite_test.go b/models/buffer_suite_test.go index 25c0e48522b1a..06c31b35636ec 100644 --- a/models/buffer_suite_test.go +++ b/models/buffer_suite_test.go @@ -9,7 +9,6 @@ import ( "github.com/influxdata/telegraf/metric" models "github.com/influxdata/telegraf/models/mock" "github.com/influxdata/telegraf/testutil" - "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -17,20 +16,23 @@ type BufferSuiteTest struct { suite.Suite bufferType string bufferPath string + + hasMaxCapacity bool // whether the buffer type being tested supports a maximum metric capacity } -func (suite *BufferSuiteTest) SetupTest() { - if suite.bufferType == "disk" { +func (s *BufferSuiteTest) SetupTest() { + if s.bufferType == "disk" { path, err := os.MkdirTemp("", "*-buffer-test") - suite.NoError(err) - suite.bufferPath = path + s.NoError(err) + s.bufferPath = path + s.hasMaxCapacity = false } } -func (suite *BufferSuiteTest) TearDownTest() { - if suite.bufferPath != "" { - _ = os.RemoveAll(suite.bufferPath) - suite.bufferPath = "" +func (s *BufferSuiteTest) TearDownTest() { + if s.bufferPath != "" { + _ = os.RemoveAll(s.bufferPath) + s.bufferPath = "" } } @@ -58,120 +60,128 @@ func MetricTime(sec int64) telegraf.Metric { return m } -func (suite *BufferSuiteTest) newTestBuffer(capacity int) Buffer { - suite.T().Helper() - buf, err := NewBuffer("test", "", capacity, suite.bufferType, suite.bufferPath) - require.NoError(suite.T(), err) +func (s *BufferSuiteTest) newTestBuffer(capacity int) Buffer { + s.T().Helper() + buf, err := NewBuffer("test", "", capacity, s.bufferType, s.bufferPath) + s.Require().NoError(err) buf.Stats().MetricsAdded.Set(0) buf.Stats().MetricsWritten.Set(0) buf.Stats().MetricsDropped.Set(0) return buf } -func (suite *BufferSuiteTest) TestBuffer_LenEmpty() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_LenEmpty() { + b := s.newTestBuffer(5) - suite.Equal(0, b.Len()) + s.Equal(0, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_LenOne() { +func (s *BufferSuiteTest) TestBuffer_LenOne() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m) - suite.Equal(1, b.Len()) + s.Equal(1, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_LenFull() { +func (s *BufferSuiteTest) TestBuffer_LenFull() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) - suite.Equal(5, b.Len()) + s.Equal(5, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_LenOverfill() { +func (s *BufferSuiteTest) TestBuffer_LenOverfill() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m, m) - suite.Equal(5, b.Len()) + s.Equal(5, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_BatchLenZero() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_BatchLenZero() { + b := s.newTestBuffer(5) batch := b.Batch(0) - suite.Empty(batch) + s.Empty(batch) } -func (suite *BufferSuiteTest) TestBuffer_BatchLenBufferEmpty() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_BatchLenBufferEmpty() { + b := s.newTestBuffer(5) batch := b.Batch(2) - suite.Empty(batch) + s.Empty(batch) } -func (suite *BufferSuiteTest) TestBuffer_BatchLenUnderfill() { +func (s *BufferSuiteTest) TestBuffer_BatchLenUnderfill() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m) batch := b.Batch(2) - suite.Len(batch, 1) + s.Len(batch, 1) } -func (suite *BufferSuiteTest) TestBuffer_BatchLenFill() { +func (s *BufferSuiteTest) TestBuffer_BatchLenFill() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m) batch := b.Batch(2) - suite.Len(batch, 2) + s.Len(batch, 2) } -func (suite *BufferSuiteTest) TestBuffer_BatchLenExact() { +func (s *BufferSuiteTest) TestBuffer_BatchLenExact() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m) batch := b.Batch(2) - suite.Len(batch, 2) + s.Len(batch, 2) } -func (suite *BufferSuiteTest) TestBuffer_BatchLenLargerThanBuffer() { +func (s *BufferSuiteTest) TestBuffer_BatchLenLargerThanBuffer() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(6) - suite.Len(batch, 5) + s.Len(batch, 5) } -func (suite *BufferSuiteTest) TestBuffer_BatchWrap() { +func (s *BufferSuiteTest) TestBuffer_BatchWrap() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(2) b.Accept(batch) b.Add(m, m) batch = b.Batch(5) - suite.Len(batch, 5) + s.Len(batch, 5) } -func (suite *BufferSuiteTest) TestBuffer_BatchLatest() { - b := suite.newTestBuffer(4) +func (s *BufferSuiteTest) TestBuffer_BatchLatest() { + b := s.newTestBuffer(4) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) batch := b.Batch(2) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), }, batch) } -func (suite *BufferSuiteTest) TestBuffer_BatchLatestWrap() { - b := suite.newTestBuffer(4) +func (s *BufferSuiteTest) TestBuffer_BatchLatestWrap() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(4) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -179,15 +189,15 @@ func (suite *BufferSuiteTest) TestBuffer_BatchLatestWrap() { b.Add(MetricTime(5)) batch := b.Batch(2) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(2), MetricTime(3), }, batch) } -func (suite *BufferSuiteTest) TestBuffer_MultipleBatch() { - b := suite.newTestBuffer(10) +func (s *BufferSuiteTest) TestBuffer_MultipleBatch() { + b := s.newTestBuffer(10) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -195,7 +205,7 @@ func (suite *BufferSuiteTest) TestBuffer_MultipleBatch() { b.Add(MetricTime(5)) b.Add(MetricTime(6)) batch := b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -205,15 +215,15 @@ func (suite *BufferSuiteTest) TestBuffer_MultipleBatch() { }, batch) b.Accept(batch) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(6), }, batch) b.Accept(batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectWithRoom() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectWithRoom() { + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -222,10 +232,10 @@ func (suite *BufferSuiteTest) TestBuffer_RejectWithRoom() { b.Add(MetricTime(5)) b.Reject(batch) - suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -235,8 +245,8 @@ func (suite *BufferSuiteTest) TestBuffer_RejectWithRoom() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectNothingNewFull() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectNothingNewFull() { + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -245,10 +255,10 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNothingNewFull() { batch := b.Batch(2) b.Reject(batch) - suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -258,8 +268,12 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNothingNewFull() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectNoRoom() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectNoRoom() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) @@ -274,10 +288,10 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNoRoom() { b.Reject(batch) - suite.Equal(int64(3), b.Stats().MetricsDropped.Get()) + s.Equal(int64(3), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(4), MetricTime(5), @@ -287,8 +301,8 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNoRoom() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectRoomExact() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectRoomExact() { + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) batch := b.Batch(2) @@ -298,10 +312,10 @@ func (suite *BufferSuiteTest) TestBuffer_RejectRoomExact() { b.Reject(batch) - suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(1), MetricTime(2), @@ -311,8 +325,12 @@ func (suite *BufferSuiteTest) TestBuffer_RejectRoomExact() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectRoomOverwriteOld() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectRoomOverwriteOld() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -323,10 +341,10 @@ func (suite *BufferSuiteTest) TestBuffer_RejectRoomOverwriteOld() { b.Reject(batch) - suite.Equal(int64(1), b.Stats().MetricsDropped.Get()) + s.Equal(int64(1), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(2), MetricTime(3), @@ -336,8 +354,12 @@ func (suite *BufferSuiteTest) TestBuffer_RejectRoomOverwriteOld() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectPartialRoom() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectPartialRoom() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) @@ -350,10 +372,10 @@ func (suite *BufferSuiteTest) TestBuffer_RejectPartialRoom() { b.Add(MetricTime(7)) b.Reject(batch) - suite.Equal(int64(2), b.Stats().MetricsDropped.Get()) + s.Equal(int64(2), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(3), MetricTime(4), @@ -363,8 +385,12 @@ func (suite *BufferSuiteTest) TestBuffer_RejectPartialRoom() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -373,7 +399,7 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { b.Add(MetricTime(5)) // buffer: 1, 4, 5; batch: 2, 3 - suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsDropped.Get()) b.Add(MetricTime(6)) b.Add(MetricTime(7)) @@ -382,7 +408,7 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { b.Add(MetricTime(10)) // buffer: 8, 9, 10, 6, 7; batch: 2, 3 - suite.Equal(int64(3), b.Stats().MetricsDropped.Get()) + s.Equal(int64(3), b.Stats().MetricsDropped.Get()) b.Add(MetricTime(11)) b.Add(MetricTime(12)) @@ -390,13 +416,13 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { b.Add(MetricTime(14)) b.Add(MetricTime(15)) // buffer: 13, 14, 15, 11, 12; batch: 2, 3 - suite.Equal(int64(8), b.Stats().MetricsDropped.Get()) + s.Equal(int64(8), b.Stats().MetricsDropped.Get()) b.Reject(batch) - suite.Equal(int64(10), b.Stats().MetricsDropped.Get()) + s.Equal(int64(10), b.Stats().MetricsDropped.Get()) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(11), MetricTime(12), @@ -406,8 +432,12 @@ func (suite *BufferSuiteTest) TestBuffer_RejectNewMetricsWrapped() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectWrapped() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectWrapped() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(5) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -427,7 +457,7 @@ func (suite *BufferSuiteTest) TestBuffer_RejectWrapped() { b.Reject(batch) batch = b.Batch(5) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(8), MetricTime(9), @@ -437,8 +467,12 @@ func (suite *BufferSuiteTest) TestBuffer_RejectWrapped() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_RejectAdjustFirst() { - b := suite.newTestBuffer(10) +func (s *BufferSuiteTest) TestBuffer_RejectAdjustFirst() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + + b := s.newTestBuffer(10) b.Add(MetricTime(1)) b.Add(MetricTime(2)) b.Add(MetricTime(3)) @@ -469,7 +503,7 @@ func (suite *BufferSuiteTest) TestBuffer_RejectAdjustFirst() { b.Add(MetricTime(19)) batch = b.Batch(10) - testutil.RequireMetricsEqual(suite.T(), + testutil.RequireMetricsEqual(s.T(), []telegraf.Metric{ MetricTime(10), MetricTime(11), @@ -484,142 +518,151 @@ func (suite *BufferSuiteTest) TestBuffer_RejectAdjustFirst() { }, batch) } -func (suite *BufferSuiteTest) TestBuffer_AddDropsOverwrittenMetrics() { +func (s *BufferSuiteTest) TestBuffer_AddDropsOverwrittenMetrics() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) b.Add(m, m, m, m, m) - suite.Equal(int64(5), b.Stats().MetricsDropped.Get()) - suite.Equal(int64(0), b.Stats().MetricsWritten.Get()) + s.Equal(int64(5), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsWritten.Get()) } -func (suite *BufferSuiteTest) TestBuffer_AcceptRemovesBatch() { +func (s *BufferSuiteTest) TestBuffer_AcceptRemovesBatch() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m) batch := b.Batch(2) b.Accept(batch) - suite.Equal(1, b.Len()) + s.Equal(1, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_RejectLeavesBatch() { +func (s *BufferSuiteTest) TestBuffer_RejectLeavesBatch() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m) batch := b.Batch(2) b.Reject(batch) - suite.Equal(3, b.Len()) + s.Equal(3, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_AcceptWritesOverwrittenBatch() { +func (s *BufferSuiteTest) TestBuffer_AcceptWritesOverwrittenBatch() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(5) b.Add(m, m, m, m, m) b.Accept(batch) - suite.Equal(int64(0), b.Stats().MetricsDropped.Get()) - suite.Equal(int64(5), b.Stats().MetricsWritten.Get()) + s.Equal(int64(0), b.Stats().MetricsDropped.Get()) + s.Equal(int64(5), b.Stats().MetricsWritten.Get()) } -func (suite *BufferSuiteTest) TestBuffer_BatchRejectDropsOverwrittenBatch() { +func (s *BufferSuiteTest) TestBuffer_BatchRejectDropsOverwrittenBatch() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(5) b.Add(m, m, m, m, m) b.Reject(batch) - suite.Equal(int64(5), b.Stats().MetricsDropped.Get()) - suite.Equal(int64(0), b.Stats().MetricsWritten.Get()) + s.Equal(int64(5), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsWritten.Get()) } -func (suite *BufferSuiteTest) TestBuffer_MetricsOverwriteBatchAccept() { +func (s *BufferSuiteTest) TestBuffer_MetricsOverwriteBatchAccept() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(3) b.Add(m, m, m) b.Accept(batch) - suite.Equal(int64(0), b.Stats().MetricsDropped.Get(), "dropped") - suite.Equal(int64(3), b.Stats().MetricsWritten.Get(), "written") + s.Equal(int64(0), b.Stats().MetricsDropped.Get(), "dropped") + s.Equal(int64(3), b.Stats().MetricsWritten.Get(), "written") } -func (suite *BufferSuiteTest) TestBuffer_MetricsOverwriteBatchReject() { +func (s *BufferSuiteTest) TestBuffer_MetricsOverwriteBatchReject() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(3) b.Add(m, m, m) b.Reject(batch) - suite.Equal(int64(3), b.Stats().MetricsDropped.Get()) - suite.Equal(int64(0), b.Stats().MetricsWritten.Get()) + s.Equal(int64(3), b.Stats().MetricsDropped.Get()) + s.Equal(int64(0), b.Stats().MetricsWritten.Get()) } -func (suite *BufferSuiteTest) TestBuffer_MetricsBatchAcceptRemoved() { +func (s *BufferSuiteTest) TestBuffer_MetricsBatchAcceptRemoved() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(3) b.Add(m, m, m, m, m) b.Accept(batch) - suite.Equal(int64(2), b.Stats().MetricsDropped.Get()) - suite.Equal(int64(3), b.Stats().MetricsWritten.Get()) + s.Equal(int64(2), b.Stats().MetricsDropped.Get()) + s.Equal(int64(3), b.Stats().MetricsWritten.Get()) } -func (suite *BufferSuiteTest) TestBuffer_WrapWithBatch() { +func (s *BufferSuiteTest) TestBuffer_WrapWithBatch() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m) b.Batch(3) b.Add(m, m, m, m, m, m) - suite.Equal(int64(1), b.Stats().MetricsDropped.Get()) + s.Equal(int64(1), b.Stats().MetricsDropped.Get()) } -func (suite *BufferSuiteTest) TestBuffer_BatchNotRemoved() { +func (s *BufferSuiteTest) TestBuffer_BatchNotRemoved() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) b.Batch(2) - suite.Equal(5, b.Len()) + s.Equal(5, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_BatchRejectAcceptNoop() { +func (s *BufferSuiteTest) TestBuffer_BatchRejectAcceptNoop() { m := Metric() - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(m, m, m, m, m) batch := b.Batch(2) b.Reject(batch) b.Accept(batch) - suite.Equal(5, b.Len()) + s.Equal(5, b.Len()) } -func (suite *BufferSuiteTest) TestBuffer_AcceptCallsMetricAccept() { - var accept int - mm := &models.MockMetric{ - Metric: Metric(), - AcceptF: func() { - accept++ - }, +func (s *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNoBatch() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") } - b := suite.newTestBuffer(5) - b.Add(mm, mm, mm) - batch := b.Batch(2) - b.Accept(batch) - suite.Equal(2, accept) -} -func (suite *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNoBatch() { var reject int mm := &models.MockMetric{ Metric: Metric(), @@ -627,13 +670,17 @@ func (suite *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNoBatch() { reject++ }, } - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm) - suite.Equal(2, reject) + s.Equal(2, reject) } -func (suite *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNotInBatch() { +func (s *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNotInBatch() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + var reject int mm := &models.MockMetric{ Metric: Metric(), @@ -641,33 +688,20 @@ func (suite *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNotInBatch() { reject++ }, } - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(2) b.Add(mm, mm, mm, mm) - suite.Equal(2, reject) + s.Equal(2, reject) b.Reject(batch) - suite.Equal(4, reject) + s.Equal(4, reject) } -func (suite *BufferSuiteTest) TestBuffer_RejectCallsMetricRejectWithOverwritten() { - var reject int - mm := &models.MockMetric{ - Metric: Metric(), - RejectF: func() { - reject++ - }, +func (s *BufferSuiteTest) TestBuffer_AddOverwriteAndReject() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") } - b := suite.newTestBuffer(5) - b.Add(mm, mm, mm, mm, mm) - batch := b.Batch(5) - b.Add(mm, mm) - suite.Equal(0, reject) - b.Reject(batch) - suite.Equal(2, reject) -} -func (suite *BufferSuiteTest) TestBuffer_AddOverwriteAndReject() { var reject int mm := &models.MockMetric{ Metric: Metric(), @@ -675,19 +709,23 @@ func (suite *BufferSuiteTest) TestBuffer_AddOverwriteAndReject() { reject++ }, } - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(mm, mm, mm, mm, mm) batch := b.Batch(5) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm, mm, mm, mm) b.Add(mm, mm, mm, mm, mm) - suite.Equal(15, reject) + s.Equal(15, reject) b.Reject(batch) - suite.Equal(20, reject) + s.Equal(20, reject) } -func (suite *BufferSuiteTest) TestBuffer_AddOverwriteAndRejectOffset() { +func (s *BufferSuiteTest) TestBuffer_AddOverwriteAndRejectOffset() { + if !s.hasMaxCapacity { + s.T().Skip("tested buffer does not have a maximum capacity") + } + var reject int var accept int mm := &models.MockMetric{ @@ -699,42 +737,32 @@ func (suite *BufferSuiteTest) TestBuffer_AddOverwriteAndRejectOffset() { accept++ }, } - b := suite.newTestBuffer(5) + b := s.newTestBuffer(5) b.Add(mm, mm, mm) b.Add(mm, mm, mm, mm) - suite.Equal(2, reject) + s.Equal(2, reject) batch := b.Batch(5) b.Add(mm, mm, mm, mm) - suite.Equal(2, reject) + s.Equal(2, reject) b.Add(mm, mm, mm, mm) - suite.Equal(5, reject) + s.Equal(5, reject) b.Add(mm, mm, mm, mm) - suite.Equal(9, reject) + s.Equal(9, reject) b.Add(mm, mm, mm, mm) - suite.Equal(13, reject) + s.Equal(13, reject) b.Accept(batch) - suite.Equal(13, reject) - suite.Equal(5, accept) + s.Equal(13, reject) + s.Equal(5, accept) } -func (suite *BufferSuiteTest) TestBuffer_RejectEmptyBatch() { - b := suite.newTestBuffer(5) +func (s *BufferSuiteTest) TestBuffer_RejectEmptyBatch() { + b := s.newTestBuffer(5) batch := b.Batch(2) b.Add(MetricTime(1)) b.Reject(batch) b.Add(MetricTime(2)) batch = b.Batch(2) for _, m := range batch { - suite.NotNil(m) - } -} - -// Benchmark test outside the suite -func BenchmarkMemoryAddMetrics(b *testing.B) { - buf, err := NewBuffer("test", "", 10000, "memory", "") - require.NoError(b, err) - m := Metric() - for n := 0; n < b.N; n++ { - buf.Add(m) + s.NotNil(m) } } From 8d3987a9fb84298f354c49686dabd822f68dfa87 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Tue, 4 Jun 2024 13:41:07 -0500 Subject: [PATCH 07/10] add new libraries to dependency license list --- docs/LICENSE_OF_DEPENDENCIES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/LICENSE_OF_DEPENDENCIES.md b/docs/LICENSE_OF_DEPENDENCIES.md index 4d3f189483269..06928cc12126f 100644 --- a/docs/LICENSE_OF_DEPENDENCIES.md +++ b/docs/LICENSE_OF_DEPENDENCIES.md @@ -349,6 +349,8 @@ following works: - github.com/tidwall/gjson [MIT License](https://github.com/tidwall/gjson/blob/master/LICENSE) - github.com/tidwall/match [MIT License](https://github.com/tidwall/match/blob/master/LICENSE) - github.com/tidwall/pretty [MIT License](https://github.com/tidwall/pretty/blob/master/LICENSE) +- github.com/tidwall/tinylru [MIT License](https://github.com/tidwall/tinylru/blob/master/LICENSE) +- github.com/tidwall/wal [MIT License](https://github.com/tidwall/wal/blob/master/LICENSE) - github.com/tinylib/msgp [MIT License](https://github.com/tinylib/msgp/blob/master/LICENSE) - github.com/tklauser/go-sysconf [BSD 3-Clause "New" or "Revised" License](https://github.com/tklauser/go-sysconf/blob/master/LICENSE) - github.com/tklauser/numcpus [Apache License 2.0](https://github.com/tklauser/numcpus/blob/master/LICENSE) From 5d75b73391ea54baac4178c4102669bdefd5420d Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Tue, 4 Jun 2024 14:56:48 -0500 Subject: [PATCH 08/10] address errcheck issue --- models/buffer_disk.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/models/buffer_disk.go b/models/buffer_disk.go index ee5aae59bd915..8e2c49f155264 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -188,6 +188,9 @@ func (b *DiskBuffer) resetBatch() { func (b *DiskBuffer) resetWalFile() { b.walFile.Close() os.Remove(b.walFilePath) - walFile, _ := wal.Open(b.walFilePath, nil) + walFile, err := wal.Open(b.walFilePath, nil) + if err != nil { + panic(err) + } b.walFile = walFile } From cd2d3f4f0ed785fde7da1d71e40bd15379ac1a14 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Wed, 19 Jun 2024 14:55:59 -0500 Subject: [PATCH 09/10] fix tracking metrics being accepted too early --- accumulator.go | 4 + agent/agent.go | 8 +- metric.go | 4 +- metric/deserialize.go | 78 +++++++++++++++ metric/init.go | 7 ++ metric/metric.go | 21 ---- metric/tracking.go | 60 +++++++----- models/buffer_disk.go | 47 ++++++--- models/buffer_disk_test.go | 95 ++++++++++++++++--- models/buffer_suite_test.go | 6 +- models/mock/metric.go | 4 + models/testdata/testwal/00000000000000000001 | Bin 0 -> 1812 bytes 12 files changed, 258 insertions(+), 76 deletions(-) create mode 100644 metric/deserialize.go create mode 100644 metric/init.go create mode 100644 models/testdata/testwal/00000000000000000001 diff --git a/accumulator.go b/accumulator.go index ece69f3b176d1..4991752bb2d94 100644 --- a/accumulator.go +++ b/accumulator.go @@ -57,6 +57,10 @@ type Accumulator interface { // TrackingID uniquely identifies a tracked metric group type TrackingID uint64 +type TrackingData interface { + ID() TrackingID +} + // DeliveryInfo provides the results of a delivered metric group. type DeliveryInfo interface { // ID is the TrackingID diff --git a/agent/agent.go b/agent/agent.go index d9c3a6c039080..14d8a80280c79 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -14,6 +14,7 @@ import ( "github.com/influxdata/telegraf/config" "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/internal/snmp" + "github.com/influxdata/telegraf/metric" "github.com/influxdata/telegraf/models" "github.com/influxdata/telegraf/plugins/processors" "github.com/influxdata/telegraf/plugins/serializers/influx" @@ -127,6 +128,7 @@ func (a *Agent) Run(ctx context.Context) error { startTime := time.Now() log.Printf("D! [agent] Connecting outputs") + metric.Init() next, ou, err := a.startOutputs(ctx, a.Config.Outputs) if err != nil { return err @@ -871,12 +873,12 @@ func (a *Agent) runOutputs( }(output) } - for metric := range unit.src { + for m := range unit.src { for i, output := range unit.outputs { if i == len(a.Config.Outputs)-1 { - output.AddMetric(metric) + output.AddMetric(m) } else { - output.AddMetric(metric.Copy()) + output.AddMetric(m.Copy()) } } } diff --git a/metric.go b/metric.go index 93d46858b0db6..adad3cfc38c33 100644 --- a/metric.go +++ b/metric.go @@ -125,9 +125,6 @@ type Metric interface { // Drop marks the metric as processed successfully without being written // to any output. Drop() - - // ToBytes converts the metric a byte array using the gob encoder. - ToBytes() ([]byte, error) } // TemplateMetric is an interface to use in templates (e.g text/template) @@ -152,5 +149,6 @@ type UnwrappableMetric interface { type TrackingMetric interface { // TrackingID returns the ID used for tracking the metric TrackingID() TrackingID + TrackingData() TrackingData UnwrappableMetric } diff --git a/metric/deserialize.go b/metric/deserialize.go new file mode 100644 index 0000000000000..a253406a3b50c --- /dev/null +++ b/metric/deserialize.go @@ -0,0 +1,78 @@ +package metric + +import ( + "bytes" + "encoding/gob" + "errors" + "fmt" + "sync" + + "github.com/influxdata/telegraf" +) + +// storage for tracking data that can't be serialized to disk +var ( + // todo need some way to empty this map out when done with a tracking ID. + // grouped tracking metrics means that ID->Data association is not one to one, + // many metrics could be associated with one tracking ID so we cannot just + // clear this every time in FromBytes. + trackingStore = make(map[telegraf.TrackingID]telegraf.TrackingData) + mu = sync.Mutex{} + + // ErrSkipTracking indicates that tracking information could not be found after + // deserializing a metric from bytes. In this case we should skip the metric + // and continue as if it does not exist. + ErrSkipTracking = errors.New("metric tracking data not found") +) + +type serializedMetric struct { + M telegraf.Metric + TID telegraf.TrackingID +} + +func ToBytes(m telegraf.Metric) ([]byte, error) { + var sm serializedMetric + if um, ok := m.(telegraf.UnwrappableMetric); ok { + sm.M = um.Unwrap() + } else { + sm.M = m + } + + if tm, ok := m.(telegraf.TrackingMetric); ok { + sm.TID = tm.TrackingID() + + mu.Lock() + trackingStore[sm.TID] = tm.TrackingData() + mu.Unlock() + } + + var buf bytes.Buffer + encoder := gob.NewEncoder(&buf) + if err := encoder.Encode(&sm); err != nil { + return nil, fmt.Errorf("failed to encode metric to bytes: %w", err) + } + return buf.Bytes(), nil +} + +func FromBytes(b []byte) (telegraf.Metric, error) { + buf := bytes.NewBuffer(b) + decoder := gob.NewDecoder(buf) + + var sm *serializedMetric + if err := decoder.Decode(&sm); err != nil { + return nil, fmt.Errorf("failed to decode metric from bytes: %w", err) + } + + m := sm.M + if sm.TID != 0 { + mu.Lock() + td := trackingStore[sm.TID] + mu.Unlock() + + if td == nil { + return nil, ErrSkipTracking + } + m = rebuildTrackingMetric(m, td) + } + return m, nil +} diff --git a/metric/init.go b/metric/init.go new file mode 100644 index 0000000000000..85c901ce16b39 --- /dev/null +++ b/metric/init.go @@ -0,0 +1,7 @@ +package metric + +import "encoding/gob" + +func Init() { + gob.RegisterName("metric.metric", &metric{}) +} diff --git a/metric/metric.go b/metric/metric.go index a087a7a98d85b..f07f54c2672e6 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -1,8 +1,6 @@ package metric import ( - "bytes" - "encoding/gob" "fmt" "hash/fnv" "sort" @@ -386,22 +384,3 @@ func convertField(v interface{}) interface{} { } return nil } - -func (m *metric) ToBytes() ([]byte, error) { - var buf bytes.Buffer - encoder := gob.NewEncoder(&buf) - if err := encoder.Encode(m); err != nil { - return nil, fmt.Errorf("failed to encode metric to bytes: %w", err) - } - return buf.Bytes(), nil -} - -func FromBytes(b []byte) (telegraf.Metric, error) { - buf := bytes.NewBuffer(b) - decoder := gob.NewDecoder(buf) - var m *metric - if err := decoder.Decode(&m); err != nil { - return nil, fmt.Errorf("failed to decode metric from bytes: %w", err) - } - return m, nil -} diff --git a/metric/tracking.go b/metric/tracking.go index 50f11c74d6dae..8e700d8f98ae9 100644 --- a/metric/tracking.go +++ b/metric/tracking.go @@ -33,35 +33,36 @@ func newTrackingID() telegraf.TrackingID { } type trackingData struct { - id telegraf.TrackingID - rc int32 - acceptCount int32 - rejectCount int32 + //nolint:revive // method is already named ID + Id telegraf.TrackingID + Rc int32 + AcceptCount int32 + RejectCount int32 notifyFunc NotifyFunc } func (d *trackingData) incr() { - atomic.AddInt32(&d.rc, 1) + atomic.AddInt32(&d.Rc, 1) } func (d *trackingData) decr() int32 { - return atomic.AddInt32(&d.rc, -1) + return atomic.AddInt32(&d.Rc, -1) } func (d *trackingData) accept() { - atomic.AddInt32(&d.acceptCount, 1) + atomic.AddInt32(&d.AcceptCount, 1) } func (d *trackingData) reject() { - atomic.AddInt32(&d.rejectCount, 1) + atomic.AddInt32(&d.RejectCount, 1) } func (d *trackingData) notify() { d.notifyFunc( &deliveryInfo{ - id: d.id, - accepted: int(d.acceptCount), - rejected: int(d.rejectCount), + id: d.Id, + accepted: int(d.AcceptCount), + rejected: int(d.RejectCount), }, ) } @@ -75,10 +76,10 @@ func newTrackingMetric(metric telegraf.Metric, fn NotifyFunc) (telegraf.Metric, m := &trackingMetric{ Metric: metric, d: &trackingData{ - id: newTrackingID(), - rc: 1, - acceptCount: 0, - rejectCount: 0, + Id: newTrackingID(), + Rc: 1, + AcceptCount: 0, + RejectCount: 0, notifyFunc: fn, }, } @@ -86,15 +87,22 @@ func newTrackingMetric(metric telegraf.Metric, fn NotifyFunc) (telegraf.Metric, if finalizer != nil { runtime.SetFinalizer(m.d, finalizer) } - return m, m.d.id + return m, m.d.Id +} + +func rebuildTrackingMetric(metric telegraf.Metric, td telegraf.TrackingData) telegraf.Metric { + return &trackingMetric{ + Metric: metric, + d: td.(*trackingData), + } } func newTrackingMetricGroup(group []telegraf.Metric, fn NotifyFunc) ([]telegraf.Metric, telegraf.TrackingID) { d := &trackingData{ - id: newTrackingID(), - rc: 0, - acceptCount: 0, - rejectCount: 0, + Id: newTrackingID(), + Rc: 0, + AcceptCount: 0, + RejectCount: 0, notifyFunc: fn, } @@ -114,7 +122,7 @@ func newTrackingMetricGroup(group []telegraf.Metric, fn NotifyFunc) ([]telegraf. d.notify() } - return group, d.id + return group, d.Id } func (m *trackingMetric) Copy() telegraf.Metric { @@ -152,7 +160,11 @@ func (m *trackingMetric) decr() { // Unwrap allows to access the underlying metric directly e.g. for go-templates func (m *trackingMetric) TrackingID() telegraf.TrackingID { - return m.d.id + return m.d.ID() +} + +func (m *trackingMetric) TrackingData() telegraf.TrackingData { + return m.d } // Unwrap allows to access the underlying metric directly e.g. for go-templates @@ -173,3 +185,7 @@ func (r *deliveryInfo) ID() telegraf.TrackingID { func (r *deliveryInfo) Delivered() bool { return r.rejected == 0 } + +func (d *trackingData) ID() telegraf.TrackingID { + return d.Id +} diff --git a/models/buffer_disk.go b/models/buffer_disk.go index 8e2c49f155264..3cc61ef4ee9e7 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -1,6 +1,7 @@ package models import ( + "errors" "fmt" "os" "sync" @@ -17,8 +18,12 @@ type DiskBuffer struct { walFile *wal.Log walFilePath string - batchFirst uint64 // index of the first metric in the batch - batchSize uint64 // number of metrics currently in the batch + batchFirst uint64 // Index of the first metric in the batch + batchSize uint64 // Number of metrics currently in the batch + + // Ending point of metrics read from disk on telegraf launch. + // Used to know whether to discard tracking metrics. + originalEnd uint64 } func NewDiskBuffer(name string, path string, stats BufferStats) (*DiskBuffer, error) { @@ -82,12 +87,11 @@ func (b *DiskBuffer) Add(metrics ...telegraf.Metric) int { } func (b *DiskBuffer) addSingle(m telegraf.Metric) bool { - data, err := m.ToBytes() + data, err := metric.ToBytes(m) if err != nil { panic(err) } err = b.walFile.Write(b.writeIndex(), data) - m.Accept() if err == nil { b.metricAdded() return true @@ -100,11 +104,10 @@ func (b *DiskBuffer) addBatch(metrics []telegraf.Metric) int { written := 0 batch := new(wal.Batch) for _, m := range metrics { - data, err := m.ToBytes() + data, err := metric.ToBytes(m) if err != nil { panic(err) } - m.Accept() // accept here, since the metric object is no longer retained from here batch.Write(b.writeIndex(), data) b.metricAdded() written++ @@ -124,20 +127,36 @@ func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { // no metrics in the wal file, so return an empty array return make([]telegraf.Metric, 0) } - b.batchSize = uint64(min(b.length(), batchSize)) b.batchFirst = b.readIndex() - metrics := make([]telegraf.Metric, b.batchSize) + var metrics []telegraf.Metric - for i := 0; i < int(b.batchSize); i++ { - data, err := b.walFile.Read(b.batchFirst + uint64(i)) + b.batchSize = 0 + readIndex := b.batchFirst + endIndex := b.writeIndex() + for batchSize > 0 && readIndex < endIndex { + data, err := b.walFile.Read(readIndex) if err != nil { panic(err) } + readIndex++ + m, err := metric.FromBytes(data) + if errors.Is(err, metric.ErrSkipTracking) { + // could not look up tracking information for metric, skip + continue + } if err != nil { + // non-recoverable error in deserialization, abort panic(err) } - metrics[i] = m + if _, ok := m.(telegraf.TrackingMetric); ok && readIndex < b.originalEnd { + // tracking metric left over from previous instance, skip + continue + } + + metrics = append(metrics, m) + b.batchSize++ + batchSize-- } return metrics } @@ -161,6 +180,12 @@ func (b *DiskBuffer) Accept(batch []telegraf.Metric) { panic(err) } } + + // check if the original end index is still valid, clear if not + if b.originalEnd < b.readIndex() { + b.originalEnd = 0 + } + b.resetBatch() b.BufferSize.Set(int64(b.length())) } diff --git a/models/buffer_disk_test.go b/models/buffer_disk_test.go index e9f675b1c0403..17d8b19ab001e 100644 --- a/models/buffer_disk_test.go +++ b/models/buffer_disk_test.go @@ -1,18 +1,27 @@ package models import ( + "fmt" + "io" "os" + "path/filepath" "testing" - models "github.com/influxdata/telegraf/models/mock" + "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/metric" + "github.com/influxdata/telegraf/testutil" "github.com/stretchr/testify/require" ) func newTestDiskBuffer(t testing.TB) Buffer { - t.Helper() path, err := os.MkdirTemp("", "*-buffer-test") require.NoError(t, err) - buf, err := NewBuffer("test", "", 0, "disk", path) + return newTestDiskBufferWithPath(t, "test", path) +} + +func newTestDiskBufferWithPath(t testing.TB, name string, path string) Buffer { + t.Helper() + buf, err := NewBuffer(name, "", 0, "disk", path) require.NoError(t, err) buf.Stats().MetricsAdded.Set(0) buf.Stats().MetricsWritten.Set(0) @@ -20,18 +29,74 @@ func newTestDiskBuffer(t testing.TB) Buffer { return buf } -func TestBuffer_AddCallsMetricAccept(t *testing.T) { - var accept int - mm := &models.MockMetric{ - Metric: Metric(), - AcceptF: func() { - accept++ - }, - } +func TestBuffer_RetainsTrackingInformation(t *testing.T) { + var delivered int + mm, _ := metric.WithTracking(Metric(), func(_ telegraf.DeliveryInfo) { + delivered++ + }) + metric.Init() b := newTestDiskBuffer(t) - b.Add(mm, mm, mm) - batch := b.Batch(2) + b.Add(mm) + batch := b.Batch(1) b.Accept(batch) - // all 3 metrics should be accepted as metric Accept() is called on buffer Add() - require.Equal(t, 3, accept) + require.Equal(t, 1, delivered) +} + +// WAL file tested here was written as: +// 1: Metric() +// 2: Metric() +// 3: Metric() +// 4: metric.WithTracking(Metric()) +// 5: Metric() +// +// Expected to drop the 4th metric, as tracking metrics from +// previous instances are dropped when the wal file is reopened. +func TestBuffer_TrackingDroppedFromOldWal(t *testing.T) { + // copy the testdata so we do not destroy the testdata wal file + path, err := os.MkdirTemp("", "*-buffer-test") + require.NoError(t, err) + f, err := os.Create(path + "/00000000000000000001") + require.NoError(t, err) + f1, err := os.Open("testdata/testwal/00000000000000000001") + require.NoError(t, err) + written, err := io.Copy(f, f1) + require.NoError(t, err) + fmt.Println(written) + + metric.Init() + b := newTestDiskBufferWithPath(t, filepath.Base(path), filepath.Dir(path)) + batch := b.Batch(4) + expected := []telegraf.Metric{ + Metric(), Metric(), Metric(), Metric(), + } + testutil.RequireMetricsEqual(t, expected, batch) +} + +/* +// Function used to create the test data used in the test above +func Test_CreateTestData(t *testing.T) { + metric.Init() + walfile, _ := wal.Open("testdata/testwal", nil) + + data, err := metric.ToBytes(Metric()) + require.NoError(t, err) + require.NoError(t, walfile.Write(1, data)) + + data, err = metric.ToBytes(Metric()) + require.NoError(t, err) + require.NoError(t, walfile.Write(2, data)) + + data, err = metric.ToBytes(Metric()) + require.NoError(t, err) + require.NoError(t, walfile.Write(3, data)) + + m, _ := metric.WithTracking(Metric(), func(di telegraf.DeliveryInfo) {}) + data, err = metric.ToBytes(m) + require.NoError(t, err) + require.NoError(t, walfile.Write(4, data)) + + data, err = metric.ToBytes(Metric()) + require.NoError(t, err) + require.NoError(t, walfile.Write(5, data)) } +*/ diff --git a/models/buffer_suite_test.go b/models/buffer_suite_test.go index 06c31b35636ec..288b432737ab4 100644 --- a/models/buffer_suite_test.go +++ b/models/buffer_suite_test.go @@ -23,9 +23,13 @@ type BufferSuiteTest struct { func (s *BufferSuiteTest) SetupTest() { if s.bufferType == "disk" { path, err := os.MkdirTemp("", "*-buffer-test") - s.NoError(err) + s.Require().NoError(err) s.bufferPath = path s.hasMaxCapacity = false + // lets gob properly encode our metrics + metric.Init() + } else { + s.hasMaxCapacity = true } } diff --git a/models/mock/metric.go b/models/mock/metric.go index a4de92ac5704e..0f0378732f1f4 100644 --- a/models/mock/metric.go +++ b/models/mock/metric.go @@ -20,3 +20,7 @@ func (m *MockMetric) Reject() { func (m *MockMetric) Drop() { m.DropF() } + +func (m *MockMetric) Unwrap() telegraf.Metric { + return m.Metric +} diff --git a/models/testdata/testwal/00000000000000000001 b/models/testdata/testwal/00000000000000000001 new file mode 100644 index 0000000000000000000000000000000000000000..a4d72e966edfb738eb6a137c512fdbd7eaf1437f GIT binary patch literal 1812 zcmaFCq+QR<$S6>pT9lcXlUbFT;+tAhl$p%5EV_@WgNV{dG=A-~c zx)3Tdfg1kzAo!I9sf;W@Tjl<@0$su%9jjH6nvW%UTAuhdnJPKe5EjgoB0QpQ{4{BR?ZhBVYN%??8%y;ol5U;Jp|P e>Cuo58D44n)o7X~HBDQPReCToj+P&EE2#k!ulq{? literal 0 HcmV?d00001 From 3d8a6d74cdf530472f56cd2ccd6e45608e451200 Mon Sep 17 00:00:00 2001 From: Dane Strandboge <136023093+DStrand1@users.noreply.github.com> Date: Wed, 19 Jun 2024 15:05:52 -0500 Subject: [PATCH 10/10] reviews --- models/buffer.go | 9 +++------ models/buffer_disk.go | 33 +++++++++++++++++---------------- models/buffer_mem_test.go | 5 ++--- models/buffer_suite_test.go | 32 +++++++++++++++++++++++++++----- models/mock/metric.go | 26 -------------------------- 5 files changed, 49 insertions(+), 56 deletions(-) delete mode 100644 models/mock/metric.go diff --git a/models/buffer.go b/models/buffer.go index 9684ce4440a17..cdfad68b5e46d 100644 --- a/models/buffer.go +++ b/models/buffer.go @@ -1,6 +1,8 @@ package models import ( + "fmt" + "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/selfstat" ) @@ -52,14 +54,9 @@ func NewBuffer(name string, alias string, capacity int, strategy string, path st return NewMemoryBuffer(capacity, bm) case "disk": return NewDiskBuffer(name, path, bm) - case "overflow": - // todo implementme - // todo log currently unimplemented - return NewMemoryBuffer(capacity, bm) } - // todo log invalid buffer strategy configuration provided, falling back to memory - return NewMemoryBuffer(capacity, bm) + return nil, fmt.Errorf("invalid buffer strategy %q", strategy) } func NewBufferMetrics(name string, alias string, capacity int) BufferStats { diff --git a/models/buffer_disk.go b/models/buffer_disk.go index 3cc61ef4ee9e7..ef05791618247 100644 --- a/models/buffer_disk.go +++ b/models/buffer_disk.go @@ -6,17 +6,18 @@ import ( "os" "sync" + "github.com/tidwall/wal" + "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/metric" - "github.com/tidwall/wal" ) type DiskBuffer struct { BufferStats sync.Mutex - walFile *wal.Log - walFilePath string + file *wal.Log + path string batchFirst uint64 // Index of the first metric in the batch batchSize uint64 // Number of metrics currently in the batch @@ -34,8 +35,8 @@ func NewDiskBuffer(name string, path string, stats BufferStats) (*DiskBuffer, er } return &DiskBuffer{ BufferStats: stats, - walFile: walFile, - walFilePath: filePath, + file: walFile, + path: filePath, }, nil } @@ -55,7 +56,7 @@ func (b *DiskBuffer) length() int { // readIndex is the first index to start reading metrics from, or the head of the buffer func (b *DiskBuffer) readIndex() uint64 { - index, err := b.walFile.FirstIndex() + index, err := b.file.FirstIndex() if err != nil { panic(err) // can only occur with a corrupt wal file } @@ -64,7 +65,7 @@ func (b *DiskBuffer) readIndex() uint64 { // writeIndex is the first index to start writing metrics to, or the tail of the buffer func (b *DiskBuffer) writeIndex() uint64 { - index, err := b.walFile.LastIndex() + index, err := b.file.LastIndex() if err != nil { panic(err) // can only occur with a corrupt wal file } @@ -91,7 +92,7 @@ func (b *DiskBuffer) addSingle(m telegraf.Metric) bool { if err != nil { panic(err) } - err = b.walFile.Write(b.writeIndex(), data) + err = b.file.Write(b.writeIndex(), data) if err == nil { b.metricAdded() return true @@ -112,7 +113,7 @@ func (b *DiskBuffer) addBatch(metrics []telegraf.Metric) int { b.metricAdded() written++ } - err := b.walFile.WriteBatch(batch) + err := b.file.WriteBatch(batch) if err != nil { return 0 // todo error handle, test if a partial write occur } @@ -125,7 +126,7 @@ func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { if b.length() == 0 { // no metrics in the wal file, so return an empty array - return make([]telegraf.Metric, 0) + return []telegraf.Metric{} } b.batchFirst = b.readIndex() var metrics []telegraf.Metric @@ -134,7 +135,7 @@ func (b *DiskBuffer) Batch(batchSize int) []telegraf.Metric { readIndex := b.batchFirst endIndex := b.writeIndex() for batchSize > 0 && readIndex < endIndex { - data, err := b.walFile.Read(readIndex) + data, err := b.file.Read(readIndex) if err != nil { panic(err) } @@ -175,7 +176,7 @@ func (b *DiskBuffer) Accept(batch []telegraf.Metric) { if b.length() == len(batch) { b.resetWalFile() } else { - err := b.walFile.TruncateFront(b.batchFirst + uint64(len(batch))) + err := b.file.TruncateFront(b.batchFirst + uint64(len(batch))) if err != nil { panic(err) } @@ -211,11 +212,11 @@ func (b *DiskBuffer) resetBatch() { // todo to actually clear the walfile completely if needed, since Truncate() calls require // todo at least one entry remains in them otherwise they return an error. func (b *DiskBuffer) resetWalFile() { - b.walFile.Close() - os.Remove(b.walFilePath) - walFile, err := wal.Open(b.walFilePath, nil) + b.file.Close() + os.Remove(b.path) + walFile, err := wal.Open(b.path, nil) if err != nil { panic(err) } - b.walFile = walFile + b.file = walFile } diff --git a/models/buffer_mem_test.go b/models/buffer_mem_test.go index 2c470685f8da5..a4ec6568a6b96 100644 --- a/models/buffer_mem_test.go +++ b/models/buffer_mem_test.go @@ -3,7 +3,6 @@ package models import ( "testing" - models "github.com/influxdata/telegraf/models/mock" "github.com/stretchr/testify/require" ) @@ -19,7 +18,7 @@ func newTestMemoryBuffer(t testing.TB, capacity int) Buffer { func TestBuffer_AcceptCallsMetricAccept(t *testing.T) { var accept int - mm := &models.MockMetric{ + mm := &MockMetric{ Metric: Metric(), AcceptF: func() { accept++ @@ -34,7 +33,7 @@ func TestBuffer_AcceptCallsMetricAccept(t *testing.T) { func TestBuffer_RejectCallsMetricRejectWithOverwritten(t *testing.T) { var reject int - mm := &models.MockMetric{ + mm := &MockMetric{ Metric: Metric(), RejectF: func() { reject++ diff --git a/models/buffer_suite_test.go b/models/buffer_suite_test.go index 288b432737ab4..4f5e46d21bc24 100644 --- a/models/buffer_suite_test.go +++ b/models/buffer_suite_test.go @@ -7,11 +7,33 @@ import ( "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/metric" - models "github.com/influxdata/telegraf/models/mock" "github.com/influxdata/telegraf/testutil" "github.com/stretchr/testify/suite" ) +type MockMetric struct { + telegraf.Metric + AcceptF func() + RejectF func() + DropF func() +} + +func (m *MockMetric) Accept() { + m.AcceptF() +} + +func (m *MockMetric) Reject() { + m.RejectF() +} + +func (m *MockMetric) Drop() { + m.DropF() +} + +func (m *MockMetric) Unwrap() telegraf.Metric { + return m.Metric +} + type BufferSuiteTest struct { suite.Suite bufferType string @@ -668,7 +690,7 @@ func (s *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNoBatch() { } var reject int - mm := &models.MockMetric{ + mm := &MockMetric{ Metric: Metric(), RejectF: func() { reject++ @@ -686,7 +708,7 @@ func (s *BufferSuiteTest) TestBuffer_AddCallsMetricRejectWhenNotInBatch() { } var reject int - mm := &models.MockMetric{ + mm := &MockMetric{ Metric: Metric(), RejectF: func() { reject++ @@ -707,7 +729,7 @@ func (s *BufferSuiteTest) TestBuffer_AddOverwriteAndReject() { } var reject int - mm := &models.MockMetric{ + mm := &MockMetric{ Metric: Metric(), RejectF: func() { reject++ @@ -732,7 +754,7 @@ func (s *BufferSuiteTest) TestBuffer_AddOverwriteAndRejectOffset() { var reject int var accept int - mm := &models.MockMetric{ + mm := &MockMetric{ Metric: Metric(), RejectF: func() { reject++ diff --git a/models/mock/metric.go b/models/mock/metric.go deleted file mode 100644 index 0f0378732f1f4..0000000000000 --- a/models/mock/metric.go +++ /dev/null @@ -1,26 +0,0 @@ -package models - -import "github.com/influxdata/telegraf" - -type MockMetric struct { - telegraf.Metric - AcceptF func() - RejectF func() - DropF func() -} - -func (m *MockMetric) Accept() { - m.AcceptF() -} - -func (m *MockMetric) Reject() { - m.RejectF() -} - -func (m *MockMetric) Drop() { - m.DropF() -} - -func (m *MockMetric) Unwrap() telegraf.Metric { - return m.Metric -}