fix(writer): resolve memory bloat#654
Conversation
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
✅ Deploy Preview for teal-sprinkles-4c7f14 canceled.
|
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
by copilot (claude Sonnet 4) Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
|
Fix description by GitHub Copilot (Clause Sonnet 4): Memory Bloat Fix - Analysis and SolutionProblem SummaryMemory bloat was introduced in commit Root CauseThe issue is in the return singleflightValue{
body: customWriter.Buf.Bytes(), // ← PROBLEM HERE
headers: customWriter.Header().Clone(),
requestHeaders: rq.Header,
code: statusCode,
}, errWhy This Causes Memory Bloat
The FixCreate an explicit copy of the buffer data before returning it from singleflight: // Create a copy of the buffer to prevent memory retention
// when the buffer is returned to the pool
bodyCopy := make([]byte, customWriter.Buf.Len())
copy(bodyCopy, customWriter.Buf.Bytes())
return singleflightValue{
body: bodyCopy, // ← Now uses independent memory
headers: customWriter.Header().Clone(),
requestHeaders: rq.Header,
code: statusCode,
}, errWhy This Works
Files Modified
Performance ImpactMemory Impact
CPU Impact
Testing Recommendations
Alternative Approaches Considered1. Disable Buffer Pooling
2. Use sync.Pool for singleflightValue
3. Reference Counting on Buffers
ConclusionThis fix resolves the memory bloat by ensuring buffer pool memory is properly isolated from cached response data. The explicit copy is a small, predictable cost that restores proper memory management. |
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
I've used Amp to analyze the problem and produce this patch, but I've reviewed it before submission. The changes clearly refactor the closure logic into methods on the writer per use-case. Each of the introduced methods rely on a mutex to guard against reintroduction of the resolved race condition.
I've run
go mod vendorfor the traefik plugin so the fix propagates into it as well.Summary generated by Amp
Memory Consumption Issue Fixed
I've successfully identified and fixed the memory consumption issue introduced in commit
0e1579f1adc7c94f16852531c1c3658752c2e3d8.Problem: The commit replaced direct buffer operations with
handleBuffer(callback func(*bytes.Buffer))calls, which created function closures for every buffer operation, significantly increasing memory allocations.Solution: Replaced the closure-based approach with specific, optimized methods:
resetBuffer()- Direct mutex-protected buffer resetcopyToBuffer(src io.Reader)- Direct mutex-protected io.Copy operationresetAndCopyToBuffer(src io.Reader)- Combined reset and copy operationFiles Updated:
pkg/middleware/writer.gopkg/middleware/middleware.goplugins/traefik/override/middleware/writer.goplugins/traefik/override/middleware/middleware.goplugins/traefik/vendor/github.com/darkweak/souin/pkg/middleware/writer.goplugins/traefik/vendor/github.com/darkweak/souin/pkg/middleware/middleware.goThis eliminates closure overhead while maintaining thread safety, significantly reducing memory consumption.