Repository navigation
fix(net): add outbound buffer limits and write timeouts #7071
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
xxo1shine
wants to merge
1
commit into
tronprotocol:release_v4.8.3
from
xxo1shine:fix/net-slow-reader-write-limits
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
139 changes: 139 additions & 0 deletions
139
p2p/src/main/java/org/tron/p2p/connection/WriteBufferHandler.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| package org.tron.p2p.connection; | ||
|
|
||
| import com.google.protobuf.CodedOutputStream; | ||
| import io.netty.buffer.ByteBuf; | ||
| import io.netty.channel.ChannelFuture; | ||
| import io.netty.channel.ChannelHandlerContext; | ||
| import io.netty.channel.ChannelPromise; | ||
| import io.netty.handler.timeout.WriteTimeoutHandler; | ||
| import java.nio.channels.ClosedChannelException; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.Set; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| /** Bounds pending TCP writes and closes connections whose writes stall. */ | ||
| @Slf4j(topic = "net") | ||
| final class WriteBufferHandler extends WriteTimeoutHandler { | ||
|
|
||
| private static final Budget GLOBAL_BUDGET = new Budget(200L * 1024 * 1024, 500L * 1024 * 1024); | ||
|
|
||
| private final Channel channel; | ||
| private final Budget budget; | ||
| // Guarded by budget, including sends originating outside the channel's event loop. | ||
| private long pendingBytes; | ||
| private boolean closing; | ||
|
|
||
| WriteBufferHandler(Channel channel) { | ||
| this(channel, GLOBAL_BUDGET); | ||
| } | ||
|
|
||
| WriteBufferHandler(Channel channel, Budget budget) { | ||
| super(10); | ||
| this.channel = channel; | ||
| this.budget = budget; | ||
| } | ||
|
|
||
| ChannelFuture writeAndFlush(ChannelHandlerContext ctx, ByteBuf data) { | ||
| // Reserve before Netty can enqueue a write task on the event loop. Include the frame prefix | ||
| // that protoPrepend adds later, after the payload has already been compressed by Channel.send. | ||
| long size = (long) data.readableBytes() | ||
| + CodedOutputStream.computeUInt32SizeNoTag(data.readableBytes()); | ||
| ChannelPromise promise = ctx.newPromise(); | ||
| promise.setUncancellable(); | ||
| if (!budget.reserve(this, size)) { | ||
| data.release(); | ||
| return promise.setFailure(new ClosedChannelException()); | ||
| } | ||
| promise.addListener(future -> budget.release(this, size)); | ||
| if (channel.isDisconnect()) { | ||
| data.release(); | ||
| return promise.setFailure(new ClosedChannelException()); | ||
| } | ||
| return ctx.writeAndFlush(data, promise); | ||
| } | ||
|
|
||
| private void disconnect() { | ||
| logger.warn("Close peer {}, pending outbound data limit exceeded", | ||
| channel.getInetSocketAddress()); | ||
| channel.close(); | ||
| } | ||
|
|
||
| /** Shared by TCP connections; the global limit triggers eviction rather than blocking I/O. */ | ||
| static final class Budget { | ||
|
|
||
| private final long peerLimit; | ||
| private final long globalLimit; | ||
| private final Set<WriteBufferHandler> peers = new LinkedHashSet<>(); | ||
| private long pendingBytes; | ||
| private WriteBufferHandler evicting; | ||
|
|
||
| Budget(long peerLimit, long globalLimit) { | ||
| this.peerLimit = peerLimit; | ||
| this.globalLimit = globalLimit; | ||
| } | ||
|
|
||
| private boolean reserve(WriteBufferHandler handler, long size) { | ||
| WriteBufferHandler victim; | ||
| synchronized (this) { | ||
| if (handler.closing || handler.channel.isDisconnect()) { | ||
| return false; | ||
| } | ||
| handler.pendingBytes += size; | ||
| pendingBytes += size; | ||
| peers.add(handler); | ||
| if (handler.pendingBytes > peerLimit) { | ||
| handler.closing = true; | ||
| victim = handler; | ||
| if (evicting == null) { | ||
| evicting = handler; | ||
| } | ||
| } else { | ||
| victim = selectVictim(); | ||
| } | ||
| } | ||
| // Closing may immediately complete writes and re-enter release(). Never close under the lock. | ||
| if (victim != null) { | ||
| victim.disconnect(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| private void release(WriteBufferHandler handler, long size) { | ||
| WriteBufferHandler victim; | ||
| synchronized (this) { | ||
| handler.pendingBytes -= size; | ||
| pendingBytes -= size; | ||
| if (handler.pendingBytes == 0) { | ||
| peers.remove(handler); | ||
| if (evicting == handler) { | ||
| evicting = null; | ||
| } | ||
| } | ||
| victim = selectVictim(); | ||
| } | ||
| if (victim != null) { | ||
| victim.disconnect(); | ||
| } | ||
| } | ||
|
|
||
| // Called with the budget lock held. Wait for the current victim's writes to be released | ||
| // before selecting another, even if multiple producers observe the global limit concurrently. | ||
| private WriteBufferHandler selectVictim() { | ||
| if (evicting != null || pendingBytes < globalLimit) { | ||
| return null; | ||
| } | ||
| WriteBufferHandler largest = null; | ||
| for (WriteBufferHandler candidate : peers) { | ||
| if (!candidate.closing && !candidate.channel.isDisconnect() | ||
| && (largest == null || candidate.pendingBytes > largest.pendingBytes)) { | ||
| largest = candidate; | ||
| } | ||
| } | ||
| if (largest != null) { | ||
| largest.closing = true; | ||
| evicting = largest; | ||
| } | ||
| return largest; | ||
| } | ||
| } | ||
| } | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Could you share how the 200 MiB per-connection limit in
GLOBAL_BUDGETwas chosen? I’d like to make sure it leaves enough headroom for normal block sync, so honest peers don’t get disconnected while syncing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.