Skip to content

fix: drain response body before signaling readiness in concurrent downloader to ensure connection reuse#407

Merged
SuperCoolPencil merged 4 commits intomainfrom
fix-keep-alive
Apr 23, 2026
Merged

fix: drain response body before signaling readiness in concurrent downloader to ensure connection reuse#407
SuperCoolPencil merged 4 commits intomainfrom
fix-keep-alive

Conversation

@SuperCoolPencil
Copy link
Copy Markdown
Member

@SuperCoolPencil SuperCoolPencil commented Apr 23, 2026

Greptile Summary

This PR fixes a connection-reuse bug in prewarmConnections where ready was signalled before the response body was drained and closed. Go's net/http transport only returns a connection to the idle pool after the body is fully consumed and closed, so signalling first meant consumers could start new requests before the connection was actually available, defeating prewarming. The fix drains and closes the body first, then signals — and the new TestPrewarmConnections_Reuse test validates this with httptrace.

Confidence Score: 5/5

Safe to merge — the change is minimal, correct, and directly validated by the new test.

The fix is a two-line reorder with a clear rationale backed by Go's transport contract. The prewarm request already uses bytes=0-0, so the body drained is a single byte and blocking the goroutine is negligible. The new test uses httptrace to confirm end-to-end connection reuse. No P0 or P1 issues found.

No files require special attention.

Important Files Changed

Filename Overview
internal/engine/concurrent/downloader.go Reorders body drain and close before the ready signal so the connection is guaranteed to be in the idle pool before consumers start making requests — correct fix for the connection-reuse bug.
internal/engine/concurrent/prewarm_reuse_test.go New test that uses httptrace.GotConnInfo.Reused to verify a prewarmed connection is actually reused by the next request; errors are properly handled with t.Fatalf.

Reviews (2): Last reviewed commit: "fix: comments" | Re-trigger Greptile

Comment thread internal/engine/concurrent/downloader.go Outdated
Comment thread internal/engine/concurrent/prewarm_reuse_test.go
@SuperCoolPencil SuperCoolPencil merged commit e4b72bf into main Apr 23, 2026
16 checks passed
@SuperCoolPencil SuperCoolPencil deleted the fix-keep-alive branch April 23, 2026 04:37
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.

1 participant