Skip to content

Comments

Fix Netty deprecation warnings in transport-reactor-netty4 module#20429

Merged
reta merged 2 commits intoopensearch-project:mainfrom
fdesu:issues/reactor-netty-deprecation-warnings
Jan 16, 2026
Merged

Fix Netty deprecation warnings in transport-reactor-netty4 module#20429
reta merged 2 commits intoopensearch-project:mainfrom
fdesu:issues/reactor-netty-deprecation-warnings

Conversation

@fdesu
Copy link
Contributor

@fdesu fdesu commented Jan 16, 2026

Description

Similar to #20233 this PR fixes some of the deprecation warnings in the transport-reactor-netty4 module.

  1. More details on the NioEventLoopGroup -> MultiThreadIoEventLoopGroup migration can be found here https://netty.io/wiki/netty-4.2-migration-guide.html#new-best-practices.
  2. A trivial io.netty.handler.codec.http.HttpRequest# getUri -> io.netty.handler.codec.http.HttpRequest#uri

Netty-related deprecation warnings that are left:

  1. Usages of reactor.netty.tcp.SslProvider.SslContextSpec#sslContext(reactor.netty.tcp.SslProvider.ProtocolSslContextSpec) e.g. in ReactorHttpClient#createClient. This is fine, since ProtocolSslContextSpec already extends GenericSslContextSpec<SslContextBuilder> and thus, when the SslContextSpec#sslContext(SslProvider.ProtocolSslContextSpec) gets removed, we'll compile against the non-deprecatedreactor.netty.tcp.SslProvider.SslCon1textSpec#sslContext(SslProvider.GenericSslContextSpec<?>).
  2. A few usages of deprecated reactor.netty.http.HttpDecoderSpec#maxChunkSize(int) are still in place where the server-side requests decoder gets configured. The deprecation comment says Deprecated as of 1.1.0. This will be removed in 2.0.0 as Netty 5 does not support this configuration., so decoder might need to get a more comprehensive update once the dependency upgrade happens.

Related Issues

N/A

Check List

  • Functionality includes testing.
    - [ ] API changes companion pull request created, if applicable.
    - [ ] Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
@fdesu fdesu requested a review from a team as a code owner January 16, 2026 10:28
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

Fixes Netty deprecation warnings in the transport-reactor-netty4 module by replacing deprecated NioEventLoopGroup with MultiThreadIoEventLoopGroup and adding NioIoHandler.newFactory(). Changes applied to main code and test files with corresponding import updates.

Changes

Cohort / File(s) Summary
Documentation
CHANGELOG.md
Added entry documenting the Netty deprecation fix in the Unreleased 3.x section.
Netty EventLoopGroup Migration
plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java
Replaced NioEventLoopGroup with MultiThreadIoEventLoopGroup, added NioIoHandler.newFactory() factory parameter to constructors for both HTTP and generic transport groups. Updated imports accordingly.
Netty EventLoopGroup Migration
plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java, ...ReactorNetty4HttpServerTransportTests.java, .../ssl/SecureReactorNetty4HttpServerTransportTests.java
Replaced NioEventLoopGroup with generic EventLoopGroup backed by MultiThreadIoEventLoopGroup and NioIoHandler.newFactory(). Updated method signatures and variable types accordingly. Minor API updates (request.uri() instead of request.getUri()).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing Netty deprecation warnings in the transport-reactor-netty4 module, which matches the primary objective of the PR.
Description check ✅ Passed The description includes detailed explanation of changes, references to related documentation, notes on remaining deprecations, and confirmation of testing. The Description and Check List sections are properly completed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddf20f8 and 13948ce.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20017
File: plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java:256-256
Timestamp: 2025-12-12T18:40:08.452Z
Learning: In the OpenSearch ReactorNetty4 secure HTTP transport tests (plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java), URI limit validation has been moved from the protocol layer to the transport layer, making it protocol-agnostic. The random protocol selection in ReactorHttpClient.https(settings) is intentional to ensure all tests validate correct behavior across HTTP/1.1, HTTP/2, and HTTP/3.
📚 Learning: 2025-12-12T18:40:08.452Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20017
File: plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java:256-256
Timestamp: 2025-12-12T18:40:08.452Z
Learning: In the OpenSearch ReactorNetty4 secure HTTP transport tests (plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java), URI limit validation has been moved from the protocol layer to the transport layer, making it protocol-agnostic. The random protocol selection in ReactorHttpClient.https(settings) is intentional to ensure all tests validate correct behavior across HTTP/1.1, HTTP/2, and HTTP/3.

Applied to files:

  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java
  • plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java
📚 Learning: 2025-12-12T13:31:51.234Z
Learnt from: andriyredko
Repo: opensearch-project/OpenSearch PR: 20017
File: plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java:642-646
Timestamp: 2025-12-12T13:31:51.234Z
Learning: In the OpenSearch ReactorNetty4 secure HTTP transport tests (plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java), the createBuilderWithPort() helper intentionally uses randomBoolean() for the HTTP/3 enabled setting to ensure all tests validate correct behavior with both HTTP/3 enabled and disabled configurations.

Applied to files:

  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java
📚 Learning: 2025-12-13T20:16:15.318Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20017
File: modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4Http3ServerTransport.java:101-123
Timestamp: 2025-12-13T20:16:15.318Z
Learning: In OpenSearch, only one HTTP transport implementation can be active and loaded at a time, so duplicate setting definitions (such as h3.max_stream_local_length, h3.max_stream_remote_length, and h3.max_streams) across different transport implementations like Netty4Http3ServerTransport and ReactorNetty4HttpServerTransport will not cause setting registration conflicts.

Applied to files:

  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java
📚 Learning: 2025-12-19T21:26:37.090Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20017
File: modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4QuicServerTransport.java:0-0
Timestamp: 2025-12-19T21:26:37.090Z
Learning: In Netty4QuicServerTransport (modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4QuicServerTransport.java), the secureHttpTransportSettingsProvider parameter is guaranteed to be non-null because the plugin verifies its presence during instantiation, so no explicit null-check is needed in the constructor.

Applied to files:

  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportTests.java
  • plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: gradle-check
  • GitHub Check: Analyze (java)
  • GitHub Check: detect-breaking-change
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
🔇 Additional comments (12)
CHANGELOG.md (1)

37-37: Changelog entry looks good.

plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java (2)

70-72: Import-only change.


509-548: LGTM.
Test setup and teardown still properly manage the client group lifecycle.

plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransportTests.java (2)

78-80: Import-only change.


502-540: LGTM.
Test remains deterministic with explicit group shutdown in the finally block.

plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java (3)

29-30: Import-only change.


91-95: LGTM.


104-108: LGTM.

plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java (4)

39-41: Import-only change.


204-213: LGTM.


245-251: LGTM.


281-287: LGTM.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
@github-actions
Copy link
Contributor

✅ Gradle check result for 13948ce: SUCCESS

@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.23%. Comparing base (ddf20f8) to head (13948ce).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ensearch/transport/reactor/SharedGroupFactory.java 50.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #20429      +/-   ##
============================================
- Coverage     73.30%   73.23%   -0.08%     
+ Complexity    71832    71760      -72     
============================================
  Files          5792     5792              
  Lines        328639   328641       +2     
  Branches      47311    47311              
============================================
- Hits         240905   240665     -240     
- Misses        68458    68675     +217     
- Partials      19276    19301      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@reta reta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @fdesu !

@reta reta merged commit b981203 into opensearch-project:main Jan 16, 2026
33 of 34 checks passed
@fdesu fdesu deleted the issues/reactor-netty-deprecation-warnings branch January 16, 2026 13:55
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