From 35d46267b8b95c4c70462446dcae9d2d143b8622 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 6 Jul 2017 10:48:16 -0700 Subject: [PATCH] jsonrpc: remove support for batch requests This will greatly simplify the implementation of asynchronous requests, and asynchronous requests also allow clients to submit multiple overlapped requests, making batches unnecessary for executing multiple RPCs at once. Additionally, our RPC client (scripts/rpc.py) does not use batch requests. Change-Id: I2529793c54b43acbacd934d82926aa32e286210c Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/368449 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/jsonrpc/jsonrpc_internal.h | 2 - lib/jsonrpc/jsonrpc_server.c | 66 ++---------------- lib/jsonrpc/jsonrpc_server_tcp.c | 1 - .../jsonrpc_server.c/jsonrpc_server_ut.c | 68 +------------------ 4 files changed, 7 insertions(+), 130 deletions(-) diff --git a/lib/jsonrpc/jsonrpc_internal.h b/lib/jsonrpc/jsonrpc_internal.h index dbefedb11..05e2ac630 100644 --- a/lib/jsonrpc/jsonrpc_internal.h +++ b/lib/jsonrpc/jsonrpc_internal.h @@ -58,8 +58,6 @@ struct spdk_jsonrpc_server_conn { uint8_t recv_buf[SPDK_JSONRPC_RECV_BUF_SIZE]; size_t send_len; uint8_t send_buf[SPDK_JSONRPC_SEND_BUF_SIZE]; - struct spdk_json_write_ctx *json_writer; - bool batch; }; struct spdk_jsonrpc_server { diff --git a/lib/jsonrpc/jsonrpc_server.c b/lib/jsonrpc/jsonrpc_server.c index 63b0508cd..9e3e11680 100644 --- a/lib/jsonrpc/jsonrpc_server.c +++ b/lib/jsonrpc/jsonrpc_server.c @@ -112,48 +112,6 @@ done: } } -static void -parse_batch_request(struct spdk_jsonrpc_server_conn *conn, struct spdk_json_val *values) -{ - size_t num_values, i; - - assert(values[0].type == SPDK_JSON_VAL_ARRAY_BEGIN); - num_values = values[0].len; - values++; - - assert(conn->json_writer == NULL); - - - if (num_values == 0) { - struct spdk_jsonrpc_request error_request; - - SPDK_TRACELOG(SPDK_TRACE_RPC, "empty batch array not allowed"); - - /* Build a request with id = NULL since we don't have a valid request ID */ - error_request.conn = conn; - error_request.id = NULL; - spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); - return; - } - - i = 0; - while (i < num_values) { - struct spdk_json_val *v = &values[i]; - - parse_single_request(conn, v); - i += spdk_json_val_len(v); - } - - if (conn->json_writer) { - /* - * There was at least one response - finish the batch array. - */ - spdk_json_write_array_end(conn->json_writer); - spdk_json_write_end(conn->json_writer); - conn->json_writer = NULL; - } -} - int spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, size_t size) { @@ -161,14 +119,10 @@ spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, si ssize_t rc; void *end = NULL; - assert(conn->json_writer == NULL); - /* Build a request with id = NULL since we don't have a valid request ID */ error_request.conn = conn; error_request.id = NULL; - conn->batch = false; - /* Check to see if we have received a full JSON value. */ rc = spdk_json_parse(json, size, NULL, 0, &end, 0); if (rc == SPDK_JSON_PARSE_INCOMPLETE) { @@ -198,8 +152,8 @@ spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, si if (conn->values[0].type == SPDK_JSON_VAL_OBJECT_BEGIN) { parse_single_request(conn, conn->values); } else if (conn->values[0].type == SPDK_JSON_VAL_ARRAY_BEGIN) { - conn->batch = true; - parse_batch_request(conn, conn->values); + SPDK_TRACELOG(SPDK_TRACE_RPC, "Got batch array (not currently supported)\n"); + spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); } else { SPDK_TRACELOG(SPDK_TRACE_RPC, "top-level JSON value was not array or object\n"); spdk_jsonrpc_server_handle_error(&error_request, SPDK_JSONRPC_ERROR_INVALID_REQUEST); @@ -211,12 +165,9 @@ spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, si static struct spdk_json_write_ctx * begin_response(struct spdk_jsonrpc_server_conn *conn, const struct spdk_json_val *id) { - struct spdk_json_write_ctx *w = conn->json_writer; - - if (w == NULL) { - conn->json_writer = w = spdk_json_write_begin(spdk_jsonrpc_server_write_cb, conn, 0); - } + struct spdk_json_write_ctx *w; + w = spdk_json_write_begin(spdk_jsonrpc_server_write_cb, conn, 0); if (w == NULL) { return NULL; } @@ -237,12 +188,8 @@ static void end_response(struct spdk_jsonrpc_server_conn *conn, struct spdk_json_write_ctx *w) { spdk_json_write_object_end(w); - - if (!conn->batch) { - spdk_json_write_end(w); - spdk_jsonrpc_server_write_cb(conn, "\n", 1); - conn->json_writer = NULL; - } + spdk_json_write_end(w); + spdk_jsonrpc_server_write_cb(conn, "\n", 1); } struct spdk_json_write_ctx * @@ -264,7 +211,6 @@ void spdk_jsonrpc_end_result(struct spdk_jsonrpc_request *request, struct spdk_json_write_ctx *w) { assert(w != NULL); - assert(w == request->conn->json_writer); end_response(request->conn, w); } diff --git a/lib/jsonrpc/jsonrpc_server_tcp.c b/lib/jsonrpc/jsonrpc_server_tcp.c index d950b737c..14c9d6cda 100644 --- a/lib/jsonrpc/jsonrpc_server_tcp.c +++ b/lib/jsonrpc/jsonrpc_server_tcp.c @@ -136,7 +136,6 @@ spdk_jsonrpc_server_accept(struct spdk_jsonrpc_server *server) conn->sockfd = rc; conn->recv_len = 0; conn->send_len = 0; - conn->json_writer = 0; nonblock = 1; rc = ioctl(conn->sockfd, FIONBIO, &nonblock); diff --git a/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c b/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c index 4c84403f9..2ff6a19f3 100644 --- a/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c +++ b/test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c @@ -282,32 +282,7 @@ test_parse_request(void) REQ_ID_MISSING(); REQ_PARAMS_MISSING(); - /* invalid batch */ - PARSE_PASS("[1]", ""); - REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); - REQ_METHOD_MISSING(); - REQ_ID_MISSING(); - REQ_PARAMS_MISSING(); - - /* invalid batch */ - PARSE_PASS("[1,2,3]", ""); - - REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); - REQ_METHOD_MISSING(); - REQ_ID_MISSING(); - REQ_PARAMS_MISSING(); - - REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); - REQ_METHOD_MISSING(); - REQ_ID_MISSING(); - REQ_PARAMS_MISSING(); - - REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); - REQ_METHOD_MISSING(); - REQ_ID_MISSING(); - REQ_PARAMS_MISSING(); - - /* batch */ + /* batch - not supported */ PARSE_PASS( "[" "{\"jsonrpc\": \"2.0\", \"method\": \"sum\", \"params\": [1,2,4], \"id\": \"1\"}," @@ -318,52 +293,11 @@ test_parse_request(void) "{\"jsonrpc\": \"2.0\", \"method\": \"get_data\", \"id\": \"9\"}" "]", ""); - REQ_BEGIN_VALID(); - REQ_METHOD("sum"); - REQ_ID_STRING("1"); - REQ_PARAMS_BEGIN(); - PARAM_ARRAY_BEGIN(); - PARAM_NUM("1"); - PARAM_NUM("2"); - PARAM_NUM("4"); - PARAM_ARRAY_END(); - - REQ_BEGIN_VALID(); - REQ_METHOD("notify_hello"); - REQ_ID_MISSING(); - REQ_PARAMS_BEGIN(); - PARAM_ARRAY_BEGIN(); - PARAM_NUM("7"); - PARAM_ARRAY_END(); - - REQ_BEGIN_VALID(); - REQ_METHOD("subtract"); - REQ_ID_STRING("2"); - REQ_PARAMS_BEGIN(); - PARAM_ARRAY_BEGIN(); - PARAM_NUM("42"); - PARAM_NUM("23"); - PARAM_ARRAY_END(); - REQ_BEGIN_INVALID(SPDK_JSONRPC_ERROR_INVALID_REQUEST); REQ_METHOD_MISSING(); REQ_ID_MISSING(); REQ_PARAMS_MISSING(); - REQ_BEGIN_VALID(); - REQ_METHOD("foo.get"); - REQ_ID_STRING("5"); - REQ_PARAMS_BEGIN(); - PARAM_OBJECT_BEGIN(); - PARAM_NAME("name"); - PARAM_STRING("myself"); - PARAM_OBJECT_END(); - - REQ_BEGIN_VALID(); - REQ_METHOD("get_data"); - REQ_ID_STRING("9"); - REQ_PARAMS_MISSING(); - free(conn); free(server); }