feat(wal): Benchmark and improve WAL writes using Reset. #13272
Conversation
pkg/storage/wal/segment.go
Outdated
| streams: swiss.NewMap[streamID, *streamSegment](64), | ||
| buf1: encoding.EncWith(make([]byte, 0, 4)), | ||
| func NewWalSegmentWriter() (*SegmentWriter, error) { | ||
| idxWriter, err := index.NewWriter(context.TODO()) |
There was a problem hiding this comment.
Do we have to pass a ctx to the index? I think we can remove it because we only ever pass context.TODO(). We also don't pass a context to the parent SegmentWriter so the utility is very limited.
There was a problem hiding this comment.
Yes I'll look into that.
pkg/storage/wal/segment.go
Outdated
| s = streamSegmentPool.Get().(*streamSegment) | ||
| s.lbls = lbls | ||
| s.tenantID = tenantID | ||
| s.entries = s.entries[:0] |
There was a problem hiding this comment.
Should a streamSegment have a reset() method?
It looks like only one line to reset them but it would maintain consistency with our other Readers/Writers.
| t.Logf("Series sizes: [%s]\n", sizesString) | ||
| } | ||
|
|
||
| func TestReset(t *testing.T) { |
There was a problem hiding this comment.
Could you add/modify this test case so the re-use case adds more data than the initial case?
If we had re-use errors, we'd likely panic when re-using a small buffer buit we'd be less likely to panic when re-using a large buffer.
| for i := 0; i < b.N; i++ { | ||
| writer := pool.Get().(*SegmentWriter) | ||
|
|
||
| dst.Reset() |
There was a problem hiding this comment.
This benchmark isn't running in parallel (since dst is reused). You could remove the pool for writers from the benchmark, or add a pool for dst and run the whole thing in parallel :)
What this PR does / why we need it:
This adds a benchmark and some improvement to the write path of wal segment.
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/loki-private/issues/1005 https://github.com/grafana/loki-private/issues/1004