Skip to content
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

Channel#bytesBefore[un]writable off by 1 #13389

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

Scottmitch
Copy link
Member

Motivation:
Channel#bytesBefore[un]writable methods are described as returning the number of bytes until writability state changes. The ChannelConfig buffer high/low water marks are described as the thresholds must be exceeded before writability changes. The implementation of bytesBefore[un]writable methods return the number of bytes until the threshold is meet but not exceeded. If implementations depend upon this to drive readability they may hang.

Modifications:

  • Channel#bytesBefore[un]writable implementations in http/2 and ChannelOutboundBuffer increment the value by 1 to return how much is required to cross the water mark therefore trigger a change in writability.

Motivation:
Channel#bytesBefore[un]writable methods are described as returning the
number of bytes until writability state changes. The ChannelConfig
buffer high/low water marks are described as the thresholds must
be exceeded before writability changes. The implementation of
bytesBefore[un]writable methods return the number of bytes until the
threshold is meet but not exceeded. If implementations depend upon this
to drive readability they may hang.

Modifications:
- Channel#bytesBefore[un]writable implementations in http/2 and
ChannelOutboundBuffer increment the value by 1 to return how much
is required to cross the water mark therefore trigger a change in
writability.
@Scottmitch
Copy link
Member Author

build failure attributed to #13390

@normanmaurer normanmaurer added this to the 4.1.93.Final milestone May 22, 2023
@normanmaurer normanmaurer merged commit 0e8b025 into netty:4.1 May 22, 2023
14 checks passed
@normanmaurer
Copy link
Member

@Scottmitch thanks a lot!

@Scottmitch Scottmitch deleted the writable branch May 22, 2023 13:32
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Jun 28, 2023
### What changes were proposed in this pull request?
This pr aims to upgrade netty from 4.1.92 to 4.1.93.

### Why are the changes needed?
1.v4.1.92 VS v4.1.93
netty/netty@netty-4.1.92.Final...netty-4.1.93.Final

2.The new version brings some bug fix, eg:
- Reset byte buffer in loop for AbstractDiskHttpData.setContent ([#13320](netty/netty#13320))
- OpenSSL MAX_CERTIFICATE_LIST_BYTES option supported ([#13365](netty/netty#13365))
- Adapt to DirectByteBuffer constructor in Java 21 ([#13366](netty/netty#13366))
- HTTP/2 encoder: allow HEADER_TABLE_SIZE greater than Integer.MAX_VALUE ([#13368](netty/netty#13368))
- Upgrade to latest netty-tcnative to fix memory leak ([#13375](netty/netty#13375))
- H2/H2C server stream channels deactivated while write still in progress ([#13388](netty/netty#13388))
- Channel#bytesBefore(un)writable off by 1 ([#13389](netty/netty#13389))
- HTTP/2 should forward shutdown user events to active streams ([#13394](netty/netty#13394))
- Respect the number of bytes read per datagram when using recvmmsg ([#13399](netty/netty#13399))

3.The release notes as follows:
- https://netty.io/news/2023/05/25/4-1-93-Final.html

4.Why not upgrade to `4-1-94-Final` version?
Because the return value of the 'threadCache()' (from `PoolThreadCache` to `PoolArenasCache`) method of the netty Inner class used in the 'arrow memory netty' version '12.0.1' has changed and belongs to break change, let's wait for the upgrade of the 'arrow memory netty' before upgrading to the '4-1-94-Final' version.

The reference is as follows:
https://github.com/apache/arrow/blob/6af660f48472b8b45a5e01b7136b9b040b185eb1/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java#L164
https://github.com/netty/netty/blob/da1a448d5bc4f36cc1744db93fcaf64e198db2bd/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L732-L736

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass GA.

Closes #41681 from panbingkun/upgrade_netty.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
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.

None yet

2 participants