From 5c1c99e5d73a28c73612d897418e35e1aaa5867e Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Thu, 25 May 2017 00:00:30 -0400 Subject: [PATCH 1/2] Remove use of bucket token when downloading Reduces the overall requests to the bridge by around 1/3 when downloading files. --- src/downloader.c | 215 +++++------------------------------------------ src/downloader.h | 17 ---- src/http.c | 12 --- src/http.h | 1 - src/storj.c | 8 +- src/storj.h | 3 - src/uploader.c | 7 +- 7 files changed, 27 insertions(+), 236 deletions(-) diff --git a/src/downloader.c b/src/downloader.c index dd5d276..5831679 100644 --- a/src/downloader.c +++ b/src/downloader.c @@ -1,13 +1,5 @@ #include "downloader.h" -static void free_bucket_token(storj_download_state_t *state) -{ - if (state->token) { - free(state->token); - state->token = NULL; - } -} - static void free_exchange_report(storj_exchange_report_t *report) { free(report->data_hash); @@ -30,8 +22,6 @@ static void free_download_state(storj_download_state_t *state) free_exchange_report(pointer->report); } - free_bucket_token(state); - if (state->excluded_farmer_ids) { free(state->excluded_farmer_ids); } @@ -62,153 +52,6 @@ static void free_download_state(storj_download_state_t *state) free(state); } -static void request_token(uv_work_t *work) -{ - token_request_download_t *req = work->data; - storj_download_state_t *state = req->state; - - int path_len = 9 + strlen(req->bucket_id) + 7; - char *path = calloc(path_len + 1, sizeof(char)); - if (!path) { - req->error_status = STORJ_MEMORY_ERROR; - return; - } - - strcat(path, "/buckets/"); - strcat(path, req->bucket_id); - strcat(path, "/tokens"); - - struct json_object *body = json_object_new_object(); - json_object *op_string = json_object_new_string(req->bucket_op); - json_object_object_add(body, "operation", op_string); - - int status_code = 0; - struct json_object *response = NULL; - int request_status = fetch_json(req->http_options, - req->options, - "POST", - path, - body, - true, - NULL, - &response, - &status_code); - - if (request_status) { - req->error_status = STORJ_BRIDGE_REQUEST_ERROR; - - state->log->warn(state->env->log_options, state->handle, - "Request token error: %i", request_status); - - } else if (status_code == 201) { - struct json_object *token_value; - if (!json_object_object_get_ex(response, "token", &token_value)) { - req->error_status = STORJ_BRIDGE_JSON_ERROR; - } - - if (!json_object_is_type(token_value, json_type_string)) { - req->error_status = STORJ_BRIDGE_JSON_ERROR; - } - - char *token = (char *)json_object_get_string(token_value); - req->token = strdup(token); - - } else if (status_code == 403 || status_code == 401) { - req->error_status = STORJ_BRIDGE_AUTH_ERROR; - } else if (status_code == 404) { - req->error_status = STORJ_BRIDGE_BUCKET_NOTFOUND_ERROR; - } else if (status_code == 500) { - req->error_status = STORJ_BRIDGE_INTERNAL_ERROR; - } else { - req->error_status = STORJ_BRIDGE_REQUEST_ERROR; - } - - req->status_code = status_code; - - if (response) { - json_object_put(response); - } - json_object_put(body); - free(path); -} - -static void after_request_token(uv_work_t *work, int status) -{ - token_request_download_t *req = work->data; - - req->state->pending_work_count--; - req->state->requesting_token = false; - - if (status != 0) { - req->state->error_status = STORJ_BRIDGE_TOKEN_ERROR; - } else if (req->status_code == 201) { - req->state->token = req->token; - } else if (req->error_status){ - switch (req->error_status) { - case STORJ_BRIDGE_REQUEST_ERROR: - case STORJ_BRIDGE_INTERNAL_ERROR: - req->state->token_fail_count += 1; - break; - default: - req->state->error_status = req->error_status; - break; - } - if (req->state->token_fail_count >= STORJ_MAX_TOKEN_TRIES) { - req->state->token_fail_count = 0; - req->state->error_status = req->error_status; - } - - } else { - req->state->error_status = STORJ_BRIDGE_TOKEN_ERROR; - } - - queue_next_work(req->state); - - free(req); - free(work); -} - -static void queue_request_bucket_token(storj_download_state_t *state) -{ - if (state->requesting_token || state->canceled) { - return; - } - - uv_work_t *work = malloc(sizeof(uv_work_t)); - if (!work) { - state->error_status = STORJ_MEMORY_ERROR; - return; - } - - token_request_download_t *req = malloc(sizeof(token_request_download_t)); - if (!req) { - state->error_status = STORJ_MEMORY_ERROR; - return; - } - - req->http_options = state->env->http_options; - req->options = state->env->bridge_options; - req->bucket_id = state->bucket_id; - req->bucket_op = (char *)BUCKET_OP[BUCKET_PULL]; - req->state = state; - req->status_code = 0; - req->error_status = 0; - - work->data = req; - - state->pending_work_count++; - int status = uv_queue_work(state->env->loop, (uv_work_t*) work, - request_token, after_request_token); - - if (status) { - state->error_status = STORJ_QUEUE_ERROR; - return; - } - - state->requesting_token = true; - -} - static void request_pointers(uv_work_t *work) { json_request_download_t *req = work->data; @@ -216,7 +59,7 @@ static void request_pointers(uv_work_t *work) int status_code = 0; int request_status = fetch_json(req->http_options, req->options, req->method, - req->path, req->body, req->auth, req->token, + req->path, req->body, req->auth, &req->response, &status_code); @@ -263,7 +106,7 @@ static void request_replace_pointer(uv_work_t *work) strcat(path, query_args); int request_status = fetch_json(req->http_options, req->options, "GET", - path, NULL, NULL, req->token, + path, NULL, NULL, &req->response, &status_code); if (request_status) { @@ -509,8 +352,6 @@ static void after_request_pointers(uv_work_t *work, int status) json_object_to_json_string(req->response)); } - free_bucket_token(req->state); - if (status != 0) { state->error_status = STORJ_BRIDGE_POINTER_ERROR; @@ -564,8 +405,6 @@ static void after_request_replace_pointer(uv_work_t *work, int status) req->pointer_index, json_object_to_json_string(req->response)); - free_bucket_token(req->state); - if (status != 0) { state->error_status = STORJ_BRIDGE_REPOINTER_ERROR; @@ -687,7 +526,6 @@ static void queue_request_pointers(storj_download_state_t *state) req->http_options = state->env->http_options; req->options = state->env->bridge_options; - req->token = state->token; req->bucket_id = state->bucket_id; req->file_id = state->file_id; req->state = state; @@ -763,7 +601,6 @@ static void queue_request_pointers(storj_download_state_t *state) req->path = path; req->body = NULL; req->auth = true; - req->token = state->token; req->state = state; @@ -1085,7 +922,7 @@ static void send_exchange_report(uv_work_t *work) int request_status = fetch_json(req->http_options, req->options, "POST", "/reports/exchanges", body, - NULL, NULL, &response, &status_code); + true, &response, &status_code); if (request_status) { @@ -1378,7 +1215,6 @@ static void request_info(uv_work_t *work) path, NULL, true, - NULL, &response, &status_code); @@ -1940,13 +1776,7 @@ static void queue_next_work(storj_download_state_t *state) goto finish_up; } - if (!state->token) { - queue_request_bucket_token(state); - } - - if (state->token) { - queue_request_pointers(state); - } + queue_request_pointers(state); if (!state->info) { queue_request_info(state); @@ -1954,28 +1784,28 @@ static void queue_next_work(storj_download_state_t *state) if (state->info) { queue_request_shards(state); - } - queue_send_exchange_reports(state); - - if (state->rs) { - if (can_recover_shards(state)) { - queue_recover_shards(state); - } else { - state->error_status = STORJ_FILE_SHARD_MISSING_ERROR; - queue_next_work(state); - return; - } - } else { - if (!has_missing_shard(state)) { - queue_recover_shards(state); + if (state->rs) { + if (can_recover_shards(state)) { + queue_recover_shards(state); + } else { + state->error_status = STORJ_FILE_SHARD_MISSING_ERROR; + queue_next_work(state); + return; + } } else { - state->error_status = STORJ_FILE_SHARD_MISSING_ERROR; - queue_next_work(state); - return; + if (!has_missing_shard(state)) { + queue_recover_shards(state); + } else { + state->error_status = STORJ_FILE_SHARD_MISSING_ERROR; + queue_next_work(state); + return; + } } } + queue_send_exchange_reports(state); + finish_up: state->log->debug(state->env->log_options, state->handle, @@ -2041,9 +1871,6 @@ int storj_bridge_resolve_file(storj_env_t *env, state->requesting_pointers = false; state->error_status = STORJ_TRANSFER_OK; state->writing = false; - state->token = NULL; - state->requesting_token = false; - state->token_fail_count = 0; state->shard_size = 0; state->excluded_farmer_ids = NULL; state->hmac = NULL; diff --git a/src/downloader.h b/src/downloader.h index fdf3c15..aabb480 100644 --- a/src/downloader.h +++ b/src/downloader.h @@ -124,7 +124,6 @@ typedef struct { storj_http_options_t *http_options; storj_bridge_options_t *options; uint32_t pointer_index; - char *token; const char *bucket_id; const char *file_id; char *excluded_farmer_ids; @@ -144,7 +143,6 @@ typedef struct { char *method; char *path; bool auth; - char *token; struct json_object *body; struct json_object *response; /* state should not be modified in worker threads */ @@ -152,21 +150,6 @@ typedef struct { int status_code; } json_request_download_t; -/** @brief A structure for sharing data with worker threads for requesting - * a bucket operation token from the bridge. - */ -typedef struct { - storj_http_options_t *http_options; - storj_bridge_options_t *options; - char *token; - const char *bucket_id; - char *bucket_op; - /* state should not be modified in worker threads */ - storj_download_state_t *state; - int status_code; - int error_status; -} token_request_download_t; - /** @brief A method that determines the next work necessary to download a file * * This method is called after each individual work is complete, and will diff --git a/src/http.c b/src/http.c index 522e219..f726834 100644 --- a/src/http.c +++ b/src/http.c @@ -503,7 +503,6 @@ int fetch_json(storj_http_options_t *http_options, char *path, struct json_object *request_body, bool auth, - char *token, struct json_object **response, int *status_code) { @@ -602,17 +601,6 @@ int fetch_json(storj_http_options_t *http_options, } struct curl_slist *header_list = NULL; - if (token) { - char *token_header = calloc(9 + strlen(token) + 1, sizeof(char)); - if (!token_header) { - return 1; - } - strcat(token_header, "X-Token: "); - strcat(token_header, token); - header_list = curl_slist_append(header_list, token_header); - free(token_header); - curl_easy_setopt(curl, CURLOPT_HTTPHEADER, header_list); - } // Include body if request body json is provided http_body_send_t *post_body = NULL; diff --git a/src/http.h b/src/http.h index 3ac5b64..ee4cc6c 100644 --- a/src/http.h +++ b/src/http.h @@ -168,7 +168,6 @@ int fetch_json(storj_http_options_t *http_options, char *path, struct json_object *request_body, bool auth, - char *token, struct json_object **response, int *status_code); diff --git a/src/storj.c b/src/storj.c index 32913b6..444634f 100644 --- a/src/storj.c +++ b/src/storj.c @@ -12,7 +12,7 @@ static void json_request_worker(uv_work_t *work) req->error_code = fetch_json(req->http_options, req->options, req->method, req->path, req->body, - req->auth, NULL, &req->response, &status_code); + req->auth, &req->response, &status_code); req->status_code = status_code; } @@ -64,7 +64,7 @@ static void create_bucket_request_worker(uv_work_t *work) req->error_code = fetch_json(req->http_options, req->bridge_options, "POST", "/buckets", body, - true, NULL, &req->response, &status_code); + true, &req->response, &status_code); json_object_put(body); @@ -89,7 +89,7 @@ static void get_buckets_request_worker(uv_work_t *work) req->error_code = fetch_json(req->http_options, req->options, req->method, req->path, req->body, - req->auth, NULL, &req->response, &status_code); + req->auth, &req->response, &status_code); req->status_code = status_code; @@ -173,7 +173,7 @@ static void list_files_request_worker(uv_work_t *work) req->error_code = fetch_json(req->http_options, req->options, req->method, req->path, req->body, - req->auth, NULL, &req->response, &status_code); + req->auth, &req->response, &status_code); req->status_code = status_code; diff --git a/src/storj.h b/src/storj.h index dc2f223..af38115 100644 --- a/src/storj.h +++ b/src/storj.h @@ -410,9 +410,6 @@ typedef struct { bool requesting_pointers; int error_status; bool writing; - char *token; - bool requesting_token; - uint32_t token_fail_count; uint8_t *decrypt_key; uint8_t *decrypt_ctr; const char *hmac; diff --git a/src/uploader.c b/src/uploader.c index aaf61b7..3df0de2 100644 --- a/src/uploader.c +++ b/src/uploader.c @@ -439,7 +439,6 @@ static void create_bucket_entry(uv_work_t *work) path, body, true, - NULL, &req->response, &status_code); @@ -1069,7 +1068,6 @@ static void push_frame(uv_work_t *work) resource, body, true, - NULL, &response, &status_code); @@ -1712,7 +1710,6 @@ static void request_frame_id(uv_work_t *work) "/frames", body, true, - NULL, &response, &status_code); @@ -2050,7 +2047,7 @@ static void send_exchange_report(uv_work_t *work) int request_status = fetch_json(req->http_options, req->options, "POST", "/reports/exchanges", body, - NULL, NULL, &response, &status_code); + true, &response, &status_code); if (request_status) { @@ -2197,7 +2194,7 @@ static void verify_file_name(uv_work_t *work) req->error_code = fetch_json(req->http_options, req->options, req->method, req->path, req->body, - req->auth, NULL, &req->response, &status_code); + req->auth, &req->response, &status_code); req->status_code = status_code; } From 3377095cede7bcb6f9a7ad89f8d7a9d2ad0622df Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Thu, 25 May 2017 00:56:58 -0400 Subject: [PATCH 2/2] Reduce limit of pointers per request from 6 to 3 To help improve the timeout rate for this request. Setting a timeout to 20 seconds will often result in the client closing the connection before the request can be made, a timeout of 60 seconds does not have this issue. If we reduce the number of pointers that need to be requested, there are less requests that need to be made to the network. --- src/downloader.c | 2 +- test/mockbridge.c | 13 +++++++++++-- test/mockbridge.json | 16 +++++++++++----- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/downloader.c b/src/downloader.c index 5831679..8c0bf5a 100644 --- a/src/downloader.c +++ b/src/downloader.c @@ -579,7 +579,7 @@ static void queue_request_pointers(storj_download_state_t *state) char query_args[BUFSIZ]; memset(query_args, '\0', BUFSIZ); - snprintf(query_args, BUFSIZ, "?limit=6&skip=%d", state->total_pointers); + snprintf(query_args, BUFSIZ, "?limit=3&skip=%d", state->total_pointers); int path_len = 9 + strlen(state->bucket_id) + 7 + strlen(state->file_id) + strlen(query_args); diff --git a/test/mockbridge.c b/test/mockbridge.c index 8c0bd45..3b1c6b8 100644 --- a/test/mockbridge.c +++ b/test/mockbridge.c @@ -112,12 +112,21 @@ int mock_bridge_server(void *cls, if (!skip || 0 == strcmp(skip, "0")) { page = get_response_string(responses, "getfilepointers-0"); status_code = MHD_HTTP_OK; - } else if (0 == strcmp(skip, "6")) { + } else if (0 == strcmp(skip, "3")) { page = get_response_string(responses, "getfilepointers-1"); status_code = MHD_HTTP_OK; - } else if (0 == strcmp(skip, "12")) { + } else if (0 == strcmp(skip, "6")) { page = get_response_string(responses, "getfilepointers-2"); status_code = MHD_HTTP_OK; + } else if (0 == strcmp(skip, "9")) { + page = get_response_string(responses, "getfilepointers-3"); + status_code = MHD_HTTP_OK; + } else if (0 == strcmp(skip, "12")) { + page = get_response_string(responses, "getfilepointers-4"); + status_code = MHD_HTTP_OK; + } else if (0 == strcmp(skip, "15")) { + page = get_response_string(responses, "getfilepointers-5"); + status_code = MHD_HTTP_OK; } else if (0 == strcmp(skip, "4")) { // TODO check exclude and limit query page = get_response_string(responses, "getfilepointers-r"); diff --git a/test/mockbridge.json b/test/mockbridge.json index e687e56..279912a 100644 --- a/test/mockbridge.json +++ b/test/mockbridge.json @@ -305,7 +305,9 @@ "operation": "PULL", "index": 2, "size": 16777216 - }, + } + ], + "getfilepointers-1": [ { "token": "cc02fb5c9687c397fd1ead5a2e71239dcffb0f4c", "hash": "214ed86cb1287fe0fd18c174eecbf84341bf2655", @@ -352,7 +354,7 @@ "size": 16777216 } ], - "getfilepointers-1": [ + "getfilepointers-2": [ { "token": "de8e83dcf41789d66f2855259f35b729c9834eeb", "hash": "ebcbe78dd209a03d3ce29f2e5460304de2060031", @@ -387,7 +389,9 @@ "hash": "88c5e8885160c449b1dbb00ccf317067200b39a0", "index": 8, "size": 16777216 - }, + } + ], + "getfilepointers-3": [ { "token": "cc02fb5c9687c397fd1ead5a2e71239dcffb0f4c", "hash": "76b1a97498e026c47c924374b5b1148543d5c0ab", @@ -434,7 +438,7 @@ "size": 16777216 } ], - "getfilepointers-2": [ + "getfilepointers-4": [ { "token": "de8e83dcf41789d66f2855259f35b729c9834eeb", "hash": "973701b43290e3bef7007db0cb75744f9556ae3b", @@ -480,7 +484,9 @@ "index": 14, "parity": true, "size": 16777216 - }, + } + ], + "getfilepointers-5": [ { "token": "3b3f8ccb23ec47cec1ad57f7f68cab285dde54d6", "hash": "424cdf090604317570da38ef7d5b41abea0952df",