Skip to content

chore!: Bind real config address for Thrift and Raft services#16717

Merged
CritasWang merged 3 commits intomasterfrom
wx_internal_address
Nov 11, 2025
Merged

chore!: Bind real config address for Thrift and Raft services#16717
CritasWang merged 3 commits intomasterfrom
wx_internal_address

Conversation

@CritasWang
Copy link
Collaborator

@CritasWang CritasWang commented Nov 7, 2025

The listening addresses for Thrift and Raft services are now explicitly bound to a non-loopback interface. This may affect service discovery and communication between nodes in a cluster.

BREAKING CHANGE: The Thrift and Raft services now bind to the physical interface address (e.g., 192.168.1.100) instead of 0.0.0.0. Similarly, the ExternalRpc Address, which defaults to 0.0.0.0, will now bind to the explicitly configured IP address. Ensure your cluster configuration and firewall rules are updated to allow communication on the real internal network and the configured ExternalRpc address.


image

@CritasWang CritasWang requested a review from Copilot November 7, 2025 10:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies network binding behavior for Thrift services and Ratis consensus to properly handle the case where services should bind to all available network interfaces. The key changes ensure that when no specific bind address is provided, services use the proper InetSocketAddress constructor overload.

  • Changes ExternalRPCService to pass null instead of getBindIP() for the bind address
  • Updates AbstractThriftServiceThread to properly handle null bind addresses with conditional logic
  • Adds explicit host configuration for Ratis gRPC server

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
AbstractThriftServiceThread.java Added null-checks and conditional logic to properly handle binding to all interfaces when bindAddress is null
ExternalRPCService.java Changed from passing getBindIP() to passing null to bind to all available network interfaces
RatisConsensus.java Added explicit host configuration via GrpcConfigKeys.Server.setHost()
Comments suppressed due to low confidence (1)

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/ExternalRPCService.java:104

  • The getBindIP() method is now unused in this class after changing lines 74 and 87 to pass null. If this method is not part of an interface contract or used elsewhere, consider whether it still serves a purpose. If it's required by a parent class or interface, this is acceptable, but it creates potential confusion about why the method exists if it's not being used locally.
  public String getBindIP() {
    return config.getRpcAddress();
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.75%. Comparing base (685141a) to head (cd17b0e).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
...b/commons/service/AbstractThriftServiceThread.java 33.33% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #16717      +/-   ##
============================================
+ Coverage     38.72%   38.75%   +0.02%     
  Complexity      207      207              
============================================
  Files          4986     5001      +15     
  Lines        330362   331347     +985     
  Branches      42000    42103     +103     
============================================
+ Hits         127938   128408     +470     
- Misses       202424   202939     +515     

☔ 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.

@CritasWang CritasWang changed the title chore: Bind real internal address for Thrift and Raft services chore!: Bind real config address for Thrift and Raft services Nov 10, 2025
@CritasWang CritasWang requested a review from Copilot November 10, 2025 02:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/ExternalRPCService.java:87

  • Inconsistent bind address handling between SSL and non-SSL configurations. Line 74 passes null for the SSL-enabled branch, but this line still passes getBindIP() for the non-SSL branch. This creates different binding behavior depending on SSL configuration. For consistency with the SSL branch and the updated AbstractThriftServiceThread logic, this should also be changed to null.
                  getBindIP(),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ExternalRPCService.java

Co-authored-by: Haonan <hhaonan@outlook.com>
@sonarqubecloud
Copy link

@CritasWang CritasWang merged commit 8dbf200 into master Nov 11, 2025
31 of 35 checks passed
CritasWang added a commit that referenced this pull request Nov 11, 2025
* chore: Bind real internal address for Thrift and Raft services

* keep ExternalRpcAddress use config value

* Update iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/ExternalRPCService.java

Co-authored-by: Haonan <hhaonan@outlook.com>

---------

Co-authored-by: Haonan <hhaonan@outlook.com>
JackieTien97 pushed a commit that referenced this pull request Nov 26, 2025
* chore: Bind real internal address for Thrift and Raft services

* keep ExternalRpcAddress use config value

* Update iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/ExternalRPCService.java

Co-authored-by: Haonan <hhaonan@outlook.com>

---------

Co-authored-by: Haonan <hhaonan@outlook.com>
(cherry picked from commit 8dbf200)
@JackieTien97 JackieTien97 deleted the wx_internal_address branch November 28, 2025 23:28
JackieTien97 pushed a commit that referenced this pull request Feb 12, 2026
* chore: Bind real internal address for Thrift and Raft services

* keep ExternalRpcAddress use config value

* Update iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/ExternalRPCService.java

Co-authored-by: Haonan <hhaonan@outlook.com>

---------

Co-authored-by: Haonan <hhaonan@outlook.com>
(cherry picked from commit 8dbf200)
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.

3 participants