Add unix domain socket transport in netty 4.x via JDK16+ #13965

honglh · 2024-04-10T15:09:27Z

Motivation:

Our use case requires using nio transport for unix-domain-socket (UDS) in netty 4.x.
Although netty 5.x supports UDS, it is is not compatible with 4.x. For us and probably many users, having to migrate to 5.x in order to use UDS is not a viable option.

#10991 says it is supported in 5.x and explains the complexity of doing so in 4.x.
Here, I am proposing a simpler way of adding to 4.x using JDK16+ NIO transport.

The simplicity is achieved by allowing NioServerSocketChannel to continue to support INET only and use new classes for UDS.

This UDS support in this (4.x) PR is incompatible with 5.x. But since 5.x is incompatible with 4.x anyway, I don't this is is an issue. It is easy for user to change to use UDS from 5.x when it comes to it.

Thus in summary, this change

Simple to add UDS support in 4.x
Easy to migrate UDS application to 5.x

Modification:

Only a few APIs are overloaded to support NIO UDS class.
Most changes are added new files.

Describe the modifications you've done.

The NioServerSocketChannel and NioSocketChannel supports only INET channels. In this PR mirroring classes for NIO UDS NioDomainServerSocketChannel and NioDomainSocketChannel are added.

I modified EchoServer.java to use newly created NioDomainServerSocketChannel to show the difference creating a EchoServer with INET socket and with UDS socket.

Result:

This is a draft PR, so POM changes to use JDK16+ to build netty-transport are not included.
The EchoServer with UDS works as expected and same as it is using INET

Fixes #.

If there is no issue then describe the changes introduced by this PR.

yawkat · 2024-04-12T06:56:41Z

@chrisvest @normanmaurer

This patch was created by @honglh with some feedback from me. @honglh works for Oracle, though on a different team than me. They want to get the Micronaut HTTP server (and thus netty) working with domain sockets without epoll.

I will help on cleaning up this PR (commit message, compatibility, javadocs, builds, JDK version checks, CLA…) assuming you agree that the general approach is acceptable in netty 4. While this approach is not compatible with the existing kqueue and epoll domain socket implementations (this patch uses the JDK UnixDomainSocketAddress rather than the netty DomainSocketAddress that kqueue and epoll use), I think this tradeoff is reasonable to keep project layout changes to a minimum. Other than that I have no concerns with this approach.

normanmaurer · 2024-04-12T19:16:09Z

@yawkat let me have a look and come back to you early next week

normanmaurer · 2024-04-18T14:23:52Z

@honglh @yawkat I pushed some changes which I think makes things more correct and reduces some noise. WDYT ?

normanmaurer · 2024-04-18T14:24:39Z

/cc @chrisvest

yawkat · 2024-04-18T16:01:22Z

Thanks @normanmaurer ! Mostly looks fine to me. I've tested it locally and it works, except for the use case we already had a problem with in #13877. When directly constructing a NioDomainServerSocketChannel from a ServerSocketChannel (eg from System.inheritedChannel), and then only registering (not binding) the channel (eg systemd socket activation use case), the channel remains inactive because bound is never set to true. So we probably again either need a setter or pass in the bound value through the constructor.

Another nit: NioDomainServerSocketChannel vs EpollServerDomainSocketChannel, the server and domain are flipped.

normanmaurer · 2024-04-18T16:04:04Z

@yawkat fixed the naming... let me check what we can do about the "bound" stuff here.

yawkat · 2024-04-19T05:16:49Z

@normanmaurer the link at the bottom of https://docs.google.com/forms/d/e/1FAIpQLSfXTK6SnWWFbR50DFhoZq2floCFOUBH3kG8sZP77im5Rknctg/viewform?formkey=dG9wTmhoeGNGd1MtdFdtVXl4TlVSNlE6MQ to change the list of employees is broken

normanmaurer · 2024-04-19T06:47:16Z

@yawkat I just pushed a commit that will allow to correctly detect if a channel was bound before. Also added some tests

yawkat · 2024-04-19T07:09:27Z

You haven't

normanmaurer · 2024-04-19T07:15:32Z

@yawkat 🤦 now I did :D

yawkat · 2024-04-19T07:24:25Z

Thanks! I tried locally, and it works out of the box now.

Otherwise this PR LGTM, we just need to get the CLA sorted on our end.

normanmaurer · 2024-04-19T09:41:26Z

Thanks! I tried locally, and it works out of the box now.

Otherwise this PR LGTM, we just need to get the CLA sorted on our end.

@trustin will fix it.

normanmaurer · 2024-04-19T16:41:42Z

@chrisvest PTAL

chrisvest

Looks good. I had some comments, though.

transport/src/main/java/io/netty/channel/socket/nio/NioDomainSocketChannel.java

transport/src/main/java/io/netty/channel/socket/nio/NioServerDomainSocketChannel.java

transport/src/test/java/io/netty/channel/socket/nio/NioServerDomainSocketChannelTest.java

trustin · 2024-04-22T06:28:02Z

Thanks for your patience, @yawkat. Here's the employee list update form: https://docs.google.com/forms/d/e/1FAIpQLScnQObZBItRc5L9EC_DPB3WVdUA7K_uIgC-I7gxqq3G4JGySQ/viewform

normanmaurer · 2024-04-22T15:35:14Z

@yawkat @honglh let me know once you think we can pull this in

