Skip to content

fix(share/shwap/p2p/shrex/peers): disconnect peers that fail every request - #5146

Draft
renaynay wants to merge 3 commits into
celestiaorg:feature/shrex-eds-optimisationfrom
renaynay:rene/shrex-peer-strike-limit
Draft

fix(share/shwap/p2p/shrex/peers): disconnect peers that fail every request#5146
renaynay wants to merge 3 commits into
celestiaorg:feature/shrex-eds-optimisationfrom
renaynay:rene/shrex-peer-strike-limit

Conversation

@renaynay

@renaynay renaynay commented Aug 3, 2026

Copy link
Copy Markdown
Member

When a peer is put on cooldown, it can re-enter peer rotation almost immediately (3s) after they were put on cooldown, and with a 5-peer pool, this could happen frequently (wasting requests on an less responsive peer).

This PR implements a solution to count consecutive failed requests per peer and, at peerStrikeLimit, remove the peer from the nodes pool and close the connection instead of cooling it down.

Rules:

  • A successful request clears the count, so only peers that fail every request are kicked.
  • The disconnect makes discovery discard the peer and back off from redialing it for 10 minutes, while its 1s loop looks for a replacement, so the peer stays out of rotation for minutes rather than
    seconds.
  • Strikes are held in a bounded LRU as peers churn.

@codecov-commenter

codecov-commenter commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.23077% with 12 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feature/shrex-eds-optimisation@00e2029). Learn more about missing BASE report.

Files with missing lines Patch % Lines
share/shwap/p2p/shrex/peers/metrics.go 27.27% 8 Missing ⚠️
share/shwap/p2p/shrex/peers/manager.go 85.18% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@                        Coverage Diff                        @@
##             feature/shrex-eds-optimisation    #5146   +/-   ##
=================================================================
  Coverage                                  ?   37.26%           
=================================================================
  Files                                     ?      311           
  Lines                                     ?    21626           
  Branches                                  ?        0           
=================================================================
  Hits                                      ?     8058           
  Misses                                    ?    12555           
  Partials                                  ?     1013           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@renaynay

Copy link
Copy Markdown
Member Author

Concerns with this PR:

  • With ConcurrencyLimit 16, several concurrent requests to one peer can each fail and each observe a count at or above the limit before resetStrikes inside kickPeer lands. Every one calls kickPeer.
  • Kick closes a connection the whole node shares host-wide
  • Strikes count things that are not just peer faults
  • Strikes can empty out peer pool - and the archival node pool is not healthy enough on any network to combat that risk

Re-running archival benchmarks without this commit to see if had demonstrable impact on the shrex-eds speed-up

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.

2 participants