Skip to content

Allow multiple concurrent allocations per peer per context#9599

Draft
Copilot wants to merge 4 commits intomasterfrom
copilot/allow-multiple-allocations-per-peer
Draft

Allow multiple concurrent allocations per peer per context#9599
Copilot wants to merge 4 commits intomasterfrom
copilot/allow-multiple-allocations-per-peer

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 30, 2025

Summary

This PR implements support for allocating the same peer multiple times per allocation context, allowing peers to serve multiple concurrent requests.

Changes Made

  • Added MaxAllocationsPerPeerPerContext configuration parameter (default: 2) in ISyncConfig and SyncConfig
  • Modified PeerInfo to track allocation counts per context using ConcurrentDictionary instead of binary flags
  • Updated CanBeAllocated() to check allocation count against configured limit
  • Updated TryAllocate() to increment allocation count per context
  • Updated Free() to decrement allocation count and clear allocated flag when count reaches 0
  • Updated SyncPeerPool constructor to accept and pass configuration to PeerInfo
  • Added 5 new tests for multiple allocation scenarios
  • Updated 7 existing tests to work with new behavior
  • All 2067 synchronization tests passing
  • Code formatting verified
  • Addressed code review feedback

Benefits

✅ Peers can serve multiple requests simultaneously
✅ Improves sync efficiency for low-peer networks (e.g., base-mainnet)
✅ Configurable limit allows network-specific tuning
✅ Backward compatible (set to 1 to restore old behavior)

Technical Details

The implementation uses a per-context allocation counter instead of binary flags. When allocating:

  1. Checks if allocation count < maxAllocationsPerContext (default: 2)
  2. Increments the count for each context being allocated
  3. Sets the allocated flag for the context

When freeing:

  1. Decrements the count for each context being freed
  2. Clears the allocated flag only when count reaches 0

This allows the same peer to be allocated up to maxAllocationsPerContext times for each context (Headers, Bodies, Receipts, etc.), significantly improving throughput.

Original prompt

This section details on the original issue you should resolve

<issue_title>Allow allocating to the same peer multiple time.</issue_title>
<issue_description>Is your feature request related to a problem? Please describe.

  • Current version of SyncPeerPool.Allocate allow allocating a peer for an allocation context, but the same peer can only be allocated once.
  • This is slightly inefficient as the during the latency of the request, the peer is not serving the data.

Describe the solution you'd like

  • Allow multiple allocation per peer per allocation context. Make the limit configurable, set default to 2.

Describe alternatives you've considered

  • Increasing the peer count is inefficient.

Additional context

  • base-mainnet struggle to sync due to low number of peer.</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits October 30, 2025 06:03
…le limit

- Added MaxAllocationsPerPeerPerContext config parameter (default: 2)
- Modified PeerInfo to track allocation counts per context instead of binary flags
- Updated allocation/free logic to increment/decrement counts
- Updated tests to reflect new behavior where peers can be allocated multiple times
- This allows peers to serve multiple requests during latency periods, improving efficiency

Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com>
Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com>
Copilot AI changed the title [WIP] Allow allocating to the same peer multiple times Allow multiple concurrent allocations per peer per context Oct 30, 2025
Copilot AI requested a review from asdacap October 30, 2025 06:10
SyncPeerAllocation allocation1 = await ctx.Pool.Allocate(new BySpeedStrategy(TransferSpeedType.Headers, true));
SyncPeerAllocation allocation2 = await ctx.Pool.Allocate(new BySpeedStrategy(TransferSpeedType.Headers, true));
Assert.That(allocation2.Current, Is.Not.SameAs(allocation1.Current), "first");
// With multiple allocations per peer, both may point to same PeerInfo or different - both are valid
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot add more allocation in this test to complete the two slot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a1c3297. Added 2 more allocations to test all 4 slots (2 peers × 2 slots each).

[ConfigItem(Description = "_Technical._ Estimated max size of blocks in block processing queue before stop downloading.", DefaultValue = "200000000", HiddenFromDocs = true)]
long ForwardSyncBlockProcessingQueueMemoryBudget { get; set; }

[ConfigItem(Description = "The maximum number of concurrent allocations per peer per allocation context. Allows a peer to serve multiple requests simultaneously during request latency periods.", DefaultValue = "2")]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the mention of "request latency periods", its weird.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a1c3297. Simplified description to "Allows a peer to serve multiple requests simultaneously."

Assert.That(allocation2.HasPeer, Is.True);
Assert.That(allocation3.HasPeer, Is.False);
// With 2 awake peers and max 2 allocations per peer, we can have up to 4 allocations, so third allocation succeeds
Assert.That(allocation3.HasPeer, Is.True);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot a sleeping peer cannot allocate even if it has slot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a1c3297. Updated test to allocate 4 times (maxing out the 2 awake peers) and then verify the 5th allocation fails, confirming sleeping peers cannot be allocated even with available slots.

- Remove mention of "request latency periods" from config description
- Add more allocations in Can_borrow_many test to fully test 2 slots per peer
- Fix Does_not_allocate_sleeping_peers test to properly verify sleeping peers cannot be allocated

Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com>
Copilot AI requested a review from asdacap October 30, 2025 07:36
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.

Allow allocating to the same peer multiple time.

2 participants