proxy: add opt-in immediate batch forwarding - #147
Open
HashimTheArab wants to merge 11 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe standalone RakNet proxy gains opt-in batch forwarding with capability checks, immediate batch writes, ordered packet handling, and transfer-aware backend processing. Tests cover the new behavior, while Go dependencies and setup documentation are updated. ChangesStandalone batch forwarding
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ProxySession
participant Backend
Client->>ProxySession: ReadBatch
ProxySession->>ProxySession: Rewrite and handle packets
ProxySession->>Backend: WritePacketImmediate
Backend->>ProxySession: ReadBatch
ProxySession->>ProxySession: Handle transfer packet
ProxySession->>Client: WritePacketImmediate
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
example/default/default.go (1)
29-32: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMake the example’s opt-in behavior explicit.
EnableBatchForwardingdefaults to false, but this checked-in example enables it, so users following the documented startup command receive immediate forwarding and its CPU/bandwidth trade-off. Either document that the example intentionally opts in or keep the flag false if this example is meant to represent defaults.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/default/default.go` around lines 29 - 32, Clarify the intended default behavior in the example configuration around EnableBatchForwarding: either remove the explicit true value so the example preserves the field’s false default, or add nearby documentation explaining that the example intentionally opts into immediate forwarding and its CPU/bandwidth trade-off.integration/proxy/proxy_test.go (1)
322-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
ReadBatch/WritePacketImmediatelogic betweenfakeBatchBackendandfakeBatchClient.Both fakes implement identical queue-draining
ReadBatchand copy-on-writeWritePacketImmediatebodies. Extracting a shared embeddable recorder removes the duplication.♻️ Proposed refactor: shared batch recorder
+type batchRecorder struct { + batches [][]packet.Packet + immediate [][]packet.Packet +} + +func (b *batchRecorder) ReadBatch() ([]packet.Packet, error) { + if len(b.batches) == 0 { + return nil, io.EOF + } + batch := b.batches[0] + b.batches = b.batches[1:] + return batch, nil +} + +func (b *batchRecorder) WritePacketImmediate(packets ...packet.Packet) error { + b.immediate = append(b.immediate, append([]packet.Packet(nil), packets...)) + return nil +} + type fakeBatchBackend struct { *fakeBackend - batches [][]packet.Packet - immediate [][]packet.Packet + batchRecorder } func newFakeBatchBackend() *fakeBatchBackend { return &fakeBatchBackend{fakeBackend: &fakeBackend{}} } -func (f *fakeBatchBackend) ReadBatch() ([]packet.Packet, error) { - if len(f.batches) == 0 { - return nil, io.EOF - } - batch := f.batches[0] - f.batches = f.batches[1:] - return batch, nil -} - -func (f *fakeBatchBackend) WritePacketImmediate(packets ...packet.Packet) error { - f.immediate = append(f.immediate, append([]packet.Packet(nil), packets...)) - return nil -}Apply the analogous change to
fakeBatchClient(drop its ownReadBatch/WritePacketImmediateand embedbatchRecorderinstead).Also applies to: 358-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration/proxy/proxy_test.go` around lines 322 - 345, Extract the shared queue-draining ReadBatch and copy-on-write WritePacketImmediate behavior into an embeddable batchRecorder type. Embed batchRecorder in fakeBatchBackend and fakeBatchClient, remove their duplicate method implementations and redundant state, and preserve the existing EOF and packet-recording behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@example/default/default.go`:
- Around line 29-32: Clarify the intended default behavior in the example
configuration around EnableBatchForwarding: either remove the explicit true
value so the example preserves the field’s false default, or add nearby
documentation explaining that the example intentionally opts into immediate
forwarding and its CPU/bandwidth trade-off.
In `@integration/proxy/proxy_test.go`:
- Around line 322-345: Extract the shared queue-draining ReadBatch and
copy-on-write WritePacketImmediate behavior into an embeddable batchRecorder
type. Embed batchRecorder in fakeBatchBackend and fakeBatchClient, remove their
duplicate method implementations and redundant state, and preserve the existing
EOF and packet-recording behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5999f9a-1f7e-4b4f-b896-2091ec6f9410
⛔ Files ignored due to path filters (2)
example/default/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
docs/setup.mdexample/default/default.goexample/default/go.modgo.modintegration/proxy/proxy.gointegration/proxy/proxy_test.go
ethaniccc
approved these changes
Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When enabled, forwarding no longer waits for the independent 50 ms flush timers. This removes 0–50 ms of buffering in each direction (about 50 ms average round-trip reduction, depending on timer alignment). The default remains disabled so existing users retain 50 ms coalescing and its CPU/compression benefits.
Verification
go test ./...(cd example/default && go test ./...)go test -race ./integration/proxygo mod tidy -diffin the root and example modulegit diff --checkSummary by CodeRabbit
New Features
Documentation