Skip to content

fix: pool zstd writers to mitigate leaking memory#412

Draft
roessland wants to merge 2 commits into
axiomhq:mainfrom
roessland:main
Draft

fix: pool zstd writers to mitigate leaking memory#412
roessland wants to merge 2 commits into
axiomhq:mainfrom
roessland:main

Conversation

@roessland

Copy link
Copy Markdown

IngestEvents creates a new zstd.NewWriter on every call, allocating ~4 MB of encoder state each time. With the slog adapter flushing every second, this causes 336 MB peak heap in 2.5 minutes of tiny log traffic, and 896 MB under 4 concurrent callers — leading to OOM kills in production.

Fix: use a sync.Pool of zstd.Encoder with SpeedFastest level, reusing writers via Reset(). This reduces peak heap from 336 MB to 2 MB.

Containers running latest version of axiom-go requires seem to require at least ~100 MB more memory due to zstd encoder state, even with these fixes.

Disclaimer: PR description and code 100% AI generated.

IngestEvents creates a new zstd.NewWriter on every call, allocating ~4 MB
of encoder state each time. With the slog adapter flushing every second,
this causes 336 MB peak heap in 2.5 minutes of tiny log traffic, and
896 MB under 4 concurrent callers — leading to OOM kills in production.

Fix: use a sync.Pool of zstd.Encoder with SpeedFastest level, reusing
writers via Reset(). This reduces peak heap from 336 MB to 2 MB.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roessland roessland requested a review from lukasmalkmus as a code owner March 2, 2026 12:18
@lukasmalkmus

Copy link
Copy Markdown
Collaborator

Hi,

Thanks for the PR!

Disclaimer: PR description and code 100% AI generated.

Totally fine.

But: I would have loved to see the journey some more of the journey that lead to the discovery and fix (as far as possible, given that you obviously can't share all production context). But something more than just numbers?

Also: Are the numbers in the PR description (for the fix) obtained from running the reproducer committed as part of this PR?

Not a criticism by the way, I just think we have a bit of catch-up to do now that AI assisted (or even fully automated) contributions start to ramp up and land here. I'm operating pretty autonomous myself, as well. I just think this repository would deserve some more details in documentation for agents on how to operate. Because the fix itself is good and demonstrates the problem quite well. What stops me from merging this today is:

  • Inconsistency: encoder.go:ZstdEncoder() is not pooled but has the same problem
  • The committed test file is a good demonstration but not a guard. I would not like to see it committed. Also the TotalAlloc log line in the pooled test is misleading since runtime.MemStats.TotalAlloc is monotonically increasing across the process and includes allocations from earlier tests
  • panic being used
  • Silently changing from SpeedDefault to SpeedFastest. It's unclear what impact this has on the numbers you shared in the PR description

Feel free to iterate but I can also give this a shot. I hold some opinions on the API, e.g. giving consumers a helper for managing their own pools.

Replace io.Pipe + goroutine pattern with eager bytes.Buffer compression.
The old pattern leaked goroutines when the pipe reader was abandoned
during HTTP retries or context cancellation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roessland

roessland commented Mar 2, 2026

Copy link
Copy Markdown
Author

Thanks for the review! I'll clean it up a bit and remove all the AI slop.

I noticed one of our newer internal services had a huge increase in memory usage. This service has very little traffic so I wasn't expecting much memory usage, and we hadn't set up alarms yet. The only symptom was occasional 502s from the load balancer when all instances (or request instance) OOM-ed simultaneously.

Service was configured with 512 MB memory. Bumped to 1024 BM but still saw OOM-ing, but now less frequently.

image

2026-02-27 14:00:00 LOCAL: Upgraded axiom-go version from v0.26 (I think?) to v0.30.

image

10:00 ish: Memory bump 512M -> 1G
Until 12:45: Using axiomhq/axiom-go.
Until 14:50: Using roessland@18011dd
After 14:50: Also using roessland@13ca2a6

This was a bit worrying since we have other more important services also using axiom-go (but luckily hadn't upgraded version yet).

@lukasmalkmus

Copy link
Copy Markdown
Collaborator

FYI this is how I think it could/should look like: #413.

@roessland roessland marked this pull request as draft March 2, 2026 16:23
@lukasmalkmus

Copy link
Copy Markdown
Collaborator

@roessland Sorry, I forgot to follow up on this. We went ahead and merged #413 a while ago. Do you still want to work on something in this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants