diff --git a/bftclient/include/bftclient/bft_client.h b/bftclient/include/bftclient/bft_client.h index eace3f27fa..6b2d520528 100644 --- a/bftclient/include/bftclient/bft_client.h +++ b/bftclient/include/bftclient/bft_client.h @@ -50,7 +50,7 @@ class Client { Reply send(const ReadConfig& config, Msg&& request); SeqNumToReplyMap sendBatch(std::deque& write_requests, const std::string& cid); - // Return true if the client has at least num_replicas_required active replica connecions. + // Return true if the client has at least num_replicas_required active replica connections. bool isServing(int num_replicas, int num_replicas_required) const; // Useful for testing. Shouldn't be relied on in production. diff --git a/bftengine/include/bftengine/ClientMsgs.hpp b/bftengine/include/bftengine/ClientMsgs.hpp index 5d67700302..c8c749c44f 100644 --- a/bftengine/include/bftengine/ClientMsgs.hpp +++ b/bftengine/include/bftengine/ClientMsgs.hpp @@ -53,6 +53,7 @@ struct ClientReplyMsgHeader { uint32_t spanContextSize = 0u; uint16_t currentPrimaryId; uint64_t reqSeqNum; + uint32_t result; // Request execution result // Reply length is the total length of the reply, including any replica specific info. uint32_t replyLength; diff --git a/bftengine/include/bftengine/IRequestHandler.hpp b/bftengine/include/bftengine/IRequestHandler.hpp index 6efcc139f1..7464c2132b 100644 --- a/bftengine/include/bftengine/IRequestHandler.hpp +++ b/bftengine/include/bftengine/IRequestHandler.hpp @@ -48,7 +48,7 @@ class IRequestsHandler { uint64_t requestSequenceNum = executionSequenceNum; uint32_t outActualReplySize = 0; uint32_t outReplicaSpecificInfoSize = 0; - int outExecutionStatus = 1; + uint32_t outExecutionStatus = 1; uint64_t blockId = 0; }; diff --git a/bftengine/include/bftengine/SimpleClient.hpp b/bftengine/include/bftengine/SimpleClient.hpp index 749a3705ea..7f14d763ed 100644 --- a/bftengine/include/bftengine/SimpleClient.hpp +++ b/bftengine/include/bftengine/SimpleClient.hpp @@ -46,11 +46,11 @@ enum ClientMsgFlag : uint8_t { EMPTY_CLIENT_REQ = 0x10, }; -enum OperationResult : int8_t { SUCCESS, NOT_READY, TIMEOUT, BUFFER_TOO_SMALL, INVALID_REQUEST }; -// Call back for request - at this point we know for sure that a client is handling the request so we can assure that we -// will have reply. This callback will be attached to the client reply struct and whenever we get the reply from, the -// bft client we will activate the callback. -typedef std::variant SendResult; +enum OperationResult : uint32_t { SUCCESS, INVALID_REQUEST, NOT_READY, TIMEOUT, BUFFER_TOO_SMALL, EMPTY_EXEC_RESULT }; +// Call back for request - at this point we know for sure that a client is handling the request, so we can assure that +// we will have reply. This callback will be attached to the client reply struct and whenever we get the reply from, +// the bft client we will activate the callback. +typedef std::variant SendResult; typedef std::function RequestCallBack; struct ClientRequest { diff --git a/bftengine/src/bftengine/messages/ClientReplyMsg.cpp b/bftengine/src/bftengine/messages/ClientReplyMsg.cpp index b5686f0156..12b641cb0d 100644 --- a/bftengine/src/bftengine/messages/ClientReplyMsg.cpp +++ b/bftengine/src/bftengine/messages/ClientReplyMsg.cpp @@ -19,30 +19,33 @@ namespace impl { ClientReplyMsg::ClientReplyMsg(ReplicaId primaryId, ReqId reqSeqNum, ReplicaId replicaId) : MessageBase(replicaId, MsgCode::ClientReply, ReplicaConfig::instance().getmaxExternalMessageSize()) { - b()->reqSeqNum = reqSeqNum; - b()->currentPrimaryId = primaryId; - b()->replyLength = 0; - b()->replicaSpecificInfoLength = 0; + setHeaderParameters(primaryId, reqSeqNum, 0, 0); setMsgSize(sizeof(ClientReplyMsgHeader)); } ClientReplyMsg::ClientReplyMsg(ReplicaId replicaId, ReqId reqSeqNum, char* reply, uint32_t replyLength) : MessageBase(replicaId, MsgCode::ClientReply, sizeof(ClientReplyMsgHeader) + replyLength) { - b()->reqSeqNum = reqSeqNum; - b()->currentPrimaryId = 0; - b()->replyLength = replyLength; - + setHeaderParameters(0, reqSeqNum, replyLength, 0); memcpy(body() + sizeof(ClientReplyMsgHeader), reply, replyLength); - setMsgSize(sizeof(ClientReplyMsgHeader) + replyLength); } ClientReplyMsg::ClientReplyMsg(ReplicaId replicaId, uint32_t replyLength) : MessageBase(replicaId, MsgCode::ClientReply, sizeof(ClientReplyMsgHeader) + replyLength) { - b()->reqSeqNum = 0; - b()->currentPrimaryId = 0; - b()->replyLength = replyLength; + setHeaderParameters(0, 0, replyLength, 0); +} - setMsgSize(sizeof(ClientReplyMsgHeader) + replyLength); +// Reply with no data; returns an error to the client +ClientReplyMsg::ClientReplyMsg(ReplicaId primaryId, ReqId reqSeqNum, ReplicaId replicaId, uint32_t result) + : MessageBase(replicaId, MsgCode::ClientReply, sizeof(ClientReplyMsgHeader)) { + setHeaderParameters(primaryId, reqSeqNum, 0, result); +} + +void ClientReplyMsg::setHeaderParameters(ReplicaId primaryId, ReqId reqSeqNum, uint32_t replyLength, uint32_t result) { + b()->currentPrimaryId = primaryId; + b()->reqSeqNum = reqSeqNum; + b()->replyLength = replyLength; + b()->result = result; + b()->replicaSpecificInfoLength = 0; } void ClientReplyMsg::setReplyLength(uint32_t replyLength) { diff --git a/bftengine/src/bftengine/messages/ClientReplyMsg.hpp b/bftengine/src/bftengine/messages/ClientReplyMsg.hpp index 820d035c36..2b3884a902 100644 --- a/bftengine/src/bftengine/messages/ClientReplyMsg.hpp +++ b/bftengine/src/bftengine/messages/ClientReplyMsg.hpp @@ -24,13 +24,16 @@ class ClientReplyMsg : public MessageBase { static_assert(sizeof(ClientReplyMsgHeader::msgType) == sizeof(MessageBase::Header::msgType), ""); static_assert(sizeof(ClientReplyMsgHeader::reqSeqNum) == sizeof(ReqId), ""); static_assert(sizeof(ClientReplyMsgHeader::currentPrimaryId) == sizeof(ReplicaId), ""); - static_assert(sizeof(ClientReplyMsgHeader) == 24, "ClientRequestMsgHeader is 24B"); + static_assert(sizeof(ClientReplyMsgHeader::result) == sizeof(uint32_t), ""); + static_assert(sizeof(ClientReplyMsgHeader) == 28, "ClientRequestMsgHeader is 28B"); public: ClientReplyMsg(ReplicaId primaryId, ReqId reqSeqNum, ReplicaId replicaId); ClientReplyMsg(ReplicaId replicaId, ReqId reqSeqNum, char* reply, uint32_t replyLength); + ClientReplyMsg(ReplicaId primaryId, ReqId reqSeqNum, ReplicaId replicaId, uint32_t result); + ClientReplyMsg(ReplicaId replicaId, uint32_t replyLength); uint32_t maxReplyLength() const { return internalStorageSize() - sizeof(ClientReplyMsgHeader); } @@ -56,6 +59,10 @@ class ClientReplyMsg : public MessageBase { void setMsgSize(MsgSize size) { MessageBase::setMsgSize(size); } ClientReplyMsgHeader* b() const { return (ClientReplyMsgHeader*)msgBody_; } + + private: + void setHeaderParameters(ReplicaId primaryId, ReqId reqSeqNum, uint32_t replyLength, uint32_t result); }; + } // namespace impl } // namespace bftEngine diff --git a/bftengine/src/bftengine/messages/MessageBase.cpp b/bftengine/src/bftengine/messages/MessageBase.cpp index 6af3a80138..53bbad5c4a 100644 --- a/bftengine/src/bftengine/messages/MessageBase.cpp +++ b/bftengine/src/bftengine/messages/MessageBase.cpp @@ -134,9 +134,6 @@ void MessageBase::setMsgSize(MsgSize size) { ConcordAssert((msgBody_ != nullptr)); ConcordAssert(size <= storageSize_); - // TODO(GG): do we need to reset memory here? - if (storageSize_ > size) memset(body() + size, 0, (storageSize_ - size)); - msgSize_ = size; } diff --git a/client/clientservice/src/request_service.cpp b/client/clientservice/src/request_service.cpp index 4d39d4a167..a446ce5363 100644 --- a/client/clientservice/src/request_service.cpp +++ b/client/clientservice/src/request_service.cpp @@ -45,7 +45,7 @@ Status RequestServiceImpl::Send(ServerContext* context, const Request* proto_req auto callback = [&](cc::SendResult&& send_result) { if (not std::holds_alternative(send_result)) { LOG_INFO(logger_, "Send returned error"); - switch (std::get(send_result)) { + switch (std::get(send_result)) { case (concord_client_pool::Overloaded): status.set_value(grpc::Status(grpc::StatusCode::RESOURCE_EXHAUSTED, "All clients occupied")); break; diff --git a/client/concordclient/include/client/concordclient/concord_client.hpp b/client/concordclient/include/client/concordclient/concord_client.hpp index 8ab610df67..b5fa73c71e 100644 --- a/client/concordclient/include/client/concordclient/concord_client.hpp +++ b/client/concordclient/include/client/concordclient/concord_client.hpp @@ -37,7 +37,8 @@ struct SendError { }; ErrorType type; }; -typedef std::variant SendResult; + +typedef std::variant SendResult; struct ReplicaInfo { bft::client::ReplicaId id; diff --git a/kvbc/src/ClientImp.cpp b/kvbc/src/ClientImp.cpp index 533a90b42c..33abcba560 100644 --- a/kvbc/src/ClientImp.cpp +++ b/kvbc/src/ClientImp.cpp @@ -88,6 +88,8 @@ Status ClientImp::invokeCommandSynch(const char* request, return Status::InvalidArgument("Specified output buffer is too small"); case INVALID_REQUEST: return Status::InvalidArgument("Request is invalid"); + case EMPTY_EXEC_RESULT: + return Status::GeneralError("Execution result is empty"); } return Status::GeneralError("Unknown error"); } diff --git a/util/pyclient/bft_msgs.py b/util/pyclient/bft_msgs.py index c3c9188c44..3d1f850c95 100644 --- a/util/pyclient/bft_msgs.py +++ b/util/pyclient/bft_msgs.py @@ -47,7 +47,7 @@ def __init__(self, message): # Little Endian format with no padding # We don't include the msg type here, since we have to read it first to # understand what message is incoming. -REPLY_HEADER_FMT = "