Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add result code to response messages #515

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Add result code to response messages #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b2d3b3
Add result code to response messages
amichon-kalray Jun 17, 2024
3f71551
[Update PR] Add `int32_t` to `cmd_result_code`
greensky00 Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/libnuraft/async.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ limitations under the License.

namespace nuraft {

enum cmd_result_code {
enum cmd_result_code : int32_t {
OK = 0,
CANCELLED = -1,
TIMEOUT = -2,
Expand Down
35 changes: 32 additions & 3 deletions src/asio_service.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ limitations under the License.
// If set, each log entry will contain a CRC on the payload.
#define CRC_ON_PAYLOAD (0x10)

// If set, RPC message (response) includes result code
#define INCLUDE_RESULT_CODE (0x20)

// =======================

namespace nuraft {
Expand Down Expand Up @@ -735,6 +738,7 @@ class rpc_session
try {
ptr<buffer> resp_ctx = resp->get_ctx();
int32 resp_ctx_size = (resp_ctx) ? resp_ctx->size() : 0;
int32 result_code_size = sizeof(int32_t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, can you please use

int32 result_code_size = sizeof(cmd_result_code);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok
my concern was that we will use the put_i32 and get_i32 functions, so it result_code_size is not sizeof(int32_t), weird things may happen

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point, that makes more sense. Let me make a change to use sizeof(int32_t) and instead put int32_t to cmd_result_code definition.


uint32_t flags = 0x0;
size_t resp_meta_size = 0;
Expand All @@ -759,6 +763,13 @@ class rpc_session

size_t carried_data_size = resp_meta_size + resp_hint_size + resp_ctx_size;

if (req->get_type() == msg_type::client_request ||
req->get_type() == msg_type::add_server_request ||
req->get_type() == msg_type::remove_server_request) {
flags |= INCLUDE_RESULT_CODE;
carried_data_size += result_code_size;
}

int buf_size = RPC_RESP_HEADER_SIZE + carried_data_size;
ptr<buffer> resp_buf = buffer::alloc(buf_size);
buffer_serializer bs(resp_buf);
Expand Down Expand Up @@ -798,6 +809,11 @@ class rpc_session
bs.put_buffer(*resp_ctx);
}

/* Put result code at the end to avoid breaking backward compatibility */
if (flags & INCLUDE_RESULT_CODE) {
bs.put_i32(resp->get_result_code());
}

aa::write( ssl_enabled_, ssl_socket_, socket_,
asio::buffer(resp_buf->data_begin(), resp_buf->size()),
[this, self, resp_buf]
Expand Down Expand Up @@ -1688,8 +1704,9 @@ class asio_rpc_client
size_t bytes_transferred)
{
if ( !(flags & INCLUDE_META) &&
!(flags & INCLUDE_HINT) ) {
// Neither meta nor hint exists,
!(flags & INCLUDE_HINT) &&
!(flags & INCLUDE_RESULT_CODE)) {
// Neither meta nor hint nor result code exists,
// just use the buffer as it is for ctx.
ctx_buf->pos(0);
rsp->set_ctx(ctx_buf);
Expand Down Expand Up @@ -1739,9 +1756,21 @@ class asio_rpc_client
assert(remaining_len >= 0);
if (remaining_len) {
// It has context, read it.
ptr<buffer> actual_ctx = buffer::alloc(remaining_len);
size_t ctx_len = remaining_len;
if (flags & INCLUDE_RESULT_CODE) {
ctx_len -= sizeof(int32_t);
}
ptr<buffer> actual_ctx = buffer::alloc(ctx_len);
bs.get_buffer(actual_ctx);
rsp->set_ctx(actual_ctx);
remaining_len -= ctx_len;
}

// 4) Result code
if (flags & INCLUDE_RESULT_CODE) {
assert((size_t)remaining_len >= sizeof(int32_t));
cmd_result_code res = static_cast<cmd_result_code>(bs.get_i32());
rsp->set_result_code(res);
}

operation_timer_.cancel();
Expand Down
Loading