[xxo1shine]

What does this PR do?

Add pending-write accounting and timeout handling to improve TCP connection resource management.

• Set a 200 MiB pending-write threshold per connection.
• Trigger eviction of the connection with the most pending data when the shared total reaches 500 MiB.
• Close connections when a write remains incomplete for 10 seconds.
• Reserve framed payload bytes before event-loop submission and release them when writes complete or fail.

The shared threshold triggers asynchronous eviction rather than enforcing a hard global admission limit.

Why are these changes required?

This PR has been tested by:

• Unit Tests
• Manual Testing

Follow up

Extra details

[SeriousCoding789]

Nice work! Could you share how the 200 MiB per-connection limit in GLOBAL_BUDGET was chosen? I’d like to make sure it leaves enough headroom for normal block sync, so honest peers don’t get disconnected while syncing.

[xxo1shine]

The 200 MiB limit applies to outstanding outbound writes on a single connection. The budget is released when a write completes or fails, so it does not limit the cumulative amount of data transferred during sync.

Based on our production observations, blocks are typically around 100 KB. With at most 100 blocks requested per batch, that amounts to roughly 10 MB of block data, leaving substantial headroom below the 200 MiB threshold. Completed writes also continuously release the budget as data is sent, so normal syncing is generally not expected to hit this limit.

However, 100 KB is a typical size, not an upper bound. Consecutive large blocks or network congestion can still cause pending data to accumulate, and these edge cases need further validation.

[sunny-tron]

Thanks for working on this. Two things I noticed while reading the change:

1. Global eviction can pick an honest peer. selectVictim chooses the connection with the most pending bytes. During full-block sync, a single batch is up to 100 blocks of about 2 MB each, so an honest peer that is syncing from this node can easily be the largest one. The evicted peer then goes through the same disconnect() → channel.close() path as a peer that exceeded its own 200 MiB limit, so it is also banned for 60 s, and selectVictim does not treat trusted or configured active peers differently. Both cases also log the same pending outbound data limit exceeded line, without saying which limit fired or how many bytes were pending, which makes it hard to tell an evicted honest peer from a misbehaving one afterwards. Would it make sense to handle global eviction separately, for example closing without the ban, skipping or deprioritizing trusted/active peers, and logging the limit type together with the per-peer and total pending bytes?

2. The limits are hardcoded but depend on values defined elsewhere. The 200 MiB per-connection limit seems to be sized for one sync batch: MAX_BLOCK_FETCH_PER_PEER (100) × BLOCK_SIZE (2,000,000 bytes) ≈ 191 MiB, which leaves only about 9 MiB of headroom, and nothing in the code ties the two together. If either value grows later, an honest full-block sync batch would exceed the limit right away. Likewise, the 500 MiB global limit is meant to protect direct memory, so it depends on how much direct memory the node has: with -XX:MaxDirectMemorySize=1g (as in the JDK 17 vmoptions) it is half of it, but with a different setting the ratio changes. Since p2p does not depend on the java-tron modules that define these values, it might be more robust to compute both limits on the java-tron side, from the sync batch size and the direct memory limit, and pass them in through P2pConfig instead of hardcoding them in WriteBufferHandler.

[SeriousCoding789]

One more thought on how the slow-peer case is detected. Netty's writability could make the limits both tighter and gentler on honest peers, if it is used on the producer side rather than as an eviction signal:

• With the default water marks (32/64 KiB), an honest peer receiving a sync batch stays unwritable for most of the batch, just like a peer that has stopped reading, so isWritable() alone cannot pick the right victim. What separates the two is progress: an honest peer keeps completing writes, while a stalled one completes none. If global eviction stays, choosing among unwritable channels the one whose last completed write is oldest, instead of the one with the most pending bytes, would avoid evicting a peer that is simply working through a large batch.
• Going one step further, setting a write buffer water mark on p2p channels (above one block, e.g. a few MiB) and having FetchInvDataMsgHandler pause serving a peer while its channel is not writable, then resume once it is writable again, would keep each connection's backlog near the high water mark. Pending outbound memory would then be roughly bounded by the number of connections × (high water mark + one max-size message), which can be derived from configuration, and the existing 10 s write timeout would only require a peer to read a few MiB in 10 s rather than a whole 100-block batch. A peer that stops reading would still be closed by the write timeout, and the global eviction would mostly become a safety net. Simply stopping the loop when the channel turns unwritable would not work, because the channel also becomes unwritable briefly on fast links once the socket buffer fills, so the remaining hashes would need to be kept and served later.

This is a bigger change than the current p2p-only approach, so it may fit better as a follow-up, but it seemed worth raising here since it addresses both the memory bound and the honest-peer eviction.