refactor(p2p,eth): replace pair-peer with BFT write priority, close #2359#2360
refactor(p2p,eth): replace pair-peer with BFT write priority, close #2359#2360gzliudan wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes the XDPoS pair-peer dual-connection mechanism and replaces its BFT-latency goal with per-peer high/low priority write scheduling in the p2p message path.
Changes:
- Removes pair-peer state, duplicate-connection allowances, and eth pair writer fallback paths.
- Adds
PriorityMsgWriter/SendPriorityand routes Vote, Timeout, and SyncInfo messages through the high-priority lane. - Updates dial/server tests and adds priority scheduler tests for preemption, starvation guard, and fallback behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
p2p/server.go |
Rejects duplicate NodeID connections and removes pair-peer tracking. |
p2p/server_test.go |
Updates server peer tracking tests for single-peer semantics. |
p2p/peer.go |
Adds high/low write request queues and priority arbitration. |
p2p/peer_test.go |
Removes pair-peer disconnect behavior test. |
p2p/peer_priority_test.go |
Adds tests for priority write scheduling behavior. |
p2p/peer_error.go |
Removes ErrAddPairPeer. |
p2p/message.go |
Adds SendPriority helper with fallback to normal writes. |
p2p/dial.go |
Restores duplicate peer dial rejection. |
p2p/dial_test.go |
Updates dial state expectations for one connection per NodeID. |
eth/peer.go |
Removes pairRw and uses priority sends for BFT messages. |
eth/handler.go |
Simplifies peer registration now that pair peers are removed. |
4d8709c to
3771f6b
Compare
bb807f4 to
a37847c
Compare
43397e7 to
8e66ebd
Compare
8e66ebd to
4f92775
Compare
4f92775 to
77c08ce
Compare
77c08ce to
4548fb3
Compare
Proposed changes
This PR close #2359.
Drop the per-peer dual TCP (pair-peer) mechanism and restore the upstream devp2p semantics of one connection per NodeID. To preserve the original goal of low BFT write latency, introduce a two-level write priority in the per-peer write scheduler so XDPoS v2 consensus messages (VoteMsg, TimeoutMsg, SyncInfoMsg) preempt block/tx traffic at the next write boundary.
p2p: remove Peer.pairPeer/PairPeer/SetPairPeer/ClearPairPeer, ErrAddPairPeer, and the pair branches in Server.encHandshakeChecks/addpeer/removePeerTracking and dialstate.checkDial. Keep the DiscPairPeerStop enum slot for wire compatibility with peers that still send it.
p2p: add PriorityMsgWriter interface and SendPriority helper. Rework Peer.run write arbitration: replace the single writeStart token with buffered hi/lo writeSlot request queues; non-blocking hi-bias when idle, blocking select on both otherwise, and a starvation guard that forces one lo through after writePriorityStarveLimit (16) consecutive hi writes.
eth: drop pairRw from peer and the ErrAddPairPeer fast-path in ProtocolManager.handle; collapse the pair fallback in peerSet.Register. Route SendVote/SendTimeout/SendSyncInfo through p2p.SendPriority.
Tests: revert p2p/dial_test.go and p2p/server_test.go to upstream geth expectations, drop pair-only cases, and add p2p/peer_priority_test.go covering hi/lo preemption, the starvation guard, and the non-priority fallback path.
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that