Skip to content

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

Merged
normanmaurer merged 1 commit into
netty:4.1from
Scottmitch:writable
May 22, 2023
Merged

Channel#bytesBefore[un]writable off by 1#13389
normanmaurer merged 1 commit into
netty:4.1from
Scottmitch:writable

Conversation

@Scottmitch

Copy link
Copy Markdown
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
Copy Markdown
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
@normanmaurer

Copy link
Copy Markdown
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.

2 participants