-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Pipe: Fix connection leak caused by clients not closed after task dropped (2 situations) #15910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds additional checks and cleanup logic when returning or closing clients, especially when nodes are null or connectors are closed. Enhances error logging and ensures resources are properly invalidated to prevent leaks. Also makes the close() method in AsyncPipeDataTransferServiceClient public for broader access.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15910 +/- ##
============================================
- Coverage 39.09% 39.07% -0.02%
Complexity 198 198
============================================
Files 4847 4847
Lines 315799 315914 +115
Branches 39658 39672 +14
============================================
- Hits 123448 123434 -14
- Misses 192351 192480 +129 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request introduces several changes across multiple files to improve connection management, enhance error handling, and ensure proper resource cleanup. The most notable updates include replacing direct
clientManager
calls with agetClientManager()
method for safer access, adding checks to prevent operations after closure, and improving logging for debugging purposes.Connection Management Improvements:
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/connector/protocol/IoTDBSslSyncConnector.java
: Replaced directclientManager
access with agetClientManager()
method that throws an exception if the client manager is closed, ensuring safer access.iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/connector/protocol/IoTDBSslSyncConnector.java
: Updated methods likehandshake
andsendHandshakeReq
to usegetClientManager()
instead of directly accessingclientManager
. [1] [2]Resource Cleanup Enhancements:
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/connector/client/IoTDBDataNodeAsyncClientManager.java
: Added logic to close clients and invalidate resources in cases where the manager is closed, preventing connection leaks. [1] [2]iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/client/ClientManager.java
: Enhanced thereturnClient
method to handle cases where the node is null, invalidating resources to prevent leaks.Error Handling Improvements:
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/connector/protocol/thrift/async/handler/PipeTransferTsFileHandler.java
: Improved error handling and resource cleanup when the connector is closed, with detailed logging for debugging.iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/client/async/AsyncPipeDataTransferServiceClient.java
: Made theclose
method public for better resource management.Operational Safety Enhancements:
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/agent/task/subtask/connector/PipeConnectorSubtask.java
: Added a check to prevent heartbeat or transfer operations after the connector is closed.iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/connector/protocol/thrift/async/IoTDBDataRegionAsyncConnector.java
: Updated theheartbeat
method to ensure it does not execute if the connector is closed.These changes collectively enhance the robustness of the system by ensuring safer client management, preventing resource leaks, and improving error handling and operational safety.