Velocity
Velocity copied to clipboard
Add support for io_uring
This was already implemented in the https://github.com/PaperMC/Velocity/tree/experiment/io_uring branch, but I guess it didn't get merged due to the experimental status of Netty's implementation.
I think somebody tried io_uring on paper recently and it went sideways, so this should probably be a configurable thing, afaik the thing is still a bit of a moving implementation and has not really been called "stable" yet
I think somebody tried io_uring on paper recently and it went sideways, so this should probably be a configurable thing, afaik the thing is still a bit of a moving implementation and has not really been called "stable" yet
zlib packet compression seems to be broken on io_uring on PaperMC having tried this last week. Should double check if that's the case here @skbeh
@clrxbl I tested both libdeflate and Java compression, and both worked fine.
@electronicboy Added.
If you're gonna add kqueue, why not add support for it aswell while at it? Seems pretty trivial to add it. Unlikely to be used by most since it's macOS / FreeBSD only, but the dependency is there so why not?
@clrxbl The kqueue dependency will be removed once the new version of async-http-client is released.
Updating async-http-client appears to quietly break a lot of my proxy plugins, one public one being ViaVersion. Which makes me wonder whether this is a change that should be made to a Velocity 3.2.x release?
- definitely not comfortable bumping to an early snapshot release of a library mid release cycle
- We shouldn't be defaulting the proxy to use an early version of a kernel mechanism that is still not considered stable, especially when many hosts are still using older kernels in which probably have security issues still exposed with that mechanism
@electronicboy It seems that there's no sane way to configure AHC to use specific transportFactory. Also, we can't get the server instance in an enum class, so it is inconvenient to check configure file (velocity.toml).
Not sure why either of those matter?
@electronicboy I mean that I don't know how to make it possible to configure whether to enable it via configure file. AHC automatically checks transportFactory which will be used together with event loop group, but when the transportFactory is not in their predefined list, it will fail.
We're generally not going to update to a beta release of library mid cycle, so what AHC uses in its beta release is pretty much 100% irrelevant
@electronicboy That's what AHC had been always using: https://github.com/AsyncHttpClient/async-http-client/blob/e7edd2c378fb4487be11c47bd624034094c2ae24/client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java#L158
In their 2.x versions, they did not aware of io_uring.
as it stands, this PR is pretty much blocked;
We can't justify jumping a library towards a beta release midcycle, so that's generally a no-go; We could stick to 2.x and just not use a shared worker group if using io_uring, but, not sure
io_uring should also be opt in, not opt out
@electronicboy The option is made to opt in and the Kqueue dependency is removed.
I think somebody tried io_uring on paper recently and it went sideways, so this should probably be a configurable thing, afaik the thing is still a bit of a moving implementation and has not really been called "stable" yet
zlib packet compression seems to be broken on io_uring on PaperMC having tried this last week. Should double check if that's the case here @skbeh
this is due to timing the compression packet sending incorrectly on io_uring. we use io_uring in prod without issues (both on server & velocity). will likely push a PR to Paper with this fixed later
fwiw, async http client has been removed from velocity, I have no idea what to do on the basis of this PR, I generally have little investment into IO_URING, so, I'm not sure what the other contributors thing, at the very least, having the thing opt-in at least makes it mergable to me
We'll likely do #1380 instead, where the io_uring transport becomes stable, and we make it the default for systems that support it.