Resending votes in consensus #2200
Conversation
Signed-off-by: Fedor Muratov <muratovfyodor@yandex.ru>
MBoldyrev
left a comment
MBoldyrev
left a comment
There was a problem hiding this comment.
Please re-enable the test or provide a reason of its disabling. Also, there might be a problem as MST regression, as there is another MST test showing spontaneous failures - it is worth further investigation.
| * Provide current leader peer | ||
| */ | ||
| const shared_model::interface::Peer ¤tLeader(); | ||
| const std::shared_ptr<shared_model::interface::Peer> ¤tLeader(); |
There was a problem hiding this comment.
I would make
| const std::shared_ptr<shared_model::interface::Peer> ¤tLeader(); | |
| std::shared_ptr<shared_model::interface::Peer> currentLeader(); |
as returning a reference to a local object is scary, and then
| const std::shared_ptr<shared_model::interface::Peer> ¤tLeader(); | |
| std::shared_ptr<const shared_model::interface::Peer> currentLeader(); |
as we do not want anyone to change the object (no matter it is an interface)
| StatusCode::UNIMPLEMENTED, | ||
| StatusCode::UNAVAILABLE, | ||
| StatusCode::DATA_LOSS}; | ||
| return std::any_of(codes.begin(), codes.end(), [code](auto val) { |
There was a problem hiding this comment.
| return std::any_of(codes.begin(), codes.end(), [code](auto val) { | |
| return std::find(codes.begin(), codes.end(), code) != codes.end(); |
or, if using set,
| return std::any_of(codes.begin(), codes.end(), [code](auto val) { | |
| return codes.find(code) != codes.end(); |
which is faster
|
|
||
| auto is_troubles_with_recipient = [](const auto &code) { | ||
| using namespace grpc; | ||
| std::vector<int> codes = {StatusCode::CANCELLED, |
There was a problem hiding this comment.
| std::vector<int> codes = {StatusCode::CANCELLED, | |
| std::set<StatusCode> codes = {StatusCode::CANCELLED, |
performs better lookup
There was a problem hiding this comment.
Seems that set is more appropriate because it is a more reliable way to stay unique constants.
| // TODO: 2019-03-27 @muratovv the test is blocked by IR-276 | ||
| TEST_F(BasicMstPropagationFixture, | ||
| MstStateOfTransactionWithoutAllSignaturesPropagtesToOtherPeer) { | ||
| DISABLED_MstStateOfTransactionWithoutAllSignaturesPropagtesToOtherPeer) { |
There was a problem hiding this comment.
Why is the test disabled? Please provide a reason. Taking into account another failing test related to MST (see CI), I suggest possible investigating MST regression - it could cause this test failing.
There was a problem hiding this comment.
Seems, the branch was outdated and now basic_mst_state_propagation test absent.
Description of the Change
The PR introduce changes for resending votes. The feature is required due to we want to simulate the asynchronous environment among YAC processors.
Design
Possible Drawbacks
Signed-off-by: Fedor Muratov muratovfyodor@yandex.ru