-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
rh客户端 发生 RemotingException 时,重试 #1135
Conversation
WalkthroughThe recent modifications enhance error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DefaultRheaKVRpcService
participant DefaultPlacementDriverRpcService
participant Errors
Caller->>DefaultRheaKVRpcService: Trigger RPC operation
DefaultRheaKVRpcService->>Errors: Check for RemotingException
alt RemotingException
DefaultRheaKVRpcService->>Errors: setError(RPC_CONNECTION_ERROR)
DefaultRheaKVRpcService->>Caller: Complete future with RPC_CONNECTION_ERROR
else Other Error
DefaultRheaKVRpcService->>Caller: Complete future with original error
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/failover/impl/FailoverClosureImpl.java (2 hunks)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java (2 hunks)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.java (1 hunks)
Additional comments not posted (4)
jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.java (1)
29-29
: Inclusion ofErrors.RPC_ERROR
inisInvalidPeer
method.The addition of
Errors.RPC_ERROR
to theisInvalidPeer
method broadens the criteria for identifying invalid peers, enhancing error handling for RPC issues. This change is appropriate and aligns with the PR objectives.jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/failover/impl/FailoverClosureImpl.java (1)
95-100
: Enhanced error handling infailure
method forRemotingException
.The updated
failure
method now checks if the cause is an instance ofRemotingException
. If so, it sets an error state withErrors.RPC_ERROR
and runs a new status. This change improves the granularity of error handling by distinguishing between different types of exceptions.jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java (2)
39-41
: Addition of new error constantRPC_ERROR
.The new error constant
RPC_ERROR
represents a specific server availability issue due to a disconnected RPC connection. This addition enhances the granularity of error handling related to server communication issues.
70-76
: Standardization of error messages.The error messages for
INVALID_STORE_STATS
,INVALID_REGION_STATS
,STORE_HEARTBEAT_OUT_OF_DATE
, andREGION_HEARTBEAT_OUT_OF_DATE
have been updated to include a period at the end. This change improves readability and consistency across the error handling framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.java
...-core/src/main/java/com/alipay/sofa/jraft/rhea/client/failover/impl/FailoverClosureImpl.java
Outdated
Show resolved
Hide resolved
...-core/src/main/java/com/alipay/sofa/jraft/rhea/client/failover/impl/FailoverClosureImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/DefaultRheaKVRpcService.java (2 hunks)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/pd/DefaultPlacementDriverRpcService.java (2 hunks)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java (2 hunks)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.java
Additional comments not posted (4)
jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/pd/DefaultPlacementDriverRpcService.java (2)
27-27
: Import statement addition is appropriate.The import statement for
RemotingException
is necessary for the new error handling mechanism.
114-119
: Enhanced error handling is well-implemented.The new error handling mechanism improves the specificity of error handling by distinguishing between different types of errors.
However, ensure that
Errors.RPC_CONNECTION_ERROR
is correctly defined and used throughout the codebase.Verification successful
Errors.RPC_CONNECTION_ERROR
is correctly defined and used.The error code
Errors.RPC_CONNECTION_ERROR
is properly defined inErrors.java
and used consistently inDefaultRheaKVRpcService.java
,DefaultPlacementDriverRpcService.java
, andErrorsHelper.java
to handle RPC connection issues.
Errors.java
: Defined with the value-2
and the message "RPC connection disconnected."DefaultRheaKVRpcService.java
: Used to handleRemotingException
.ErrorsHelper.java
: Checked within theisInvalidPeer
method.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of Errors.RPC_CONNECTION_ERROR in the codebase. # Test: Search for the usage of Errors.RPC_CONNECTION_ERROR. Expect: Correct definition and usage. rg --type java 'Errors.RPC_CONNECTION_ERROR'Length of output: 551
Script:
#!/bin/bash # Locate the definition of Errors.RPC_CONNECTION_ERROR rg --type java 'RPC_CONNECTION_ERROR' -A 5 # Inspect the context of its usage in DefaultRheaKVRpcService.java rg --type java -A 5 -B 5 'Errors.RPC_CONNECTION_ERROR' jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/DefaultRheaKVRpcService.java # Inspect the context of its usage in ErrorsHelper.java rg --type java -A 5 -B 5 'Errors.RPC_CONNECTION_ERROR' jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ErrorsHelper.javaLength of output: 4823
jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/DefaultRheaKVRpcService.java (2)
27-27
: Import statement addition is appropriate.The import statement for
RemotingException
is necessary for the new error handling mechanism.
143-148
: Enhanced error handling is well-implemented.The new error handling mechanism improves the specificity of error handling by distinguishing between different types of errors.
However, ensure that
Errors.RPC_CONNECTION_ERROR
is correctly defined and used throughout the codebase.Verification successful
Enhanced error handling is well-implemented.
The new error handling mechanism improves the specificity of error handling by distinguishing between different types of errors. The
Errors.RPC_CONNECTION_ERROR
is correctly defined and consistently used across the codebase.
Errors.java
: Definition ofRPC_CONNECTION_ERROR
DefaultRheaKVRpcService.java
: Usage ofRPC_CONNECTION_ERROR
DefaultPlacementDriverRpcService.java
: Usage ofRPC_CONNECTION_ERROR
ErrorsHelper.java
: Usage ofRPC_CONNECTION_ERROR
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of Errors.RPC_CONNECTION_ERROR in the codebase. # Test: Search for the usage of Errors.RPC_CONNECTION_ERROR. Expect: Correct definition and usage. rg --type java 'Errors.RPC_CONNECTION_ERROR'Length of output: 551
Script:
#!/bin/bash # Description: Verify the definition of Errors.RPC_CONNECTION_ERROR in the codebase. # Test: Search for the definition of RPC_CONNECTION_ERROR in the Errors class. Expect: Correct definition. rg --type java 'RPC_CONNECTION_ERROR' -A 5Length of output: 3456
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thank you!
jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ConnectionFailureException.java (1 hunks)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/Errors.java
Additional comments not posted (5)
jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/errors/ConnectionFailureException.java (5)
1-17
: Package declaration and license information are correct.The package declaration and license information are standard and do not contain any issues.
19-21
: Class-level documentation is clear and concise.The documentation provides a brief description of the
ConnectionFailureException
.
22-24
: Class declaration and serialVersionUID are correct.The class extends
ApiException
and includes aserialVersionUID
.
25-38
: Constructors are correctly implemented.The class includes four constructors: a default constructor, a constructor with a message, a constructor with a message and cause, and a constructor with a cause. All are correctly implemented and follow standard practices.
39-39
: Class is correctly closed.The class ends correctly with a closing brace.
您好,我尝试使用1.3.15,好像问题并没有修复 |
回顾你贴出的异常:https://github.com/sofastack/sofa-jraft/issues/1131#issuecomment-2255066719。 你说的未修复的问题,可以再提供一些详细信息吗? |
您的修改为:在rpc回调的complete中添加了异常处理,我复现发现当出现RemotingException的时候,并未进入complete,而是在这里直接异常处理了。所以没有进到closure.run(new Status(-1, "RPC failed with address: %s, response: %s", endpoint, response)); |
你是对的,我测试了一下,确实直接抛异常出去了 |
最近还能在发一次版本吗,或者这个版本从发? |
应该会发一个fix版本 |
在使用RheaKV的时候,RpcOption指定blot心跳关闭,应该如何设置呢?我使用了下面的第一种方式,2、3我没有找到参数和rheaKVStore结合使用的方法
|
Implementation of Issue #1131
Summary by CodeRabbit
New Features
RPC_ERROR
andRPC_CONNECTION_ERROR
, to improve error handling for server communication issues.ConnectionFailureException
to manage RPC connection-related errors.Bug Fixes
Style
Refactor