yawkat · 2024-04-22T17:59:40Z

Will do, I've sent the CLA upstairs :)

yawkat · 2024-04-24T16:03:03Z

CLA should be submitted. Note that the Oracle Point of Contact also changed.

normanmaurer · 2024-04-26T22:26:27Z

Thanks all !

### Motivation: Our use case requires using nio transport for unix-domain-socket (UDS) in netty 4.x. Although netty 5.x supports UDS, it is is not compatible with 4.x. For us and probably many users, having to migrate to 5.x in order to use UDS is not a viable option. ### Modification: Add new classes to support domain UDS with NIO. ### Result: UDS is supported when using the NIO transport when java 16+ is used. Co-authored-by: Norman Maurer <[email protected]>

yawkat · 2024-04-29T06:39:53Z

Thanks!

From netty/netty#13965

* fix(deps): update netty monorepo to v4.1.110.final * NIO domain socket support From netty/netty#13965 --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

### What changes were proposed in this pull request? The pr aims to upgrade `netty` from `4.1.109.Final` to `4.1.110.Final`. ### Why are the changes needed? - https://netty.io/news/2024/05/22/4-1-110-Final.html This version has brought some bug fixes and improvements, such as: Fix Zstd throws Exception on read-only volumes (netty/netty#13982) Add unix domain socket transport in netty 4.x via JDK16+ ([#13965](netty/netty#13965)) Backport #13075: Add the AdaptivePoolingAllocator ([#13976](netty/netty#13976)) Add no-value key handling only for form body ([#13998](netty/netty#13998)) Add support for specifying SecureRandom in SSLContext initialization ([#14058](netty/netty#14058)) - https://github.com/netty/netty/issues?q=milestone%3A4.1.110.Final+is%3Aclosed ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46744 from panbingkun/SPARK-48420. Authored-by: panbingkun <[email protected]> Signed-off-by: yangjie01 <[email protected]>

add nio uds transport

02e2e9a

honglh changed the title ~~add nio uds transport~~ Add nio uds transport Apr 10, 2024

honglh mentioned this pull request Apr 11, 2024

Leverage improvements in openjdk 16 #10991

Closed

honglh changed the title ~~Add nio uds transport~~ Add unix domain socket transport in netty 4.x via JDK16+ Apr 11, 2024

Rework uds impl

53637fc

normanmaurer added this to the 4.1.110.Final milestone Apr 18, 2024

normanmaurer added 2 commits April 18, 2024 16:58

Fix tests

519b920

more fixes

a73f2b6

Fix naming

eccc80d

Correctly detect if channel is already bound

74f5bd6

Fix copyright year

814177f

naming

9273ea8

normanmaurer marked this pull request as ready for review April 19, 2024 16:41

normanmaurer requested a review from chrisvest April 19, 2024 16:41

chrisvest requested changes Apr 19, 2024

View reviewed changes

Address comments

6370118

normanmaurer requested a review from chrisvest April 20, 2024 13:40

Fix test

e965f65

normanmaurer added 3 commits April 22, 2024 11:16

Delete file

303dba1

Fix version check

60c4d21

Address comments

44f24e1

violetagg approved these changes Apr 24, 2024

View reviewed changes

normanmaurer merged commit 420b037 into netty:4.1 Apr 26, 2024

yawkat added a commit to micronaut-projects/micronaut-core that referenced this pull request May 23, 2024

NIO domain socket support

702dd1e

From netty/netty#13965

yawkat mentioned this pull request May 23, 2024

NIO domain socket support micronaut-projects/micronaut-core#10852

Merged

panbingkun mentioned this pull request May 26, 2024

[SPARK-48420][BUILD] Upgrade netty to 4.1.110.Final apache/spark#46744

Closed

mkurz mentioned this pull request Jun 5, 2024

Support Unix domain sockets in Netty backend playframework/playframework#12703

Open

Uh oh!

Add unix domain socket transport in netty 4.x via JDK16+ #13965

Add unix domain socket transport in netty 4.x via JDK16+ #13965

Uh oh!

Conversation

honglh commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation:

Modification:

Describe the modifications you've done.

Result:

Uh oh!

yawkat commented Apr 12, 2024

Uh oh!

normanmaurer commented Apr 12, 2024

Uh oh!

normanmaurer commented Apr 18, 2024

Uh oh!

normanmaurer commented Apr 18, 2024

Uh oh!

yawkat commented Apr 18, 2024

Uh oh!

normanmaurer commented Apr 18, 2024

Uh oh!

yawkat commented Apr 19, 2024

Uh oh!

normanmaurer commented Apr 19, 2024

Uh oh!

yawkat commented Apr 19, 2024

Uh oh!

normanmaurer commented Apr 19, 2024

Uh oh!

yawkat commented Apr 19, 2024

Uh oh!

normanmaurer commented Apr 19, 2024

Uh oh!

normanmaurer commented Apr 19, 2024

Uh oh!

chrisvest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trustin commented Apr 22, 2024

Uh oh!

normanmaurer commented Apr 22, 2024

Uh oh!

yawkat commented Apr 22, 2024

Uh oh!

yawkat commented Apr 24, 2024

Uh oh!

normanmaurer commented Apr 26, 2024

Uh oh!

yawkat commented Apr 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

honglh commented Apr 10, 2024 •

edited

Loading