-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
(bugfix) fix rheakv state machine bug #137
Conversation
Need to add some unit test |
…ugfix/halt_on_exception # Conflicts: # jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/storage/KVStoreStateMachine.java
存在冲突 |
和 #154 冲突了,我先解决下 |
@@ -84,8 +84,7 @@ public Status rangeSplit(final long regionId, final long newRegionId, final Stri | |||
if (response.isSuccess()) { | |||
return Status.OK(); | |||
} | |||
return new Status(-1, "fail to range split on region %d, error: %s", regionId, String.valueOf(response | |||
.getError())); | |||
return new Status(-1, "Fail to range split on region %d, error: %s", regionId, String.valueOf(response)); |
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.
不需要调用 String.valueOf 吧? 格式化会 %s 自动调用 toString
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.
fixed
assert first != null; | ||
batchApplyAndRecycle(first.getOpByte(), kvStates); | ||
} catch (final Throwable t) { | ||
it.setErrorAndRollback(index - applied, new Status(RaftError.EIO, "StateMachine meet critical error: %s.", |
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.
RaftError 选择 ESTATEMACHINE 更合适,并且这里建议加下日志,打印出堆栈
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.
done
Motivation:
If rheakv caught a critical error, should halt the state machine to avoid they inconsistencies between different nodes.
Modification:
As above describe.
Result:
Fixes #136