sklochkov2 opened a new pull request, #19754:
URL: https://github.com/apache/druid/pull/19754
### Description
Even though AI was used extensively to create this PR, please do not dismiss
it as another case of AI slop. The patches have been battle-tested.
The netty 3.10 -> 4.1 migration code has been running in production for
about 7 months - albeit as part of an earlier Druid version (31.0.2). The
migration was performed as part of meeting the mandatory CVE remediation
deadlines.
This PR contains the same tested patchset, adapted for druid-39-CURRENT.
The aim is zero behaviour changes compared to netty 3 code.
Co-Authored-By: Claude 4.5 (original patchset), Claude 4.8 (adapting it to
druid-39-CURRENT).
<hr>
##### Key changed/added classes in this PR
Core HTTP client (`processing`):
* `NettyHttpClient` — ported to the Netty 4 inbound handler model
(`channelRead`/`HttpContent`
rather than `messageReceived`/`HttpChunk`); also surfaces failed
`DecoderResult`s as exceptions,
which Netty 4 reports on the message instead of throwing.
* `NettyHttpClient.TimerReadTimeoutHandler` (**added**) — replaces Netty's
`ReadTimeoutHandler`,
scheduling read timeouts on the shared `HashedWheelTimer` instead of the
channel's event loop.
* `ChannelResourceFactory` — Netty 4 `Bootstrap`-based pooled connection
factory; the proxy
`CONNECT` handshake is now explicitly bounded (it previously had no
timeout and could block
callers of `NettyHttpClient#go` indefinitely).
* `HttpClientInit` — builds a `Bootstrap` + `NioEventLoopGroup` in place of
Netty 3's
`NioClientBossPool`/`NioWorkerPool`.
* `HttpClientPipelineFactory` — now extends
`ChannelInitializer<SocketChannel>` instead of
implementing `ChannelPipelineFactory`.
* `HttpResponseHandler` — **breaking API change**: `handleChunk` now takes
`io.netty.handler.codec.http.HttpContent` instead of
`org.jboss.netty...HttpChunk`. Extensions
implementing this interface must be updated.
* `Request` — `ChannelBuffer`/`HeapChannelBufferFactory` to
`ByteBuf`/`Unpooled`, and Netty's
`Base64` to `java.util.Base64`.
* `SequenceInputStreamResponseHandler` — `ByteBuf` lifecycle handling, so
retained buffers are
released rather than leaked when the consumer is interrupted.
* `HttpClientConfig`, `StatusResponseHandler`, `BytesFullResponseHandler`,
`InputStreamResponseHandler`, `InputStreamFullResponseHandler`,
`StringFullResponseHandler`,
`FrameFileHttpResponseHandler`
Server / RPC:
* `DirectDruidClient`, `DataServerResponseHandler`, `ServiceClientImpl`,
`StandardRetryPolicy`,
`LookupCoordinatorManager`, `HttpLoadQueuePeon`,
`IgnoreHttpResponseHandler`
* `WorkerHolder` (indexing-service), `SketchResponseHandler`
(multi-stage-query)
Build / dependencies:
* Root `pom.xml` (Netty 4.1 BOM, drops the `netty3.version` property and
the `io.netty:netty`
artifact, adds `requireUpperBoundDeps` exclusions), 15 module poms
declaring the per-feature
Netty 4 artifacts they use, and `licenses.yaml`.
Tests:
* `TimerReadTimeoutHandlerTest` (**added**) — `EmbeddedChannel`-based
coverage of the read-timeout
handler's scheduling, rescheduling, and teardown paths.
* `JankyServersTest` — adds `testSilentProxyServer`, covering a proxy that
accepts but never
answers `CONNECT`.
* `FriendlyServersTest` — mock proxy no longer self-destructs by calling
`Assert.fail` off-thread.
The remaining main-source files are mechanical API renames (`getStatus()` to
`status()`,
`HttpHeaders.Names.*` to `HttpHeaderNames.*`, `ChannelBuffer` to `ByteBuf`)
with no behavior change.
<hr>
<!-- Check the items by putting "x" in the brackets for the done things. Not
all of these items apply to every PR. Remove the items which are not done or
not relevant to the PR. None of the items from the checklist below are strictly
necessary, but it would be very helpful if you at least self-review the PR. -->
This PR has:
- [x ] been self-reviewed.
- [ ] using the [concurrency
checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md)
(Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [ x ] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
