From b6b7bf1e4babf8823d8cdf86b9643cdb3775a6bb Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Thu, 30 Sep 2021 17:41:12 -0700 Subject: [PATCH 01/42] added option to output stats to json --- bindings/c/test/mako/mako.c | 3 +++ bindings/c/test/mako/mako.h | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 913564eef2..6e50cbe3e6 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1447,6 +1447,7 @@ int init_args(mako_args_t* args) { args->txnspec.ops[i][OP_COUNT] = 0; } args->disable_ryw = 0; + args->output_json = 0; return 0; } @@ -1612,6 +1613,7 @@ void usage() { printf("%-24s %s\n", " --flatbuffers", "Use flatbuffers"); printf("%-24s %s\n", " --streaming", "Streaming mode: all (default), iterator, small, medium, large, serial"); printf("%-24s %s\n", " --disable_ryw", "Disable snapshot read-your-writes"); + printf("%-24s %s\n", " --json", "Output stats to a json file"); } /* parse benchmark paramters */ @@ -1658,6 +1660,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "txntagging_prefix", required_argument, NULL, ARG_TXNTAGGINGPREFIX }, { "version", no_argument, NULL, ARG_VERSION }, { "disable_ryw", no_argument, NULL, ARG_DISABLE_RYW }, + { "json", no_argument, NULL, ARG_JSON_OUTPUT }, { NULL, 0, NULL, 0 } }; idx = 0; diff --git a/bindings/c/test/mako/mako.h b/bindings/c/test/mako/mako.h index a770fea857..b64eaecbaf 100644 --- a/bindings/c/test/mako/mako.h +++ b/bindings/c/test/mako/mako.h @@ -81,7 +81,8 @@ enum Arguments { ARG_TXNTAGGING, ARG_TXNTAGGINGPREFIX, ARG_STREAMING_MODE, - ARG_DISABLE_RYW + ARG_DISABLE_RYW, + ARG_JSON_OUTPUT }; enum TPSChangeTypes { TPS_SIN, TPS_SQUARE, TPS_PULSE }; @@ -138,6 +139,7 @@ typedef struct { char txntagging_prefix[TAGPREFIXLENGTH_MAX]; FDBStreamingMode streaming_mode; int disable_ryw; + int output_json; } mako_args_t; /* shared memory */ From 88a30399f36b545fea6dd7e21711059e5d06d856 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Thu, 30 Sep 2021 23:06:23 -0700 Subject: [PATCH 02/42] implemented outputting results to json file --- bindings/c/test/mako/mako.c | 217 ++++++++++++++++++++++++++++-------- bindings/c/test/mako/mako.h | 3 +- 2 files changed, 169 insertions(+), 51 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 6e50cbe3e6..d1174cbcee 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -1660,7 +1661,6 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "txntagging_prefix", required_argument, NULL, ARG_TXNTAGGINGPREFIX }, { "version", no_argument, NULL, ARG_VERSION }, { "disable_ryw", no_argument, NULL, ARG_DISABLE_RYW }, - { "json", no_argument, NULL, ARG_JSON_OUTPUT }, { NULL, 0, NULL, 0 } }; idx = 0; @@ -1713,6 +1713,9 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { args->mode = MODE_RUN; } break; + case 'j': + args->output_json = 1; + break; case ARG_KEYLEN: args->key_length = atoi(optarg); break; @@ -1844,6 +1847,41 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { return 0; } +char* get_ops_name(int ops_code) { + switch(ops_code) { + case OP_GETREADVERSION: + return "GRV"; + case OP_GET: + return "GET"; + case OP_GETRANGE: + return "GETRANGE"; + case OP_SGET: + return "SGET"; + case OP_SGETRANGE: + return "SGETRANGE"; + case OP_UPDATE: + return "UPDATE"; + case OP_INSERT: + return "INSERT"; + case OP_INSERTRANGE: + return "INSERTRANGE"; + case OP_CLEAR: + return "CLEAR"; + case OP_SETCLEAR: + return "SETCLEAR"; + case OP_CLEARRANGE: + return "CLEARRANGE"; + case OP_SETCLEARRANGE: + return "SETCLEARRANGE"; + case OP_COMMIT: + return "COMMIT"; + case OP_TRANSACTION: + return "TRANSACTION"; + default: + return ""; + } +} + int validate_args(mako_args_t* args) { if (args->mode == MODE_INVALID) { fprintf(stderr, "ERROR: --mode has to be set\n"); @@ -1891,7 +1929,7 @@ int validate_args(mako_args_t* args) { #define STATS_TITLE_WIDTH 12 #define STATS_FIELD_WIDTH 12 -void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, struct timespec* prev) { +void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, struct timespec* prev, FILE* fp) { int i, j; int op; int print_err; @@ -1916,10 +1954,18 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s } } } + + if (fp) { + fwrite("{", 1, 1, fp); + } printf("%" STR(STATS_TITLE_WIDTH) "s ", "OPS"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0) { - printf("%" STR(STATS_FIELD_WIDTH) "lld ", ops_total[op] - ops_total_prev[op]); + uint64_t ops_total_diff = ops_total[op] - ops_total_prev[op]; + printf("%" STR(STATS_FIELD_WIDTH) "lld ", ops_total_diff); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), ops_total_diff); + } errors_diff[op] = errors_total[op] - errors_total_prev[op]; print_err = (errors_diff[op] > 0); ops_total_prev[op] = ops_total[op]; @@ -1927,11 +1973,22 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s } } /* TPS */ - printf("%" STR(STATS_FIELD_WIDTH) ".2f ", (totalxacts - totalxacts_prev) * 1000000000.0 / durationns); + double tps = (totalxacts - totalxacts_prev) * 1000000000.0 / durationns; + printf("%" STR(STATS_FIELD_WIDTH) ".2f ", tps); + if (fp) { + // char* str = NULL; + // sprintf(str, "\"TPS\": %.2f,", tps); + // fwrite(str, 1, strlen(str), fp); + fprintf(fp, "\"TPS\": %.2f,", tps); + } totalxacts_prev = totalxacts; /* Conflicts */ - printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", (conflicts - conflicts_prev) * 1000000000.0 / durationns); + double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / durationns; + printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_diff); + if (fp) { + fprintf(fp, "\"Conflicts\": %.2f},", conflicts_diff); + } conflicts_prev = conflicts; if (print_err) { @@ -1939,10 +1996,14 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0) { printf("%" STR(STATS_FIELD_WIDTH) "lld ", errors_diff[op]); + if (fp) { + fprintf(fp, "\"Errors\": %.2f", conflicts_diff); + } } } printf("\n"); } + return; } @@ -1956,44 +2017,7 @@ void print_stats_header(mako_args_t* args, bool show_commit, bool is_first_heade printf(" "); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0) { - switch (op) { - case OP_GETREADVERSION: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "GRV"); - break; - case OP_GET: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "GET"); - break; - case OP_GETRANGE: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "GETRANGE"); - break; - case OP_SGET: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "SGET"); - break; - case OP_SGETRANGE: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "SGETRANGE"); - break; - case OP_UPDATE: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "UPDATE"); - break; - case OP_INSERT: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "INSERT"); - break; - case OP_INSERTRANGE: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "INSERTRANGE"); - break; - case OP_CLEAR: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "CLEAR"); - break; - case OP_SETCLEAR: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "SETCLEAR"); - break; - case OP_CLEARRANGE: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "CLEARRANGE"); - break; - case OP_SETCLEARRANGE: - printf("%" STR(STATS_FIELD_WIDTH) "s ", "SETCLRRANGE"); - break; - } + printf("%" STR(STATS_FIELD_WIDTH) "s ", get_ops_name(op)); } } @@ -2046,7 +2070,8 @@ void print_report(mako_args_t* args, mako_stats_t* stats, struct timespec* timer_now, struct timespec* timer_start, - pid_t* pid_main) { + pid_t* pid_main, + FILE* fp) { int i, j, k, op, index; uint64_t totalxacts = 0; uint64_t conflicts = 0; @@ -2092,7 +2117,8 @@ void print_report(mako_args_t* args, } /* overall stats */ - printf("\n====== Total Duration %6.3f sec ======\n\n", (double)durationns / 1000000000); + double total_duration = durationns * 1.0 / 1000000000; + printf("\n====== Total Duration %6.3f sec ======\n\n", total_duration); printf("Total Processes: %8d\n", args->num_processes); printf("Total Threads: %8d\n", args->num_threads); if (args->tpsmax == args->tpsmin) @@ -2119,30 +2145,60 @@ void print_report(mako_args_t* args, printf("Total Errors: %8lld\n", totalerrors); printf("Overall TPS: %8lld\n\n", totalxacts * 1000000000 / durationns); + if (fp) { + fprintf(fp, "\"results\": {"); + fprintf(fp, "\"Total Duration\": %6.3f,", total_duration); + fprintf(fp, "\"Total Processes\": %8d,", args->num_processes); + fprintf(fp, "\"Total Threads\": %8d,", args->num_threads); + fprintf(fp, "\"Target TPS\": %8d,", args->tpsmax); + fprintf(fp, "\"Total Xacts\": %8lld,", totalxacts); + fprintf(fp, "\"Total Conflicts\": %8lld,", conflicts); + fprintf(fp, "\"Total Errors\": %8lld,", totalerrors); + fprintf(fp, "\"Overall TPS\": %8lld,", totalxacts * 1000000000 / durationns); + } + /* per-op stats */ print_stats_header(args, true, true, false); /* OPS */ printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Total OPS"); + if (fp) { + fprintf(fp, "\"Total OPS\": {"); + } for (op = 0; op < MAX_OP; op++) { if ((args->txnspec.ops[op][OP_COUNT] > 0 && op != OP_TRANSACTION) || op == OP_COMMIT) { printf("%" STR(STATS_FIELD_WIDTH) "lld ", ops_total[op]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), ops_total[op]); + } } } /* TPS */ - printf("%" STR(STATS_FIELD_WIDTH) ".2f ", totalxacts * 1000000000.0 / durationns); + double tps = totalxacts * 1000000000.0 / durationns; + printf("%" STR(STATS_FIELD_WIDTH) ".2f ", tps); /* Conflicts */ - printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts * 1000000000.0 / durationns); + double conflicts_rate = conflicts * 1000000000.0 / durationns; + printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_rate); + + if (fp) { + fprintf(fp, "}, \"TPS\": %.2f, \"Conflicts\": %.2f, \"Errors\": {", tps, conflicts_rate); + } /* Errors */ printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Errors"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 && op != OP_TRANSACTION) { printf("%" STR(STATS_FIELD_WIDTH) "lld ", errors_total[op]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), errors_total[op]); + } } } + if (fp) { + fprintf(fp, "}, \"Latency (us)\": {"); + } printf("\n\n"); printf("%s", "Latency (us)"); @@ -2157,11 +2213,17 @@ void print_report(mako_args_t* args, } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_samples[op]); + } } } printf("\n"); /* Min Latency */ + if (fp) { + fprintf(fp, "}, \"Min\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Min"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { @@ -2170,11 +2232,17 @@ void print_report(mako_args_t* args, } else { printf("%" STR(STATS_FIELD_WIDTH) "lld ", lat_min[op]); } + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_min[op]); + } } } printf("\n"); /* Avg Latency */ + if (fp) { + fprintf(fp, "}, \"Avg\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Avg"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { @@ -2183,11 +2251,17 @@ void print_report(mako_args_t* args, } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_total[op] / lat_samples[op]); + } } } printf("\n"); /* Max Latency */ + if (fp) { + fprintf(fp, "}, \"Max\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Max"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { @@ -2196,6 +2270,9 @@ void print_report(mako_args_t* args, } else { printf("%" STR(STATS_FIELD_WIDTH) "lld ", lat_max[op]); } + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_max[op]); + } } } printf("\n"); @@ -2205,6 +2282,9 @@ void print_report(mako_args_t* args, int point_99_9pct, point_99pct, point_95pct; /* Median Latency */ + if (fp) { + fprintf(fp, "}, \"Median\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Median"); int num_points[MAX_OP] = { 0 }; for (op = 0; op < MAX_OP; op++) { @@ -2241,6 +2321,9 @@ void print_report(mako_args_t* args, median = (dataPoints[op][num_points[op] / 2] + dataPoints[op][num_points[op] / 2 - 1]) >> 1; } printf("%" STR(STATS_FIELD_WIDTH) "lld ", median); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), median); + } } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } @@ -2249,6 +2332,9 @@ void print_report(mako_args_t* args, printf("\n"); /* 95%ile Latency */ + if (fp) { + fprintf(fp, "}, \"p95\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "95.0 pctile"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { @@ -2259,6 +2345,9 @@ void print_report(mako_args_t* args, if (lat_total[op]) { point_95pct = ((float)(num_points[op]) * 0.95) - 1; printf("%" STR(STATS_FIELD_WIDTH) "lld ", dataPoints[op][point_95pct]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), dataPoints[op][point_95pct]); + } } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } @@ -2267,6 +2356,9 @@ void print_report(mako_args_t* args, printf("\n"); /* 99%ile Latency */ + if (fp) { + fprintf(fp, "}, \"p99\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "99.0 pctile"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { @@ -2277,6 +2369,9 @@ void print_report(mako_args_t* args, if (lat_total[op]) { point_99pct = ((float)(num_points[op]) * 0.99) - 1; printf("%" STR(STATS_FIELD_WIDTH) "lld ", dataPoints[op][point_99pct]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), dataPoints[op][point_99pct]); + } } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } @@ -2285,6 +2380,9 @@ void print_report(mako_args_t* args, printf("\n"); /* 99.9%ile Latency */ + if (fp) { + fprintf(fp, "}, \"p99.9\": {"); + } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "99.9 pctile"); for (op = 0; op < MAX_OP; op++) { if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { @@ -2295,12 +2393,18 @@ void print_report(mako_args_t* args, if (lat_total[op]) { point_99_9pct = ((float)(num_points[op]) * 0.999) - 1; printf("%" STR(STATS_FIELD_WIDTH) "lld ", dataPoints[op][point_99_9pct]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), dataPoints[op][point_99_9pct]); + } } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } } } printf("\n"); + if (fp) { + fprintf(fp, "}}"); + } char command_remove[NAME_MAX] = { '\0' }; sprintf(command_remove, "rm -rf %s%d", TEMP_DATA_STORE, *pid_main); @@ -2331,6 +2435,12 @@ int stats_process_main(mako_args_t* args, if (args->verbose >= VERBOSE_DEFAULT) print_stats_header(args, false, true, false); + FILE *fp = NULL; + if (args->output_json) { + fp = fopen("mako.json", "w"); + fprintf(fp, "{\"samples\": ["); + } + clock_gettime(CLOCK_MONOTONIC_COARSE, &timer_start); timer_prev.tv_sec = timer_start.tv_sec; timer_prev.tv_nsec = timer_start.tv_nsec; @@ -2372,19 +2482,28 @@ int stats_process_main(mako_args_t* args, } if (args->verbose >= VERBOSE_DEFAULT) - print_stats(args, stats, &timer_now, &timer_prev); + print_stats(args, stats, &timer_now, &timer_prev, fp); timer_prev.tv_sec = timer_now.tv_sec; timer_prev.tv_nsec = timer_now.tv_nsec; } } + if (args->output_json) { + fprintf(fp, "],"); + } + /* print report */ if (args->verbose >= VERBOSE_DEFAULT) { clock_gettime(CLOCK_MONOTONIC_COARSE, &timer_now); while (*stopcount < args->num_threads * args->num_processes) { usleep(10000); /* 10ms */ } - print_report(args, stats, &timer_now, &timer_start, pid_main); + print_report(args, stats, &timer_now, &timer_start, pid_main, fp); + } + + if (args->output_json) { + fprintf(fp, "}"); + fclose(fp); } return 0; diff --git a/bindings/c/test/mako/mako.h b/bindings/c/test/mako/mako.h index b64eaecbaf..00818e8907 100644 --- a/bindings/c/test/mako/mako.h +++ b/bindings/c/test/mako/mako.h @@ -81,8 +81,7 @@ enum Arguments { ARG_TXNTAGGING, ARG_TXNTAGGINGPREFIX, ARG_STREAMING_MODE, - ARG_DISABLE_RYW, - ARG_JSON_OUTPUT + ARG_DISABLE_RYW }; enum TPSChangeTypes { TPS_SIN, TPS_SQUARE, TPS_PULSE }; From 48a4204668bd30e65b6aef2258b0d429b6692a6c Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Thu, 30 Sep 2021 23:08:10 -0700 Subject: [PATCH 03/42] formatting --- bindings/c/test/mako/mako.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index d1174cbcee..91c045a611 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1848,7 +1848,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { } char* get_ops_name(int ops_code) { - switch(ops_code) { + switch (ops_code) { case OP_GETREADVERSION: return "GRV"; case OP_GET: @@ -2071,7 +2071,7 @@ void print_report(mako_args_t* args, struct timespec* timer_now, struct timespec* timer_start, pid_t* pid_main, - FILE* fp) { + FILE* fp) { int i, j, k, op, index; uint64_t totalxacts = 0; uint64_t conflicts = 0; @@ -2435,7 +2435,7 @@ int stats_process_main(mako_args_t* args, if (args->verbose >= VERBOSE_DEFAULT) print_stats_header(args, false, true, false); - FILE *fp = NULL; + FILE* fp = NULL; if (args->output_json) { fp = fopen("mako.json", "w"); fprintf(fp, "{\"samples\": ["); From 6d8e924ac2fa07aa403617f81d93acfd3a5492c2 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Fri, 1 Oct 2021 13:57:02 -0700 Subject: [PATCH 04/42] json formatting --- bindings/c/test/mako/mako.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 91c045a611..2687131e8e 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -2148,13 +2148,13 @@ void print_report(mako_args_t* args, if (fp) { fprintf(fp, "\"results\": {"); fprintf(fp, "\"Total Duration\": %6.3f,", total_duration); - fprintf(fp, "\"Total Processes\": %8d,", args->num_processes); - fprintf(fp, "\"Total Threads\": %8d,", args->num_threads); - fprintf(fp, "\"Target TPS\": %8d,", args->tpsmax); - fprintf(fp, "\"Total Xacts\": %8lld,", totalxacts); - fprintf(fp, "\"Total Conflicts\": %8lld,", conflicts); - fprintf(fp, "\"Total Errors\": %8lld,", totalerrors); - fprintf(fp, "\"Overall TPS\": %8lld,", totalxacts * 1000000000 / durationns); + fprintf(fp, "\"Total Processes\": %d,", args->num_processes); + fprintf(fp, "\"Total Threads\": %d,", args->num_threads); + fprintf(fp, "\"Target TPS\": %d,", args->tpsmax); + fprintf(fp, "\"Total Xacts\": %lld,", totalxacts); + fprintf(fp, "\"Total Conflicts\": %lld,", conflicts); + fprintf(fp, "\"Total Errors\": %lld,", totalerrors); + fprintf(fp, "\"Overall TPS\": %lld,", totalxacts * 1000000000 / durationns); } /* per-op stats */ From 5876d8c410594ab2168e81452a070832d81b3067 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Fri, 1 Oct 2021 14:34:27 -0700 Subject: [PATCH 05/42] --json takes a file path --- bindings/c/test/mako/mako.c | 16 ++++++++-------- bindings/c/test/mako/mako.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 2687131e8e..8f92c1caf0 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1448,7 +1448,7 @@ int init_args(mako_args_t* args) { args->txnspec.ops[i][OP_COUNT] = 0; } args->disable_ryw = 0; - args->output_json = 0; + args->json_output_path[0] = '\0'; return 0; } @@ -1614,7 +1614,7 @@ void usage() { printf("%-24s %s\n", " --flatbuffers", "Use flatbuffers"); printf("%-24s %s\n", " --streaming", "Streaming mode: all (default), iterator, small, medium, large, serial"); printf("%-24s %s\n", " --disable_ryw", "Disable snapshot read-your-writes"); - printf("%-24s %s\n", " --json", "Output stats to a json file"); + printf("%-24s %s\n", " --json=PATH", "Output stats to the specified json file"); } /* parse benchmark paramters */ @@ -1649,9 +1649,9 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "trace_format", required_argument, NULL, ARG_TRACEFORMAT }, { "streaming", required_argument, NULL, ARG_STREAMING_MODE }, { "txntrace", required_argument, NULL, ARG_TXNTRACE }, + { "json", required_argument, NULL, 'j' }, /* no args */ { "help", no_argument, NULL, 'h' }, - { "json", no_argument, NULL, 'j' }, { "zipf", no_argument, NULL, 'z' }, { "commitget", no_argument, NULL, ARG_COMMITGET }, { "flatbuffers", no_argument, NULL, ARG_FLATBUFFERS }, @@ -1714,7 +1714,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { } break; case 'j': - args->output_json = 1; + strcpy(args->json_output_path, optarg); break; case ARG_KEYLEN: args->key_length = atoi(optarg); @@ -2436,8 +2436,8 @@ int stats_process_main(mako_args_t* args, print_stats_header(args, false, true, false); FILE* fp = NULL; - if (args->output_json) { - fp = fopen("mako.json", "w"); + if (args->json_output_path[0] != '\0') { + fp = fopen(args->json_output_path, "w"); fprintf(fp, "{\"samples\": ["); } @@ -2488,7 +2488,7 @@ int stats_process_main(mako_args_t* args, } } - if (args->output_json) { + if (fp) { fprintf(fp, "],"); } @@ -2501,7 +2501,7 @@ int stats_process_main(mako_args_t* args, print_report(args, stats, &timer_now, &timer_start, pid_main, fp); } - if (args->output_json) { + if (fp) { fprintf(fp, "}"); fclose(fp); } diff --git a/bindings/c/test/mako/mako.h b/bindings/c/test/mako/mako.h index 00818e8907..d7ff9efaad 100644 --- a/bindings/c/test/mako/mako.h +++ b/bindings/c/test/mako/mako.h @@ -138,7 +138,7 @@ typedef struct { char txntagging_prefix[TAGPREFIXLENGTH_MAX]; FDBStreamingMode streaming_mode; int disable_ryw; - int output_json; + char json_output_path[PATH_MAX]; } mako_args_t; /* shared memory */ From 128e1c985da8ca2ad4ab2bfc4fc2ebbdb6f2c1a4 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Fri, 1 Oct 2021 14:38:31 -0700 Subject: [PATCH 06/42] followed google json formatting --- bindings/c/test/mako/mako.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 8f92c1caf0..c29e7bbe1e 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1979,7 +1979,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s // char* str = NULL; // sprintf(str, "\"TPS\": %.2f,", tps); // fwrite(str, 1, strlen(str), fp); - fprintf(fp, "\"TPS\": %.2f,", tps); + fprintf(fp, "\"tps\": %.2f,", tps); } totalxacts_prev = totalxacts; @@ -1987,7 +1987,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / durationns; printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_diff); if (fp) { - fprintf(fp, "\"Conflicts\": %.2f},", conflicts_diff); + fprintf(fp, "\"conflicts\": %.2f},", conflicts_diff); } conflicts_prev = conflicts; @@ -1997,7 +1997,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s if (args->txnspec.ops[op][OP_COUNT] > 0) { printf("%" STR(STATS_FIELD_WIDTH) "lld ", errors_diff[op]); if (fp) { - fprintf(fp, "\"Errors\": %.2f", conflicts_diff); + fprintf(fp, "\"errors\": %.2f", conflicts_diff); } } } @@ -2147,14 +2147,14 @@ void print_report(mako_args_t* args, if (fp) { fprintf(fp, "\"results\": {"); - fprintf(fp, "\"Total Duration\": %6.3f,", total_duration); - fprintf(fp, "\"Total Processes\": %d,", args->num_processes); - fprintf(fp, "\"Total Threads\": %d,", args->num_threads); - fprintf(fp, "\"Target TPS\": %d,", args->tpsmax); - fprintf(fp, "\"Total Xacts\": %lld,", totalxacts); - fprintf(fp, "\"Total Conflicts\": %lld,", conflicts); - fprintf(fp, "\"Total Errors\": %lld,", totalerrors); - fprintf(fp, "\"Overall TPS\": %lld,", totalxacts * 1000000000 / durationns); + fprintf(fp, "\"totalDuration\": %6.3f,", total_duration); + fprintf(fp, "\"totalProcesses\": %d,", args->num_processes); + fprintf(fp, "\"totalThreads\": %d,", args->num_threads); + fprintf(fp, "\"targetTPS\": %d,", args->tpsmax); + fprintf(fp, "\"totalXacts\": %lld,", totalxacts); + fprintf(fp, "\"totalConflicts\": %lld,", conflicts); + fprintf(fp, "\"totalErrors\": %lld,", totalerrors); + fprintf(fp, "\"overallTPS\": %lld,", totalxacts * 1000000000 / durationns); } /* per-op stats */ @@ -2163,7 +2163,7 @@ void print_report(mako_args_t* args, /* OPS */ printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Total OPS"); if (fp) { - fprintf(fp, "\"Total OPS\": {"); + fprintf(fp, "\"totalOps\": {"); } for (op = 0; op < MAX_OP; op++) { if ((args->txnspec.ops[op][OP_COUNT] > 0 && op != OP_TRANSACTION) || op == OP_COMMIT) { @@ -2183,7 +2183,7 @@ void print_report(mako_args_t* args, printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_rate); if (fp) { - fprintf(fp, "}, \"TPS\": %.2f, \"Conflicts\": %.2f, \"Errors\": {", tps, conflicts_rate); + fprintf(fp, "}, \"tps\": %.2f, \"conflicts\": %.2f, \"errors\": {", tps, conflicts_rate); } /* Errors */ @@ -2197,7 +2197,7 @@ void print_report(mako_args_t* args, } } if (fp) { - fprintf(fp, "}, \"Latency (us)\": {"); + fprintf(fp, "}, \"latency\": {"); } printf("\n\n"); @@ -2222,7 +2222,7 @@ void print_report(mako_args_t* args, /* Min Latency */ if (fp) { - fprintf(fp, "}, \"Min\": {"); + fprintf(fp, "}, \"min\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Min"); for (op = 0; op < MAX_OP; op++) { @@ -2241,7 +2241,7 @@ void print_report(mako_args_t* args, /* Avg Latency */ if (fp) { - fprintf(fp, "}, \"Avg\": {"); + fprintf(fp, "}, \"avg\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Avg"); for (op = 0; op < MAX_OP; op++) { @@ -2260,7 +2260,7 @@ void print_report(mako_args_t* args, /* Max Latency */ if (fp) { - fprintf(fp, "}, \"Max\": {"); + fprintf(fp, "}, \"max\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Max"); for (op = 0; op < MAX_OP; op++) { @@ -2283,7 +2283,7 @@ void print_report(mako_args_t* args, /* Median Latency */ if (fp) { - fprintf(fp, "}, \"Median\": {"); + fprintf(fp, "}, \"median\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Median"); int num_points[MAX_OP] = { 0 }; From 695be07705bf27f080d69cfb6e1aa3f3c6879b15 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Fri, 1 Oct 2021 14:39:54 -0700 Subject: [PATCH 07/42] formatting --- bindings/c/test/mako/mako.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index c29e7bbe1e..d62114a2ff 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1649,7 +1649,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "trace_format", required_argument, NULL, ARG_TRACEFORMAT }, { "streaming", required_argument, NULL, ARG_STREAMING_MODE }, { "txntrace", required_argument, NULL, ARG_TXNTRACE }, - { "json", required_argument, NULL, 'j' }, + { "json", required_argument, NULL, 'j' }, /* no args */ { "help", no_argument, NULL, 'h' }, { "zipf", no_argument, NULL, 'z' }, From a484845877e1b827548a390a16711c03ccb6a8aa Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Fri, 1 Oct 2021 14:55:31 -0700 Subject: [PATCH 08/42] renamed json properties --- bindings/c/test/mako/mako.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index d62114a2ff..15658caf82 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1987,7 +1987,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / durationns; printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_diff); if (fp) { - fprintf(fp, "\"conflicts\": %.2f},", conflicts_diff); + fprintf(fp, "\"conflictsPerSec\": %.2f},", conflicts_diff); } conflicts_prev = conflicts; @@ -2183,7 +2183,7 @@ void print_report(mako_args_t* args, printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_rate); if (fp) { - fprintf(fp, "}, \"tps\": %.2f, \"conflicts\": %.2f, \"errors\": {", tps, conflicts_rate); + fprintf(fp, "}, \"tps\": %.2f, \"conflictsPerSec\": %.2f, \"errors\": {", tps, conflicts_rate); } /* Errors */ @@ -2197,7 +2197,7 @@ void print_report(mako_args_t* args, } } if (fp) { - fprintf(fp, "}, \"latency\": {"); + fprintf(fp, "}, \"numSamples\": {"); } printf("\n\n"); @@ -2222,7 +2222,7 @@ void print_report(mako_args_t* args, /* Min Latency */ if (fp) { - fprintf(fp, "}, \"min\": {"); + fprintf(fp, "}, \"minLatency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Min"); for (op = 0; op < MAX_OP; op++) { @@ -2241,7 +2241,7 @@ void print_report(mako_args_t* args, /* Avg Latency */ if (fp) { - fprintf(fp, "}, \"avg\": {"); + fprintf(fp, "}, \"avgLatency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Avg"); for (op = 0; op < MAX_OP; op++) { @@ -2260,7 +2260,7 @@ void print_report(mako_args_t* args, /* Max Latency */ if (fp) { - fprintf(fp, "}, \"max\": {"); + fprintf(fp, "}, \"maxLatency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Max"); for (op = 0; op < MAX_OP; op++) { @@ -2283,7 +2283,7 @@ void print_report(mako_args_t* args, /* Median Latency */ if (fp) { - fprintf(fp, "}, \"median\": {"); + fprintf(fp, "}, \"medianLatency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "Median"); int num_points[MAX_OP] = { 0 }; @@ -2333,7 +2333,7 @@ void print_report(mako_args_t* args, /* 95%ile Latency */ if (fp) { - fprintf(fp, "}, \"p95\": {"); + fprintf(fp, "}, \"p95Latency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "95.0 pctile"); for (op = 0; op < MAX_OP; op++) { @@ -2357,7 +2357,7 @@ void print_report(mako_args_t* args, /* 99%ile Latency */ if (fp) { - fprintf(fp, "}, \"p99\": {"); + fprintf(fp, "}, \"p99Latency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "99.0 pctile"); for (op = 0; op < MAX_OP; op++) { @@ -2381,7 +2381,7 @@ void print_report(mako_args_t* args, /* 99.9%ile Latency */ if (fp) { - fprintf(fp, "}, \"p99.9\": {"); + fprintf(fp, "}, \"p99.9Latency\": {"); } printf("%-" STR(STATS_TITLE_WIDTH) "s ", "99.9 pctile"); for (op = 0; op < MAX_OP; op++) { From b1cb343c248e991b8f7841a07222f69d0aa25d14 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Fri, 1 Oct 2021 15:03:22 -0700 Subject: [PATCH 09/42] fixed typo --- bindings/c/test/mako/mako.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 15658caf82..8c7b66b482 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1942,7 +1942,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s uint64_t totalxacts = 0; static uint64_t conflicts_prev = 0; uint64_t conflicts = 0; - double durationns = (now->tv_sec - prev->tv_sec) * 1000000000.0 + (now->tv_nsec - prev->tv_nsec); + double duration = (now->tv_sec - prev->tv_sec) * 1000000000.0 + (now->tv_nsec - prev->tv_nsec); for (i = 0; i < args->num_processes; i++) { for (j = 0; j < args->num_threads; j++) { @@ -1973,7 +1973,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s } } /* TPS */ - double tps = (totalxacts - totalxacts_prev) * 1000000000.0 / durationns; + double tps = (totalxacts - totalxacts_prev) * 1000000000.0 / duration; printf("%" STR(STATS_FIELD_WIDTH) ".2f ", tps); if (fp) { // char* str = NULL; @@ -1984,7 +1984,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s totalxacts_prev = totalxacts; /* Conflicts */ - double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / durationns; + double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / duration; printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_diff); if (fp) { fprintf(fp, "\"conflictsPerSec\": %.2f},", conflicts_diff); @@ -2083,7 +2083,7 @@ void print_report(mako_args_t* args, uint64_t lat_samples[MAX_OP] = { 0 }; uint64_t lat_max[MAX_OP] = { 0 }; - uint64_t durationns = + uint64_t duration = (timer_now->tv_sec - timer_start->tv_sec) * 1000000000 + (timer_now->tv_nsec - timer_start->tv_nsec); for (op = 0; op < MAX_OP; op++) { @@ -2117,7 +2117,7 @@ void print_report(mako_args_t* args, } /* overall stats */ - double total_duration = durationns * 1.0 / 1000000000; + double total_duration = duration * 1.0 / 1000000000; printf("\n====== Total Duration %6.3f sec ======\n\n", total_duration); printf("Total Processes: %8d\n", args->num_processes); printf("Total Threads: %8d\n", args->num_threads); @@ -2143,7 +2143,7 @@ void print_report(mako_args_t* args, printf("Total Xacts: %8lld\n", totalxacts); printf("Total Conflicts: %8lld\n", conflicts); printf("Total Errors: %8lld\n", totalerrors); - printf("Overall TPS: %8lld\n\n", totalxacts * 1000000000 / durationns); + printf("Overall TPS: %8lld\n\n", totalxacts * 1000000000 / duration); if (fp) { fprintf(fp, "\"results\": {"); @@ -2154,7 +2154,7 @@ void print_report(mako_args_t* args, fprintf(fp, "\"totalXacts\": %lld,", totalxacts); fprintf(fp, "\"totalConflicts\": %lld,", conflicts); fprintf(fp, "\"totalErrors\": %lld,", totalerrors); - fprintf(fp, "\"overallTPS\": %lld,", totalxacts * 1000000000 / durationns); + fprintf(fp, "\"overallTPS\": %lld,", totalxacts * 1000000000 / duration); } /* per-op stats */ @@ -2175,11 +2175,11 @@ void print_report(mako_args_t* args, } /* TPS */ - double tps = totalxacts * 1000000000.0 / durationns; + double tps = totalxacts * 1000000000.0 / duration; printf("%" STR(STATS_FIELD_WIDTH) ".2f ", tps); /* Conflicts */ - double conflicts_rate = conflicts * 1000000000.0 / durationns; + double conflicts_rate = conflicts * 1000000000.0 / duration; printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_rate); if (fp) { From 2a8a5e0142502cdf23e7806f444fbe7e3e843977 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Sat, 2 Oct 2021 22:16:47 -0700 Subject: [PATCH 10/42] cleanup -- addressed comments --- bindings/c/test/mako/mako.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 8c7b66b482..a75c1f26e6 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1942,7 +1942,7 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s uint64_t totalxacts = 0; static uint64_t conflicts_prev = 0; uint64_t conflicts = 0; - double duration = (now->tv_sec - prev->tv_sec) * 1000000000.0 + (now->tv_nsec - prev->tv_nsec); + double duration_nsec = (now->tv_sec - prev->tv_sec) * 1000000000.0 + (now->tv_nsec - prev->tv_nsec); for (i = 0; i < args->num_processes; i++) { for (j = 0; j < args->num_threads; j++) { @@ -1973,18 +1973,15 @@ void print_stats(mako_args_t* args, mako_stats_t* stats, struct timespec* now, s } } /* TPS */ - double tps = (totalxacts - totalxacts_prev) * 1000000000.0 / duration; + double tps = (totalxacts - totalxacts_prev) * 1000000000.0 / duration_nsec; printf("%" STR(STATS_FIELD_WIDTH) ".2f ", tps); if (fp) { - // char* str = NULL; - // sprintf(str, "\"TPS\": %.2f,", tps); - // fwrite(str, 1, strlen(str), fp); fprintf(fp, "\"tps\": %.2f,", tps); } totalxacts_prev = totalxacts; /* Conflicts */ - double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / duration; + double conflicts_diff = (conflicts - conflicts_prev) * 1000000000.0 / duration_nsec; printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_diff); if (fp) { fprintf(fp, "\"conflictsPerSec\": %.2f},", conflicts_diff); @@ -2083,7 +2080,7 @@ void print_report(mako_args_t* args, uint64_t lat_samples[MAX_OP] = { 0 }; uint64_t lat_max[MAX_OP] = { 0 }; - uint64_t duration = + uint64_t duration_nsec = (timer_now->tv_sec - timer_start->tv_sec) * 1000000000 + (timer_now->tv_nsec - timer_start->tv_nsec); for (op = 0; op < MAX_OP; op++) { @@ -2117,7 +2114,7 @@ void print_report(mako_args_t* args, } /* overall stats */ - double total_duration = duration * 1.0 / 1000000000; + double total_duration = duration_nsec * 1.0 / 1000000000; printf("\n====== Total Duration %6.3f sec ======\n\n", total_duration); printf("Total Processes: %8d\n", args->num_processes); printf("Total Threads: %8d\n", args->num_threads); @@ -2143,7 +2140,7 @@ void print_report(mako_args_t* args, printf("Total Xacts: %8lld\n", totalxacts); printf("Total Conflicts: %8lld\n", conflicts); printf("Total Errors: %8lld\n", totalerrors); - printf("Overall TPS: %8lld\n\n", totalxacts * 1000000000 / duration); + printf("Overall TPS: %8lld\n\n", totalxacts * 1000000000 / duration_nsec); if (fp) { fprintf(fp, "\"results\": {"); @@ -2154,7 +2151,7 @@ void print_report(mako_args_t* args, fprintf(fp, "\"totalXacts\": %lld,", totalxacts); fprintf(fp, "\"totalConflicts\": %lld,", conflicts); fprintf(fp, "\"totalErrors\": %lld,", totalerrors); - fprintf(fp, "\"overallTPS\": %lld,", totalxacts * 1000000000 / duration); + fprintf(fp, "\"overallTPS\": %lld,", totalxacts * 1000000000 / duration_nsec); } /* per-op stats */ @@ -2175,11 +2172,11 @@ void print_report(mako_args_t* args, } /* TPS */ - double tps = totalxacts * 1000000000.0 / duration; + double tps = totalxacts * 1000000000.0 / duration_nsec; printf("%" STR(STATS_FIELD_WIDTH) ".2f ", tps); /* Conflicts */ - double conflicts_rate = conflicts * 1000000000.0 / duration; + double conflicts_rate = conflicts * 1000000000.0 / duration_nsec; printf("%" STR(STATS_FIELD_WIDTH) ".2f\n", conflicts_rate); if (fp) { From ebaf5b3baec88b0d51edd0f15a0b9741851ffce8 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 11:56:05 -0700 Subject: [PATCH 11/42] renamed option to --json_report --- bindings/c/test/mako/mako.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index a75c1f26e6..49c6003876 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1614,7 +1614,7 @@ void usage() { printf("%-24s %s\n", " --flatbuffers", "Use flatbuffers"); printf("%-24s %s\n", " --streaming", "Streaming mode: all (default), iterator, small, medium, large, serial"); printf("%-24s %s\n", " --disable_ryw", "Disable snapshot read-your-writes"); - printf("%-24s %s\n", " --json=PATH", "Output stats to the specified json file"); + printf("%-24s %s\n", " --json_report=PATH", "Output stats to the specified json file (Default: mako.json)"); } /* parse benchmark paramters */ @@ -1649,7 +1649,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "trace_format", required_argument, NULL, ARG_TRACEFORMAT }, { "streaming", required_argument, NULL, ARG_STREAMING_MODE }, { "txntrace", required_argument, NULL, ARG_TXNTRACE }, - { "json", required_argument, NULL, 'j' }, + { "json_report", required_argument, NULL, 'j' }, /* no args */ { "help", no_argument, NULL, 'h' }, { "zipf", no_argument, NULL, 'z' }, From 2958687ed42b677a91fafb31d67ff30383c9bd74 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 13:12:13 -0700 Subject: [PATCH 12/42] --json_report defaults to mako.json if no output file path is given --- bindings/c/test/mako/mako.c | 14 +++++++++----- bindings/c/test/mako/mako.h | 3 ++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 49c6003876..1954c17e4d 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1623,7 +1623,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { int c; int idx; while (1) { - const char* short_options = "a:c:p:t:r:s:i:x:v:m:hjz"; + const char* short_options = "a:c:p:t:r:s:i:x:v:m:hz"; static struct option long_options[] = { /* name, has_arg, flag, val */ { "api_version", required_argument, NULL, 'a' }, { "cluster", required_argument, NULL, 'c' }, @@ -1649,7 +1649,6 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "trace_format", required_argument, NULL, ARG_TRACEFORMAT }, { "streaming", required_argument, NULL, ARG_STREAMING_MODE }, { "txntrace", required_argument, NULL, ARG_TXNTRACE }, - { "json_report", required_argument, NULL, 'j' }, /* no args */ { "help", no_argument, NULL, 'h' }, { "zipf", no_argument, NULL, 'z' }, @@ -1661,6 +1660,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { { "txntagging_prefix", required_argument, NULL, ARG_TXNTAGGINGPREFIX }, { "version", no_argument, NULL, ARG_VERSION }, { "disable_ryw", no_argument, NULL, ARG_DISABLE_RYW }, + { "json_report", optional_argument, NULL, ARG_JSON_REPORT }, { NULL, 0, NULL, 0 } }; idx = 0; @@ -1713,9 +1713,6 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { args->mode = MODE_RUN; } break; - case 'j': - strcpy(args->json_output_path, optarg); - break; case ARG_KEYLEN: args->key_length = atoi(optarg); break; @@ -1821,6 +1818,13 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { case ARG_DISABLE_RYW: args->disable_ryw = 1; break; + case ARG_JSON_REPORT: + if (optarg == NULL) { + strcpy(args->json_output_path, "mako.json"); + } else { + strcpy(args->json_output_path, optarg); + } + break; } } diff --git a/bindings/c/test/mako/mako.h b/bindings/c/test/mako/mako.h index d7ff9efaad..a3bd6365b8 100644 --- a/bindings/c/test/mako/mako.h +++ b/bindings/c/test/mako/mako.h @@ -81,7 +81,8 @@ enum Arguments { ARG_TXNTAGGING, ARG_TXNTAGGINGPREFIX, ARG_STREAMING_MODE, - ARG_DISABLE_RYW + ARG_DISABLE_RYW, + ARG_JSON_REPORT }; enum TPSChangeTypes { TPS_SIN, TPS_SQUARE, TPS_PULSE }; From 85a21af67a1af6f7f42df109ea9c70021ee839cc Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 13:57:59 -0700 Subject: [PATCH 13/42] added mako args to json report file --- bindings/c/test/mako/mako.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 1954c17e4d..37a479aedb 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -2439,7 +2439,39 @@ int stats_process_main(mako_args_t* args, FILE* fp = NULL; if (args->json_output_path[0] != '\0') { fp = fopen(args->json_output_path, "w"); - fprintf(fp, "{\"samples\": ["); + fprintf(fp, "{\"makoArgs\": {"); + fprintf(fp, "\"api_version\": %d,", args->api_version); + fprintf(fp, "\"json\": %d,", args->json); + fprintf(fp, "\"num_processes\": %d,", args->num_processes); + fprintf(fp, "\"num_threads\": %d,", args->num_threads); + fprintf(fp, "\"mode\": %d,", args->mode); + fprintf(fp, "\"rows\": %d,", args->rows); + fprintf(fp, "\"seconds\": %d,", args->seconds); + fprintf(fp, "\"iteration\": %d,", args->iteration); + fprintf(fp, "\"tpsmax\": %d,", args->tpsmax); + fprintf(fp, "\"tpsmin\": %d,", args->tpsmin); + fprintf(fp, "\"tpsinterval\": %d,", args->tpsinterval); + fprintf(fp, "\"tpschange\": %d,", args->tpschange); + fprintf(fp, "\"sampling\": %d,", args->sampling); + fprintf(fp, "\"key_length\": %d,", args->key_length); + fprintf(fp, "\"value_length\": %d,", args->value_length); + fprintf(fp, "\"commit_get\": %d,", args->commit_get); + fprintf(fp, "\"verbose\": %d,", args->verbose); + fprintf(fp, "\"cluster_file\": \"%s\",", args->cluster_file); + fprintf(fp, "\"log_group\": \"%s\",", args->log_group); + fprintf(fp, "\"prefixpadding\": %d,", args->prefixpadding); + fprintf(fp, "\"trace\": %d,", args->trace); + fprintf(fp, "\"tracepath\": \"%s\",", args->tracepath); + fprintf(fp, "\"traceformat\": %d,", args->traceformat); + fprintf(fp, "\"knobs\": \"%s\",", args->knobs); + fprintf(fp, "\"flatbuffers\": %d,", args->flatbuffers); + fprintf(fp, "\"txntrace\": %d,", args->txntrace); + fprintf(fp, "\"txntagging\": %d,", args->txntagging); + fprintf(fp, "\"txntagging_prefix\": \"%s\",", args->txntagging_prefix); + fprintf(fp, "\"streaming_mode\": %d,", args->streaming_mode); + fprintf(fp, "\"disable_ryw\": %d,", args->disable_ryw); + fprintf(fp, "\"json_output_path\": \"%s\",", args->json_output_path); + fprintf(fp, "},{\"samples\": ["); } clock_gettime(CLOCK_MONOTONIC_COARSE, &timer_start); From 847c58d4bc0b75b39bea16b78dce67dd83f80c55 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 14:30:47 -0700 Subject: [PATCH 14/42] fixed bug with optional argument --- bindings/c/test/mako/mako.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 37a479aedb..05ba5030cd 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1819,7 +1819,9 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { args->disable_ryw = 1; break; case ARG_JSON_REPORT: - if (optarg == NULL) { + if (optarg == NULL || (argv[optind] != NULL && argv[optind][0] == '-')) { + // if --report_json is the last option and no file is specified + // or --report_json is followed by another option strcpy(args->json_output_path, "mako.json"); } else { strcpy(args->json_output_path, optarg); From 4d99cf250b3a03e5625a77dd3a3674309359f7a5 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 16:18:17 -0700 Subject: [PATCH 15/42] fixed bug with optional argument --- bindings/c/test/mako/mako.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 05ba5030cd..ac0b3c2505 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1819,7 +1819,7 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { args->disable_ryw = 1; break; case ARG_JSON_REPORT: - if (optarg == NULL || (argv[optind] != NULL && argv[optind][0] == '-')) { + if (optarg == NULL && (argv[optind] == NULL || (argv[optind] != NULL && argv[optind][0] == '-'))) { // if --report_json is the last option and no file is specified // or --report_json is followed by another option strcpy(args->json_output_path, "mako.json"); From 37512508137fd7d6a3114823f91e4fa1426c6b21 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 18:50:28 -0700 Subject: [PATCH 16/42] addressed comment and removed one extra { --- bindings/c/test/mako/mako.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index ac0b3c2505..d0ba181ff7 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1822,9 +1822,10 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { if (optarg == NULL && (argv[optind] == NULL || (argv[optind] != NULL && argv[optind][0] == '-'))) { // if --report_json is the last option and no file is specified // or --report_json is followed by another option - strcpy(args->json_output_path, "mako.json"); + char default_file[] = "mako.json"; + strncpy(args->json_output_path, default_file, strlen(default_file)); } else { - strcpy(args->json_output_path, optarg); + strncpy(args->json_output_path, optarg, strlen(optarg) + 1); } break; } @@ -2473,7 +2474,7 @@ int stats_process_main(mako_args_t* args, fprintf(fp, "\"streaming_mode\": %d,", args->streaming_mode); fprintf(fp, "\"disable_ryw\": %d,", args->disable_ryw); fprintf(fp, "\"json_output_path\": \"%s\",", args->json_output_path); - fprintf(fp, "},{\"samples\": ["); + fprintf(fp, "},\"samples\": ["); } clock_gettime(CLOCK_MONOTONIC_COARSE, &timer_start); From ae369d52a239b49e2bc92022032c495291816943 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Tue, 12 Oct 2021 22:35:22 -0700 Subject: [PATCH 17/42] minor edits --- bindings/c/test/mako/mako.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index d0ba181ff7..cd79549256 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -2235,9 +2235,9 @@ void print_report(mako_args_t* args, printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } else { printf("%" STR(STATS_FIELD_WIDTH) "lld ", lat_min[op]); - } - if (fp) { - fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_min[op]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_min[op]); + } } } } @@ -2252,12 +2252,12 @@ void print_report(mako_args_t* args, if (args->txnspec.ops[op][OP_COUNT] > 0 || op == OP_TRANSACTION || op == OP_COMMIT) { if (lat_total[op]) { printf("%" STR(STATS_FIELD_WIDTH) "lld ", lat_total[op] / lat_samples[op]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_total[op] / lat_samples[op]); + } } else { printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } - if (fp) { - fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_total[op] / lat_samples[op]); - } } } printf("\n"); @@ -2273,9 +2273,9 @@ void print_report(mako_args_t* args, printf("%" STR(STATS_FIELD_WIDTH) "s ", "N/A"); } else { printf("%" STR(STATS_FIELD_WIDTH) "lld ", lat_max[op]); - } - if (fp) { - fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_max[op]); + if (fp) { + fprintf(fp, "\"%s\": %lld,", get_ops_name(op), lat_max[op]); + } } } } From dbfeb06c97f140eb681bcf93890fd500f51631da Mon Sep 17 00:00:00 2001 From: He Liu Date: Fri, 1 Oct 2021 14:11:24 -0700 Subject: [PATCH 18/42] Reproduced user data loss incident, and tested the improved exclude tool can fix the system metadata. --- fdbserver/CMakeLists.txt | 1 + fdbserver/DataDistribution.actor.cpp | 21 +- fdbserver/storageserver.actor.cpp | 35 ++- .../workloads/DataLossRecovery.actor.cpp | 249 ++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/fast/DataLossRecovery.toml | 13 + 6 files changed, 306 insertions(+), 14 deletions(-) create mode 100644 fdbserver/workloads/DataLossRecovery.actor.cpp create mode 100644 tests/fast/DataLossRecovery.toml diff --git a/fdbserver/CMakeLists.txt b/fdbserver/CMakeLists.txt index 4f1a82bc77..3f30d0e246 100644 --- a/fdbserver/CMakeLists.txt +++ b/fdbserver/CMakeLists.txt @@ -170,6 +170,7 @@ set(FDBSERVER_SRCS workloads/CpuProfiler.actor.cpp workloads/Cycle.actor.cpp workloads/DataDistributionMetrics.actor.cpp + workloads/DataLossRecovery.actor.cpp workloads/DDBalance.actor.cpp workloads/DDMetrics.actor.cpp workloads/DDMetricsExclude.actor.cpp diff --git a/fdbserver/DataDistribution.actor.cpp b/fdbserver/DataDistribution.actor.cpp index c686cf5c42..f3462ad142 100644 --- a/fdbserver/DataDistribution.actor.cpp +++ b/fdbserver/DataDistribution.actor.cpp @@ -922,14 +922,19 @@ struct DDTeamCollection : ReferenceCounted { // Prefer a healthy team not containing excludeServer. if (candidates.size() > 0) { - return teams[deterministicRandom()->randomInt(0, candidates.size())]->getServerIDs(); - } - - // The backup choice is a team with at least one server besides excludeServer, in this - // case, the team will be possibily relocated to a healthy destination later by DD. - if (backup.size() > 0) { - std::vector res = teams[deterministicRandom()->randomInt(0, backup.size())]->getServerIDs(); - std::remove(res.begin(), res.end(), excludeServer); + return teams[candidates[deterministicRandom()->randomInt(0, candidates.size())]]->getServerIDs(); + } else if (backup.size() > 0) { + // The backup choice is a team with at least one server besides excludeServer, in this + // case, the team will be possibily relocated to a healthy destination later by DD. + std::vector servers = + teams[backup[deterministicRandom()->randomInt(0, backup.size())]]->getServerIDs(); + std::vector res; + for (const UID& id : servers) { + if (id != excludeServer) { + res.push_back(id); + } + } + TraceEvent("FoundNonoptimalTeamForDroppedShard", excludeServer).detail("Team", describe(res)); return res; } diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index be43b3bc63..2047cb05b6 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -3244,7 +3244,7 @@ void ShardInfo::addMutation(Version version, MutationRef const& mutation) { } } -enum ChangeServerKeysContext { CSK_UPDATE, CSK_RESTORE }; +enum ChangeServerKeysContext { CSK_UPDATE, CSK_RESTORE, CSK_ASSIGN_EMPTY }; const char* changeServerKeysContextName[] = { "Update", "Restore" }; void changeServerKeys(StorageServer* data, @@ -3312,6 +3312,7 @@ void changeServerKeys(StorageServer* data, auto vr = data->newestAvailableVersion.intersectingRanges(keys); std::vector> changeNewestAvailable; std::vector removeRanges; + std::vector clearRanges; for (auto r = vr.begin(); r != vr.end(); ++r) { KeyRangeRef range = keys & r->range(); bool dataAvailable = r->value() == latestVersion || r->value() >= version; @@ -3322,7 +3323,22 @@ void changeServerKeys(StorageServer* data, // .detail("NowAssigned", nowAssigned) // .detail("NewestAvailable", r->value()) // .detail("ShardState0", data->shards[range.begin]->debugDescribeState()); - if (!nowAssigned) { + if (context == CSK_ASSIGN_EMPTY && !dataAvailable) { + ASSERT(nowAssigned); + TraceEvent("ChangeServerKeysAddEmptyRange", data->thisServerID) + .detail("Begin", range.begin) + .detail("End", range.end); + // MutationRef clearRange(MutationRef::ClearRange, range.begin, range.end); + // Version clv = data->data().getLatestVersion(); + // clearRange = data->addMutationToMutationLog(data->addVersionToMutationLog(clv), clearRange); + + // Wait (if necessary) for the latest version at which any key in keys was previously available (+1) to be + // durable + + clearRanges.push_back(range); + // changeNewestAvailable.emplace_back(range, invalidVersion); + data->addShard(ShardInfo::newReadWrite(range, data)); + } else if (!nowAssigned) { if (dataAvailable) { ASSERT(r->value() == latestVersion); // Not that we care, but this used to be checked instead of dataAvailable @@ -3335,7 +3351,7 @@ void changeServerKeys(StorageServer* data, } else if (!dataAvailable) { // SOMEDAY: Avoid restarting adding/transferred shards if (version == 0) { // bypass fetchkeys; shard is known empty at version 0 - TraceEvent("ChangeServerKeysAddEmptyRange", data->thisServerID) + TraceEvent("ChangeServerKeysInitialRange", data->thisServerID) .detail("Begin", range.begin) .detail("End", range.end); changeNewestAvailable.emplace_back(range, latestVersion); @@ -3369,6 +3385,13 @@ void changeServerKeys(StorageServer* data, removeDataRange(data, data->addVersionToMutationLog(data->data().getLatestVersion()), data->shards, *r); setAvailableStatus(data, *r, false); } + + for (auto r = clearRanges.begin(); r != clearRanges.end(); ++r) { + MutationRef clearRange(MutationRef::ClearRange, r->begin, r->end); + data->addMutation(data->data().getLatestVersion(), clearRange, *r, data->updateEagerReads); + data->newestAvailableVersion.insert(*r, latestVersion); + setAvailableStatus(data, *r, true); + } validate(data); } @@ -3513,8 +3536,8 @@ private: // the data for change.version-1 (changes from versions < change.version) // If emptyRange, treat the shard as empty, see removeKeysFromFailedServer() for more details about this // scenario. - const Version shardVersion = (emptyRange && nowAssigned) ? 0 : currentVersion - 1; - changeServerKeys(data, keys, nowAssigned, shardVersion, CSK_UPDATE); + const ChangeServerKeysContext context = emptyRange ? CSK_ASSIGN_EMPTY : CSK_UPDATE; + changeServerKeys(data, keys, nowAssigned, currentVersion - 1, context); } processedStartKey = false; @@ -5003,7 +5026,7 @@ ACTOR Future reportStorageServerState(StorageServer* self) { level = SevWarnAlways; } - TraceEvent(level, "FetchKeyCurrentStatus") + TraceEvent(level, "FetchKeysCurrentStatus", self->thisServerID) .detail("Timestamp", now()) .detail("LongestRunningTime", longestRunningFetchKeys.first) .detail("StartKey", longestRunningFetchKeys.second.begin) diff --git a/fdbserver/workloads/DataLossRecovery.actor.cpp b/fdbserver/workloads/DataLossRecovery.actor.cpp new file mode 100644 index 0000000000..46447b6bd2 --- /dev/null +++ b/fdbserver/workloads/DataLossRecovery.actor.cpp @@ -0,0 +1,249 @@ +/* + * DataLossRecovery.actor.cpp + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include "fdbclient/NativeAPI.actor.h" +#include "fdbclient/ManagementAPI.actor.h" +#include "fdbserver/MoveKeys.actor.h" +#include "fdbserver/QuietDatabase.h" +#include "fdbrpc/simulator.h" +#include "fdbserver/workloads/workloads.actor.h" +#include "flow/Error.h" +#include "flow/IRandom.h" +#include "flow/flow.h" +#include "flow/actorcompiler.h" // This must be the last #include. + +namespace { +std::string printValue(const Optional& value) { + return value.present() ? value.get().toString() : "Value Not Found."; +} +} // namespace + +struct DataLossRecoveryWorkload : TestWorkload { + FlowLock startMoveKeysParallelismLock; + FlowLock finishMoveKeysParallelismLock; + const bool enabled; + bool pass; + NetworkAddress addr; + + DataLossRecoveryWorkload(WorkloadContext const& wcx) + : TestWorkload(wcx), startMoveKeysParallelismLock(1), finishMoveKeysParallelismLock(1), enabled(!clientId), + pass(true) {} + + void validationFailed(Optional& expectedValue, Optional& actualValue) { + TraceEvent(SevError, "TestFailed") + .detail("ExpectedValue", printValue(expectedValue)) + .detail("ActualValue", printValue(actualValue)); + pass = false; + } + + std::string description() const override { return "DataLossRecovery"; } + + Future setup(Database const& cx) override { return Void(); } + + Future start(Database const& cx) override { + if (!enabled) { + return Void(); + } + return _start(this, cx); + } + + ACTOR Future _start(DataLossRecoveryWorkload* self, Database cx) { + state Key key = "TestKey"_sr; + state Key endKey = "TestKey0"_sr; + state Value oldValue = "TestValue"_sr; + state Value newValue = "TestNewValue"_sr; + + wait(self->writeAndVerify(self, cx, key, oldValue)); + + // Move [key, endKey) to team: {address}. + state NetworkAddress address = wait(self->disableDDAndMoveShard(self, cx, KeyRangeRef(key, endKey))); + wait(self->readAndVerify(self, cx, key, oldValue)); + + // Kill team {address}, and expect read to timeout. + self->killProcess(self, address); + wait(self->readAndVerify(self, cx, key, "Timeout"_sr)); + + // Reenable DD and exclude address as fail, so that [key, endKey) will be dropped and moved to a new team. + // Expect read to return 'value not found'. + int ignore = wait(setDDMode(cx, 1)); + wait(self->exclude(cx, address)); + wait(self->readAndVerify(self, cx, key, Optional())); + + // Write will scceed. + wait(self->writeAndVerify(self, cx, key, newValue)); + + return Void(); + } + + ACTOR Future readAndVerify(DataLossRecoveryWorkload* self, + Database cx, + Key key, + Optional expectedValue) { + state Transaction tr(cx); + + loop { + try { + state Optional res = wait(timeout(tr.get(key), 30.0, Optional("Timeout"_sr))); + if (res != expectedValue) { + self->validationFailed(expectedValue, res); + } + break; + } catch (Error& e) { + wait(tr.onError(e)); + } + } + + return Void(); + } + + ACTOR Future writeAndVerify(DataLossRecoveryWorkload* self, Database cx, Key key, Optional value) { + state Transaction tr(cx); + loop { + try { + if (value.present()) { + tr.set(key, value.get()); + } else { + tr.clear(key); + } + wait(timeout(tr.commit(), 10.0, Void())); + break; + } catch (Error& e) { + wait(tr.onError(e)); + } + } + + wait(self->readAndVerify(self, cx, key, value)); + + return Void(); + } + + ACTOR Future exclude(Database cx, NetworkAddress addr) { + state Transaction tr(cx); + state std::vector servers; + servers.push_back(AddressExclusion(addr.ip, addr.port)); + loop { + try { + excludeServers(tr, servers, true); + wait(tr.commit()); + break; + } catch (Error& e) { + wait(tr.onError(e)); + } + } + + // Wait until all data are moved out of servers. + std::set inProgress = wait(checkForExcludingServers(cx, servers, true)); + ASSERT(inProgress.empty()); + + TraceEvent("ExcludedFailedServer").detail("Address", addr.toString()); + return Void(); + } + + // Move keys to a random selected team consisting of a single SS, after disabling DD, so that keys won't be + // kept in the new team until DD is enabled. + // Returns the address of the single SS of the new team. + ACTOR Future disableDDAndMoveShard(DataLossRecoveryWorkload* self, Database cx, KeyRange keys) { + // Disable DD to avoid DD undoing of our move. + state int ignore = wait(setDDMode(cx, 0)); + state NetworkAddress addr; + + // Pick a random SS as the dest, keys will reside on a single server after the move. + state std::vector dest; + while (dest.empty()) { + std::vector interfs = wait(getStorageServers(cx)); + if (!interfs.empty()) { + const auto& interf = interfs[deterministicRandom()->randomInt(0, interfs.size())]; + if (g_simulator.protectedAddresses.count(interf.address()) == 0) { + dest.push_back(interf.uniqueID); + addr = interf.address(); + } + } + } + + state UID owner = deterministicRandom()->randomUniqueID(); + state DDEnabledState ddEnabledState; + + state Transaction tr(cx); + + loop { + try { + BinaryWriter wrMyOwner(Unversioned()); + wrMyOwner << owner; + tr.set(moveKeysLockOwnerKey, wrMyOwner.toValue()); + wait(tr.commit()); + + MoveKeysLock moveKeysLock; + moveKeysLock.myOwner = owner; + + wait(moveKeys(cx, + keys, + dest, + dest, + moveKeysLock, + Promise(), + &self->startMoveKeysParallelismLock, + &self->finishMoveKeysParallelismLock, + false, + UID(), // for logging only + &ddEnabledState)); + break; + } catch (Error& e) { + if (e.code() == error_code_movekeys_conflict) { + // Conflict on moveKeysLocks with the current running DD is expected, just retry. + tr.reset(); + } else { + wait(tr.onError(e)); + } + } + } + + TraceEvent("TestKeyMoved").detail("NewTeam", describe(dest)).detail("Address", addr.toString()); + + state Transaction validateTr(cx); + loop { + try { + Standalone> addresses = wait(validateTr.getAddressesForKey(keys.begin)); + // The move function is not what we are testing here, crash the test if the move fails. + ASSERT(addresses.size() == 1); + ASSERT(std::string(addresses[0]) == addr.toString()); + break; + } catch (Error& e) { + wait(validateTr.onError(e)); + } + } + + return addr; + } + + void killProcess(DataLossRecoveryWorkload* self, const NetworkAddress& addr) { + ISimulator::ProcessInfo* process = g_simulator.getProcessByAddress(addr); + ASSERT(process->addresses.contains(addr)); + g_simulator.killProcess(process, ISimulator::KillInstantly); + TraceEvent("TestTeamKilled").detail("Address", addr); + } + + Future check(Database const& cx) override { return pass; } + + void getMetrics(std::vector& m) override {} +}; + +WorkloadFactory DataLossRecoveryWorkloadFactory("DataLossRecovery"); \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 92464ddf54..dfa22ab5e3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -131,6 +131,7 @@ if(WITH_PYTHON) add_fdb_test(TEST_FILES fast/ConstrainedRandomSelector.toml) add_fdb_test(TEST_FILES fast/CycleAndLock.toml) add_fdb_test(TEST_FILES fast/CycleTest.toml) + add_fdb_test(TEST_FILES fast/DataLossRecovery.toml) add_fdb_test(TEST_FILES fast/FuzzApiCorrectness.toml) add_fdb_test(TEST_FILES fast/FuzzApiCorrectnessClean.toml) add_fdb_test(TEST_FILES fast/IncrementalBackup.toml) diff --git a/tests/fast/DataLossRecovery.toml b/tests/fast/DataLossRecovery.toml new file mode 100644 index 0000000000..6cebd91b97 --- /dev/null +++ b/tests/fast/DataLossRecovery.toml @@ -0,0 +1,13 @@ +[configuration] +config = 'triple' +storageEngineType = 0 +processesPerMachine = 2 +coordinators = 3 +machineCount = 45 + +[[test]] +testTitle = 'DataLossRecovery' +useDB = true + + [[test.workload]] + testName = 'DataLossRecovery' From 66166e09be4ebb4bfc056dfcb001d996cebb24c0 Mon Sep 17 00:00:00 2001 From: He Liu Date: Fri, 15 Oct 2021 09:28:05 -0700 Subject: [PATCH 19/42] Clear range before setting the moved-in empty range as available. --- fdbserver/storageserver.actor.cpp | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 2047cb05b6..c2815d135f 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -3312,7 +3312,7 @@ void changeServerKeys(StorageServer* data, auto vr = data->newestAvailableVersion.intersectingRanges(keys); std::vector> changeNewestAvailable; std::vector removeRanges; - std::vector clearRanges; + std::vector newEmptyRanges; for (auto r = vr.begin(); r != vr.end(); ++r) { KeyRangeRef range = keys & r->range(); bool dataAvailable = r->value() == latestVersion || r->value() >= version; @@ -3328,15 +3328,7 @@ void changeServerKeys(StorageServer* data, TraceEvent("ChangeServerKeysAddEmptyRange", data->thisServerID) .detail("Begin", range.begin) .detail("End", range.end); - // MutationRef clearRange(MutationRef::ClearRange, range.begin, range.end); - // Version clv = data->data().getLatestVersion(); - // clearRange = data->addMutationToMutationLog(data->addVersionToMutationLog(clv), clearRange); - - // Wait (if necessary) for the latest version at which any key in keys was previously available (+1) to be - // durable - - clearRanges.push_back(range); - // changeNewestAvailable.emplace_back(range, invalidVersion); + newEmptyRanges.push_back(range); data->addShard(ShardInfo::newReadWrite(range, data)); } else if (!nowAssigned) { if (dataAvailable) { @@ -3386,11 +3378,12 @@ void changeServerKeys(StorageServer* data, setAvailableStatus(data, *r, false); } - for (auto r = clearRanges.begin(); r != clearRanges.end(); ++r) { - MutationRef clearRange(MutationRef::ClearRange, r->begin, r->end); - data->addMutation(data->data().getLatestVersion(), clearRange, *r, data->updateEagerReads); - data->newestAvailableVersion.insert(*r, latestVersion); - setAvailableStatus(data, *r, true); + // Clear the moving-in empty range, and set it available at the latestVersion. + for (const auto& range : newEmptyRanges) { + MutationRef clearRange(MutationRef::ClearRange, range.begin, range.end); + data->addMutation(data->data().getLatestVersion(), clearRange, range, data->updateEagerReads); + data->newestAvailableVersion.insert(range, latestVersion); + setAvailableStatus(data, range, true); } validate(data); } From a0f62e873e8476b850ddef2b6530bdca373e1f63 Mon Sep 17 00:00:00 2001 From: He Liu Date: Fri, 15 Oct 2021 14:58:26 -0700 Subject: [PATCH 20/42] Use ErrorOr to indicate an error. --- fdbserver/workloads/DataLossRecovery.actor.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/fdbserver/workloads/DataLossRecovery.actor.cpp b/fdbserver/workloads/DataLossRecovery.actor.cpp index 8e12d48cf2..8169b6ecf0 100644 --- a/fdbserver/workloads/DataLossRecovery.actor.cpp +++ b/fdbserver/workloads/DataLossRecovery.actor.cpp @@ -51,7 +51,7 @@ struct DataLossRecoveryWorkload : TestWorkload { : TestWorkload(wcx), startMoveKeysParallelismLock(1), finishMoveKeysParallelismLock(1), enabled(!clientId), pass(true) {} - void validationFailed(ErrorOr>& expectedValue, ErrorOr>& actualValue) { + void validationFailed(ErrorOr> expectedValue, ErrorOr> actualValue) { TraceEvent(SevError, "TestFailed") .detail("ExpectedValue", printValue(expectedValue)) .detail("ActualValue", printValue(actualValue)); @@ -105,15 +105,16 @@ struct DataLossRecoveryWorkload : TestWorkload { loop { try { - state ErrorOr> res = wait(errorOr(timeoutError(tr.get(key), 30.0))); - const bool equal = (res.isError() && expectedValue.isError() && - res.getError().code() == expectedValue.getError().code()) || - (!res.isError() && !expectedValue.isError() && res.get() == expectedValue.get()); + state Optional res = wait(timeoutError(tr.get(key), 30.0)); + const bool equal = !expectedValue.isError() && res == expectedValue.get(); if (!equal) { - self->validationFailed(expectedValue, res); + self->validationFailed(expectedValue, ErrorOr>(res)); } break; } catch (Error& e) { + if (expectedValue.isError() && expectedValue.getError().code() == e.code()) { + break; + } wait(tr.onError(e)); } } @@ -130,7 +131,7 @@ struct DataLossRecoveryWorkload : TestWorkload { } else { tr.clear(key); } - wait(timeout(tr.commit(), 10.0, Void())); + wait(timeoutError(tr.commit(), 30.0)); break; } catch (Error& e) { wait(tr.onError(e)); From f20f43a85a6e0483199b66598ffe260b209f02f6 Mon Sep 17 00:00:00 2001 From: He Liu Date: Mon, 18 Oct 2021 10:38:49 -0700 Subject: [PATCH 21/42] Added warning in the `exlucde` cli command help about potential dataloss, as well as in command-line-interface.rst. --- documentation/sphinx/source/command-line-interface.rst | 4 ++++ fdbcli/ExcludeCommand.actor.cpp | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/documentation/sphinx/source/command-line-interface.rst b/documentation/sphinx/source/command-line-interface.rst index cacfccf56a..6d2243f02f 100644 --- a/documentation/sphinx/source/command-line-interface.rst +++ b/documentation/sphinx/source/command-line-interface.rst @@ -164,6 +164,10 @@ If the ``failed`` keyword is specified, the address is marked as failed and adde For more information on excluding servers, see :ref:`removing-machines-from-a-cluster`. +Warning about potential dataloss ``failed`` option: if a server is the last one in some team(s), excluding it with ``failed`` will lose all data in the team(s), and hence ``failed`` should only be set when the server(s) have permanently failed. + +In the case all servers of a team have failed permanently, excluding all the servers will clean up the corresponding keyrange, and fix the invalid metadata. The keyrange will be assigned to a new team as an empty shard. + exit ---- diff --git a/fdbcli/ExcludeCommand.actor.cpp b/fdbcli/ExcludeCommand.actor.cpp index 8580e6b063..9296f9ef88 100644 --- a/fdbcli/ExcludeCommand.actor.cpp +++ b/fdbcli/ExcludeCommand.actor.cpp @@ -393,5 +393,11 @@ CommandFactory excludeFactory( "command returns \nimmediately without checking if the exclusions have completed successfully.\n" "If 'FORCE' is set, the command does not perform safety checks before excluding.\n" "If 'failed' is set, the transaction log queue is dropped pre-emptively before waiting\n" - "for data movement to finish and the server cannot be included again.")); + "for data movement to finish and the server cannot be included again." + "\n\nWARNING of potential dataloss\n:" + "If a to-be-excluded server is the last server of some team(s), and 'failed' is set, the data in the team(s) " + "will be lost. 'failed' should be set only if the server(s) have permanently failed." + "In the case all servers of a team have failed permanently and dataloss has been a fact, excluding all the " + "servers will clean up the corresponding keyrange, and fix the invalid metadata. The keyrange will be " + "assigned to a new team as an empty shard.")); } // namespace fdb_cli From 8dd7f8f44759aa17aa164631a9fb43380f9d42c4 Mon Sep 17 00:00:00 2001 From: Josh Slocum Date: Wed, 20 Oct 2021 10:45:13 -0500 Subject: [PATCH 22/42] Fixes to ss e-brake, tlog streaming, and their interaction --- fdbclient/ServerKnobs.cpp | 6 +++-- fdbclient/ServerKnobs.h | 2 ++ fdbserver/storageserver.actor.cpp | 45 ++++++++++++++++++++++--------- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/fdbclient/ServerKnobs.cpp b/fdbclient/ServerKnobs.cpp index d540492c3c..a535747008 100644 --- a/fdbclient/ServerKnobs.cpp +++ b/fdbclient/ServerKnobs.cpp @@ -55,7 +55,7 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi init( BUGGIFY_RECOVER_MEMORY_LIMIT, 1e6 ); init( BUGGIFY_WORKER_REMOVED_MAX_LAG, 30 ); init( UPDATE_STORAGE_BYTE_LIMIT, 1e6 ); - init( TLOG_PEEK_DELAY, 0.00005 ); + init( TLOG_PEEK_DELAY, 0.0005 ); init( LEGACY_TLOG_UPGRADE_ENTRIES_PER_VERSION, 100 ); init( VERSION_MESSAGES_OVERHEAD_FACTOR_1024THS, 1072 ); // Based on a naive interpretation of the gcc version of std::deque, we would expect this to be 16 bytes overhead per 512 bytes data. In practice, it seems to be 24 bytes overhead per 512. init( VERSION_MESSAGES_ENTRY_BYTES_WITH_OVERHEAD, std::ceil(16.0 * VERSION_MESSAGES_OVERHEAD_FACTOR_1024THS / 1024) ); @@ -64,7 +64,7 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi init( TLOG_MESSAGE_BLOCK_BYTES, 10e6 ); init( TLOG_MESSAGE_BLOCK_OVERHEAD_FACTOR, double(TLOG_MESSAGE_BLOCK_BYTES) / (TLOG_MESSAGE_BLOCK_BYTES - MAX_MESSAGE_SIZE) ); //1.0121466709838096006362758832473 init( PEEK_TRACKER_EXPIRATION_TIME, 600 ); if( randomize && BUGGIFY ) PEEK_TRACKER_EXPIRATION_TIME = deterministicRandom()->coinflip() ? 0.1 : 120; - init( PEEK_USING_STREAMING, true ); + init( PEEK_USING_STREAMING, true ); if( randomize && BUGGIFY ) PEEK_USING_STREAMING = false; init( PARALLEL_GET_MORE_REQUESTS, 32 ); if( randomize && BUGGIFY ) PARALLEL_GET_MORE_REQUESTS = 2; init( MULTI_CURSOR_PRE_FETCH_LIMIT, 10 ); init( MAX_QUEUE_COMMIT_BYTES, 15e6 ); if( randomize && BUGGIFY ) MAX_QUEUE_COMMIT_BYTES = 5000; @@ -534,6 +534,8 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi init( TARGET_BYTES_PER_STORAGE_SERVER_BATCH, 750e6 ); if( smallStorageTarget ) TARGET_BYTES_PER_STORAGE_SERVER_BATCH = 1500e3; init( SPRING_BYTES_STORAGE_SERVER_BATCH, 100e6 ); if( smallStorageTarget ) SPRING_BYTES_STORAGE_SERVER_BATCH = 150e3; init( STORAGE_HARD_LIMIT_BYTES, 1500e6 ); if( smallStorageTarget ) STORAGE_HARD_LIMIT_BYTES = 4500e3; + init( STORAGE_HARD_LIMIT_BYTES_OVERAGE, 5000e3 ); if( smallStorageTarget ) STORAGE_HARD_LIMIT_BYTES_OVERAGE = 100e3; // byte+version overage ensures storage server makes enough progress on freeing up storage queue memory at hard limit by ensuring it advances desiredOldestVersion enough per commit cycle. + init( STORAGE_HARD_LIMIT_VERSION_OVERAGE, VERSIONS_PER_SECOND / 4.0 ); init( STORAGE_DURABILITY_LAG_HARD_MAX, 2000e6 ); if( smallStorageTarget ) STORAGE_DURABILITY_LAG_HARD_MAX = 100e6; init( STORAGE_DURABILITY_LAG_SOFT_MAX, 250e6 ); if( smallStorageTarget ) STORAGE_DURABILITY_LAG_SOFT_MAX = 10e6; diff --git a/fdbclient/ServerKnobs.h b/fdbclient/ServerKnobs.h index 2adf76f94e..d667cf40b5 100644 --- a/fdbclient/ServerKnobs.h +++ b/fdbclient/ServerKnobs.h @@ -471,6 +471,8 @@ public: int64_t TARGET_BYTES_PER_STORAGE_SERVER_BATCH; int64_t SPRING_BYTES_STORAGE_SERVER_BATCH; int64_t STORAGE_HARD_LIMIT_BYTES; + int64_t STORAGE_HARD_LIMIT_BYTES_OVERAGE; + int64_t STORAGE_HARD_LIMIT_VERSION_OVERAGE; int64_t STORAGE_DURABILITY_LAG_HARD_MAX; int64_t STORAGE_DURABILITY_LAG_SOFT_MAX; diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 37eb72fcd1..ec8d7b1181 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -670,6 +670,9 @@ public: bool debug_inApplyUpdate; double debug_lastValidateTime; + int64_t lastBytesInputEBrake; + Version lastDurableVersionEBrake; + int maxQueryQueue; int getAndResetMaxQueryQueueSize() { int val = maxQueryQueue; @@ -869,7 +872,7 @@ public: fetchKeysBytesBudget(SERVER_KNOBS->STORAGE_FETCH_BYTES), fetchKeysBudgetUsed(false), instanceID(deterministicRandom()->randomUniqueID().first()), shuttingDown(false), behind(false), versionBehind(false), debug_inApplyUpdate(false), debug_lastValidateTime(0), maxQueryQueue(0), - transactionTagCounter(ssi.id()), counters(this), + lastBytesInputEBrake(0), lastDurableVersionEBrake(0), transactionTagCounter(ssi.id()), counters(this), storageServerSourceTLogIDEventHolder( makeReference(ssi.id().toString() + "/StorageServerSourceTLogID")) { version.initMetric(LiteralStringRef("StorageServer.Version"), counters.cc.id); @@ -3675,18 +3678,36 @@ ACTOR Future update(StorageServer* data, bool* pReceivedUpdate) { try { // If we are disk bound and durableVersion is very old, we need to block updates or we could run out of memory // This is often referred to as the storage server e-brake (emergency brake) - state double waitStartT = 0; - while (data->queueSize() >= SERVER_KNOBS->STORAGE_HARD_LIMIT_BYTES && - data->durableVersion.get() < data->desiredOldestVersion.get()) { - if (now() - waitStartT >= 1) { - TraceEvent(SevWarn, "StorageServerUpdateLag", data->thisServerID) - .detail("Version", data->version.get()) - .detail("DurableVersion", data->durableVersion.get()); - waitStartT = now(); - } - data->behind = true; - wait(delayJittered(.005, TaskPriority::TLogPeekReply)); + // We allow the storage server to make some progress between e-brake periods, referreed to as "overage", in + // order to ensure that it advances desiredOldestVersion enough for updateStorage to make enough progress on + // freeing up queue size. + state double waitStartT = 0; + if (data->queueSize() >= SERVER_KNOBS->STORAGE_HARD_LIMIT_BYTES && + data->durableVersion.get() < data->desiredOldestVersion.get() && + ((data->desiredOldestVersion.get() - SERVER_KNOBS->STORAGE_HARD_LIMIT_VERSION_OVERAGE > + data->lastDurableVersionEBrake) || + (data->counters.bytesInput.getValue() - SERVER_KNOBS->STORAGE_HARD_LIMIT_BYTES_OVERAGE > + data->lastBytesInputEBrake))) { + + while (data->queueSize() >= SERVER_KNOBS->STORAGE_HARD_LIMIT_BYTES && + data->durableVersion.get() < data->desiredOldestVersion.get()) { + if (now() - waitStartT >= 1) { + TraceEvent(SevWarn, "StorageServerUpdateLag", data->thisServerID) + .detail("Version", data->version.get()) + .detail("DurableVersion", data->durableVersion.get()) + .detail("DesiredOldestVersion", data->desiredOldestVersion.get()) + .detail("QueueSize", data->queueSize()) + .detail("LastBytesInputEBrake", data->lastBytesInputEBrake) + .detail("LastDurableVersionEBrake", data->lastDurableVersionEBrake); + waitStartT = now(); + } + + data->behind = true; + wait(delayJittered(.005, TaskPriority::TLogPeekReply)); + } + data->lastBytesInputEBrake = data->counters.bytesInput.getValue(); + data->lastDurableVersionEBrake = data->durableVersion.get(); } if (g_network->isSimulated() && data->isTss() && g_simulator.tssMode == ISimulator::TSSMode::EnabledAddDelay && From 19495e4c0dac4963205f00ee5ab07d10f961124d Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Wed, 20 Oct 2021 11:25:33 -0700 Subject: [PATCH 23/42] formatting --- bindings/c/test/mako/mako.c | 79 +++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index 6f66ed9935..f2f1fc1dbf 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -1652,45 +1652,46 @@ int parse_args(int argc, char* argv[], mako_args_t* args) { int idx; while (1) { const char* short_options = "a:c:p:t:r:s:i:x:v:m:hz"; - static struct option long_options[] = { /* name, has_arg, flag, val */ - { "api_version", required_argument, NULL, 'a' }, - { "cluster", required_argument, NULL, 'c' }, - { "procs", required_argument, NULL, 'p' }, - { "threads", required_argument, NULL, 't' }, - { "rows", required_argument, NULL, 'r' }, - { "seconds", required_argument, NULL, 's' }, - { "iteration", required_argument, NULL, 'i' }, - { "keylen", required_argument, NULL, ARG_KEYLEN }, - { "vallen", required_argument, NULL, ARG_VALLEN }, - { "transaction", required_argument, NULL, 'x' }, - { "tps", required_argument, NULL, ARG_TPS }, - { "tpsmax", required_argument, NULL, ARG_TPSMAX }, - { "tpsmin", required_argument, NULL, ARG_TPSMIN }, - { "tpsinterval", required_argument, NULL, ARG_TPSINTERVAL }, - { "tpschange", required_argument, NULL, ARG_TPSCHANGE }, - { "sampling", required_argument, NULL, ARG_SAMPLING }, - { "verbose", required_argument, NULL, 'v' }, - { "mode", required_argument, NULL, 'm' }, - { "knobs", required_argument, NULL, ARG_KNOBS }, - { "loggroup", required_argument, NULL, ARG_LOGGROUP }, - { "tracepath", required_argument, NULL, ARG_TRACEPATH }, - { "trace_format", required_argument, NULL, ARG_TRACEFORMAT }, - { "streaming", required_argument, NULL, ARG_STREAMING_MODE }, - { "txntrace", required_argument, NULL, ARG_TXNTRACE }, - /* no args */ - { "help", no_argument, NULL, 'h' }, - { "zipf", no_argument, NULL, 'z' }, - { "commitget", no_argument, NULL, ARG_COMMITGET }, - { "flatbuffers", no_argument, NULL, ARG_FLATBUFFERS }, - { "prefix_padding", no_argument, NULL, ARG_PREFIXPADDING }, - { "trace", no_argument, NULL, ARG_TRACE }, - { "txntagging", required_argument, NULL, ARG_TXNTAGGING }, - { "txntagging_prefix", required_argument, NULL, ARG_TXNTAGGINGPREFIX }, - { "version", no_argument, NULL, ARG_VERSION }, - { "client_threads_per_version", required_argument, NULL, ARG_CLIENT_THREADS_PER_VERSION }, - { "disable_ryw", no_argument, NULL, ARG_DISABLE_RYW }, - { "json_report", optional_argument, NULL, ARG_JSON_REPORT }, - { NULL, 0, NULL, 0 } + static struct option long_options[] = { + /* name, has_arg, flag, val */ + { "api_version", required_argument, NULL, 'a' }, + { "cluster", required_argument, NULL, 'c' }, + { "procs", required_argument, NULL, 'p' }, + { "threads", required_argument, NULL, 't' }, + { "rows", required_argument, NULL, 'r' }, + { "seconds", required_argument, NULL, 's' }, + { "iteration", required_argument, NULL, 'i' }, + { "keylen", required_argument, NULL, ARG_KEYLEN }, + { "vallen", required_argument, NULL, ARG_VALLEN }, + { "transaction", required_argument, NULL, 'x' }, + { "tps", required_argument, NULL, ARG_TPS }, + { "tpsmax", required_argument, NULL, ARG_TPSMAX }, + { "tpsmin", required_argument, NULL, ARG_TPSMIN }, + { "tpsinterval", required_argument, NULL, ARG_TPSINTERVAL }, + { "tpschange", required_argument, NULL, ARG_TPSCHANGE }, + { "sampling", required_argument, NULL, ARG_SAMPLING }, + { "verbose", required_argument, NULL, 'v' }, + { "mode", required_argument, NULL, 'm' }, + { "knobs", required_argument, NULL, ARG_KNOBS }, + { "loggroup", required_argument, NULL, ARG_LOGGROUP }, + { "tracepath", required_argument, NULL, ARG_TRACEPATH }, + { "trace_format", required_argument, NULL, ARG_TRACEFORMAT }, + { "streaming", required_argument, NULL, ARG_STREAMING_MODE }, + { "txntrace", required_argument, NULL, ARG_TXNTRACE }, + /* no args */ + { "help", no_argument, NULL, 'h' }, + { "zipf", no_argument, NULL, 'z' }, + { "commitget", no_argument, NULL, ARG_COMMITGET }, + { "flatbuffers", no_argument, NULL, ARG_FLATBUFFERS }, + { "prefix_padding", no_argument, NULL, ARG_PREFIXPADDING }, + { "trace", no_argument, NULL, ARG_TRACE }, + { "txntagging", required_argument, NULL, ARG_TXNTAGGING }, + { "txntagging_prefix", required_argument, NULL, ARG_TXNTAGGINGPREFIX }, + { "version", no_argument, NULL, ARG_VERSION }, + { "client_threads_per_version", required_argument, NULL, ARG_CLIENT_THREADS_PER_VERSION }, + { "disable_ryw", no_argument, NULL, ARG_DISABLE_RYW }, + { "json_report", optional_argument, NULL, ARG_JSON_REPORT }, + { NULL, 0, NULL, 0 } }; idx = 0; c = getopt_long(argc, argv, short_options, long_options, &idx); From 7ffd7d9aaaad2bee97ed9c5764128a0749ad1e00 Mon Sep 17 00:00:00 2001 From: QA Hoang Date: Wed, 20 Oct 2021 12:10:14 -0700 Subject: [PATCH 24/42] fixed typo --- bindings/c/test/mako/mako.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/c/test/mako/mako.c b/bindings/c/test/mako/mako.c index f2f1fc1dbf..dc429aa78c 100644 --- a/bindings/c/test/mako/mako.c +++ b/bindings/c/test/mako/mako.c @@ -2525,7 +2525,7 @@ int stats_process_main(mako_args_t* args, fprintf(fp, "\"value_length\": %d,", args->value_length); fprintf(fp, "\"commit_get\": %d,", args->commit_get); fprintf(fp, "\"verbose\": %d,", args->verbose); - fprintf(fp, "\"cluster_file\": \"%s\",", args->cluster_file); + fprintf(fp, "\"cluster_file\": \"%s\",", args->cluster_files); fprintf(fp, "\"log_group\": \"%s\",", args->log_group); fprintf(fp, "\"prefixpadding\": %d,", args->prefixpadding); fprintf(fp, "\"trace\": %d,", args->trace); From c69364d5aa9ca37749cf3d6001b3213fe04c8197 Mon Sep 17 00:00:00 2001 From: Trevor Clinkenbeard Date: Thu, 21 Oct 2021 09:01:52 -0700 Subject: [PATCH 25/42] Verify that cluster is fully recovered in quietDatabase check (#5807) * Verify that cluster is fully recovered in quietDatabase check * Add trace event to waitForQuietDatabase --- fdbserver/QuietDatabase.actor.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fdbserver/QuietDatabase.actor.cpp b/fdbserver/QuietDatabase.actor.cpp index 97b9211cd1..9bdaed45d3 100644 --- a/fdbserver/QuietDatabase.actor.cpp +++ b/fdbserver/QuietDatabase.actor.cpp @@ -638,6 +638,11 @@ ACTOR Future waitForQuietDatabase(Database cx, if (g_network->isSimulated()) wait(delay(5.0)); + TraceEvent("QuietDatabaseWaitingOnFullRecovery").log(); + while (dbInfo->get().recoveryState != RecoveryState::FULLY_RECOVERED) { + wait(dbInfo->onChange()); + } + // The quiet database check (which runs at the end of every test) will always time out due to active data movement. // To get around this, quiet Database will disable the perpetual wiggle in the setup phase. From f39e34cabe05d3f33792955d380a4b5aa3818422 Mon Sep 17 00:00:00 2001 From: Renxuan Wang Date: Wed, 20 Oct 2021 16:11:11 -0700 Subject: [PATCH 26/42] =?UTF-8?q?Use=20--enable-prof=20switch=20when=20bui?= =?UTF-8?q?lding=20jemalloc=E3=80=82?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I got errors like `: Invalid conf pair: prof:true` when trying to use jemalloc. Referring to https://stackoverflow.com/questions/27422508/heap-dump-fails-with-jemalloc-mcllctl, seems that we are missing out this flag. https://github.com/jeffgriffith/native-jvm-leaks#building. --- cmake/Jemalloc.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/Jemalloc.cmake b/cmake/Jemalloc.cmake index e89ef3ce82..176c88c9b6 100644 --- a/cmake/Jemalloc.cmake +++ b/cmake/Jemalloc.cmake @@ -35,7 +35,7 @@ else() BUILD_BYPRODUCTS "${JEMALLOC_DIR}/include/jemalloc/jemalloc.h" "${JEMALLOC_DIR}/lib/libjemalloc.a" "${JEMALLOC_DIR}/lib/libjemalloc_pic.a" - CONFIGURE_COMMAND ./configure --prefix=${JEMALLOC_DIR} --enable-static --disable-cxx + CONFIGURE_COMMAND ./configure --prefix=${JEMALLOC_DIR} --enable-static --disable-cxx --enable-prof BUILD_IN_SOURCE ON BUILD_COMMAND make INSTALL_DIR "${JEMALLOC_DIR}" From ac23751608205fa2879c2f76669340edde8246dd Mon Sep 17 00:00:00 2001 From: Yao Xiao <87789492+yao-xiao-github@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:54:04 -0700 Subject: [PATCH 27/42] Update release-notes-630.rst --- .../sphinx/source/release-notes/release-notes-630.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/documentation/sphinx/source/release-notes/release-notes-630.rst b/documentation/sphinx/source/release-notes/release-notes-630.rst index b54258baca..05b972da13 100644 --- a/documentation/sphinx/source/release-notes/release-notes-630.rst +++ b/documentation/sphinx/source/release-notes/release-notes-630.rst @@ -1,6 +1,11 @@ ############# Release Notes ############# +6.3.22 +====== +* Add histograms to client GRV batcher. `(PR #5760) `_ +* Add FastAlloc memory utilization trace. `(PR #5759) `_ +* Add locality cache size to TransactionMetrics. `(PR #5771) `_ 6.3.21 ====== From 1a24ad33dde4bb00593201256844231af67da550 Mon Sep 17 00:00:00 2001 From: Yao Xiao <87789492+yao-xiao-github@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:57:34 -0700 Subject: [PATCH 28/42] Update documentation/sphinx/source/release-notes/release-notes-630.rst Co-authored-by: A.J. Beamon --- documentation/sphinx/source/release-notes/release-notes-630.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/documentation/sphinx/source/release-notes/release-notes-630.rst b/documentation/sphinx/source/release-notes/release-notes-630.rst index 05b972da13..693956182f 100644 --- a/documentation/sphinx/source/release-notes/release-notes-630.rst +++ b/documentation/sphinx/source/release-notes/release-notes-630.rst @@ -1,6 +1,7 @@ ############# Release Notes ############# + 6.3.22 ====== * Add histograms to client GRV batcher. `(PR #5760) `_ From 2a1a5af93939826a689a513f7a32f06a43c63772 Mon Sep 17 00:00:00 2001 From: Yao Xiao <87789492+yao-xiao-github@users.noreply.github.com> Date: Mon, 18 Oct 2021 16:47:03 -0700 Subject: [PATCH 29/42] Update release-notes-630.rst Resolve comments --- .../sphinx/source/release-notes/release-notes-630.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/documentation/sphinx/source/release-notes/release-notes-630.rst b/documentation/sphinx/source/release-notes/release-notes-630.rst index 693956182f..1cac06d7a3 100644 --- a/documentation/sphinx/source/release-notes/release-notes-630.rst +++ b/documentation/sphinx/source/release-notes/release-notes-630.rst @@ -4,9 +4,9 @@ Release Notes 6.3.22 ====== -* Add histograms to client GRV batcher. `(PR #5760) `_ -* Add FastAlloc memory utilization trace. `(PR #5759) `_ -* Add locality cache size to TransactionMetrics. `(PR #5771) `_ +* Added histograms to client GRV batcher. `(PR #5760) `_ +* Added FastAlloc memory utilization trace. `(PR #5759) `_ +* Added locality cache size to TransactionMetrics. `(PR #5771) `_ 6.3.21 ====== From a58140c1bd4ec2d65a71ba0dfe0f897cd928a58a Mon Sep 17 00:00:00 2001 From: Yao Xiao <87789492+yao-xiao-github@users.noreply.github.com> Date: Thu, 21 Oct 2021 10:59:35 -0700 Subject: [PATCH 30/42] Update release-notes-630.rst --- documentation/sphinx/source/release-notes/release-notes-630.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/documentation/sphinx/source/release-notes/release-notes-630.rst b/documentation/sphinx/source/release-notes/release-notes-630.rst index 1cac06d7a3..735f3714c1 100644 --- a/documentation/sphinx/source/release-notes/release-notes-630.rst +++ b/documentation/sphinx/source/release-notes/release-notes-630.rst @@ -7,6 +7,7 @@ Release Notes * Added histograms to client GRV batcher. `(PR #5760) `_ * Added FastAlloc memory utilization trace. `(PR #5759) `_ * Added locality cache size to TransactionMetrics. `(PR #5771) `_ +* Added a new feature that allows FDB to failover to remote DC when the primary is experiencing massive grey failure. This feature is turned off by default. `(PR #5774) `_ 6.3.21 ====== From f03d32f3d4dc99ccbec2aebf9a9c273791bf87cf Mon Sep 17 00:00:00 2001 From: Evan Tschannen Date: Thu, 21 Oct 2021 23:04:51 -0700 Subject: [PATCH 31/42] fix: handle the case where a fetch happens at an earlier read version than the commit version of the change feed registration --- fdbserver/storageserver.actor.cpp | 93 +++++++++++++++++++------------ 1 file changed, 56 insertions(+), 37 deletions(-) diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 066757c63e..2b8526dd1b 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -3266,41 +3266,12 @@ static const KeyRangeRef persistChangeFeedKeys = KeyRangeRef(LiteralStringRef(PERSIST_PREFIX "RF/"), LiteralStringRef(PERSIST_PREFIX "RF0")); // data keys are unmangled (but never start with PERSIST_PREFIX because they are always in allKeys) -ACTOR Future fetchChangeFeed(StorageServer* data, - Key rangeId, - KeyRange range, - bool stopped, - Version fetchVersion) { - state Reference changeFeedInfo; - wait(delay(0)); // allow this actor to be cancelled by removals - bool existing = data->uidChangeFeed.count(rangeId); - - TraceEvent(SevDebug, "FetchChangeFeed", data->thisServerID) - .detail("RangeID", rangeId.printable()) - .detail("Range", range.toString()) - .detail("Existing", existing); - - if (!existing) { - changeFeedInfo = Reference(new ChangeFeedInfo()); - changeFeedInfo->range = range; - changeFeedInfo->id = rangeId; - changeFeedInfo->stopped = stopped; - data->uidChangeFeed[rangeId] = changeFeedInfo; - auto rs = data->keyChangeFeed.modify(range); - for (auto r = rs.begin(); r != rs.end(); ++r) { - r->value().push_back(changeFeedInfo); - } - data->keyChangeFeed.coalesce(range.contents()); - auto& mLV = data->addVersionToMutationLog(data->data().getLatestVersion()); - data->addMutationToMutationLog( - mLV, - MutationRef(MutationRef::SetValue, - persistChangeFeedKeys.begin.toString() + rangeId.toString(), - changeFeedValue(range, invalidVersion, ChangeFeedStatus::CHANGE_FEED_CREATE))); - } else { - changeFeedInfo = data->uidChangeFeed[rangeId]; - } - +ACTOR Future fetchChangeFeedApplier(StorageServer* data, + Reference changeFeedInfo, + Key rangeId, + KeyRange range, + Version fetchVersion, + bool existing) { state PromiseStream>> feedResults; state Future feed = data->cx->getChangeFeedStream( feedResults, rangeId, 0, existing ? fetchVersion + 1 : data->version.get() + 1, range); @@ -3321,7 +3292,7 @@ ACTOR Future fetchChangeFeed(StorageServer* data, wait(yield()); } } catch (Error& e) { - if (e.code() != error_code_end_of_stream && e.code() != error_code_change_feed_not_registered) { + if (e.code() != error_code_end_of_stream) { throw; } return Void(); @@ -3377,13 +3348,61 @@ ACTOR Future fetchChangeFeed(StorageServer* data, wait(yield()); } } catch (Error& e) { - if (e.code() != error_code_end_of_stream && e.code() != error_code_change_feed_not_registered) { + if (e.code() != error_code_end_of_stream) { throw; } } return Void(); } +ACTOR Future fetchChangeFeed(StorageServer* data, + Key rangeId, + KeyRange range, + bool stopped, + Version fetchVersion) { + state Reference changeFeedInfo; + wait(delay(0)); // allow this actor to be cancelled by removals + state bool existing = data->uidChangeFeed.count(rangeId); + + TraceEvent(SevDebug, "FetchChangeFeed", data->thisServerID) + .detail("RangeID", rangeId.printable()) + .detail("Range", range.toString()) + .detail("Existing", existing); + + if (!existing) { + changeFeedInfo = Reference(new ChangeFeedInfo()); + changeFeedInfo->range = range; + changeFeedInfo->id = rangeId; + changeFeedInfo->stopped = stopped; + data->uidChangeFeed[rangeId] = changeFeedInfo; + auto rs = data->keyChangeFeed.modify(range); + for (auto r = rs.begin(); r != rs.end(); ++r) { + r->value().push_back(changeFeedInfo); + } + data->keyChangeFeed.coalesce(range.contents()); + auto& mLV = data->addVersionToMutationLog(data->data().getLatestVersion()); + data->addMutationToMutationLog( + mLV, + MutationRef(MutationRef::SetValue, + persistChangeFeedKeys.begin.toString() + rangeId.toString(), + changeFeedValue(range, invalidVersion, ChangeFeedStatus::CHANGE_FEED_CREATE))); + } else { + changeFeedInfo = data->uidChangeFeed[rangeId]; + } + + loop { + try { + wait(fetchChangeFeedApplier(data, changeFeedInfo, rangeId, range, fetchVersion, existing)); + return Void(); + } catch (Error& e) { + if (e.code() != error_code_change_feed_not_registered) { + throw; + } + } + wait(delay(FLOW_KNOBS->PREVENT_FAST_SPIN_DELAY)); + } +} + ACTOR Future dispatchChangeFeeds(StorageServer* data, UID fetchKeysID, KeyRange keys, Version fetchVersion) { // find overlapping range feeds state std::map> feedFetches; From e882eb33fc0dc2ff706d16591a5330f82de8df1d Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Sun, 10 Oct 2021 20:44:56 -0700 Subject: [PATCH 32/42] Abstract the cluster file into a cluster connection record that can be backed by something other than the filesystem. --- fdbbackup/backup.actor.cpp | 3 +- fdbcli/fdbcli.actor.cpp | 9 +- fdbclient/CMakeLists.txt | 6 + fdbclient/ClusterConnectionFile.actor.cpp | 174 ++++++++++++++ fdbclient/ClusterConnectionFile.h | 81 +++++++ fdbclient/ClusterConnectionKey.actor.cpp | 144 ++++++++++++ fdbclient/ClusterConnectionKey.actor.h | 97 ++++++++ .../ClusterConnectionMemoryRecord.actor.cpp | 68 ++++++ fdbclient/ClusterConnectionMemoryRecord.h | 73 ++++++ fdbclient/CoordinationInterface.h | 105 ++++++--- fdbclient/DatabaseContext.h | 12 +- fdbclient/ManagementAPI.actor.cpp | 46 ++-- fdbclient/ManagementAPI.actor.h | 4 +- fdbclient/MonitorLeader.actor.cpp | 221 +++++------------- fdbclient/MonitorLeader.h | 16 +- fdbclient/NativeAPI.actor.cpp | 92 ++++---- fdbclient/NativeAPI.actor.h | 2 +- fdbclient/PaxosConfigTransaction.actor.cpp | 2 +- fdbclient/ReadYourWrites.actor.cpp | 18 +- fdbclient/SimpleConfigTransaction.actor.cpp | 2 +- fdbclient/SpecialKeySpace.actor.cpp | 9 +- fdbclient/StatusClient.actor.cpp | 28 +-- fdbclient/ThreadSafeTransaction.cpp | 1 + fdbserver/ClusterController.actor.cpp | 23 +- fdbserver/CoordinatedState.actor.cpp | 6 +- fdbserver/Coordination.actor.cpp | 50 ++-- fdbserver/CoordinationInterface.h | 4 +- fdbserver/LeaderElection.actor.cpp | 10 +- fdbserver/RestoreWorker.actor.cpp | 4 +- fdbserver/RestoreWorkerInterface.actor.h | 4 +- fdbserver/SimulatedCluster.actor.cpp | 42 ++-- fdbserver/Status.actor.cpp | 6 +- fdbserver/TesterInterface.actor.h | 4 +- fdbserver/WorkerInterface.actor.h | 8 +- fdbserver/fdbserver.actor.cpp | 11 +- fdbserver/masterserver.actor.cpp | 2 +- fdbserver/storageserver.actor.cpp | 10 +- fdbserver/tester.actor.cpp | 20 +- fdbserver/worker.actor.cpp | 111 +++++---- fdbserver/workloads/ApiWorkload.h | 3 +- .../workloads/AtomicSwitchover.actor.cpp | 3 +- fdbserver/workloads/BackupToDBAbort.actor.cpp | 3 +- .../workloads/BackupToDBCorrectness.actor.cpp | 3 +- .../workloads/BackupToDBUpgrade.actor.cpp | 3 +- fdbserver/workloads/ChangeConfig.actor.cpp | 3 +- .../DifferentClustersSameRV.actor.cpp | 11 +- fdbserver/workloads/KillRegion.actor.cpp | 2 +- fdbserver/workloads/VersionStamp.actor.cpp | 5 +- fdbserver/workloads/WriteDuringRead.actor.cpp | 3 +- 49 files changed, 1102 insertions(+), 465 deletions(-) create mode 100644 fdbclient/ClusterConnectionFile.actor.cpp create mode 100644 fdbclient/ClusterConnectionFile.h create mode 100644 fdbclient/ClusterConnectionKey.actor.cpp create mode 100644 fdbclient/ClusterConnectionKey.actor.h create mode 100644 fdbclient/ClusterConnectionMemoryRecord.actor.cpp create mode 100644 fdbclient/ClusterConnectionMemoryRecord.h diff --git a/fdbbackup/backup.actor.cpp b/fdbbackup/backup.actor.cpp index f8833c3d78..61a734ecb6 100644 --- a/fdbbackup/backup.actor.cpp +++ b/fdbbackup/backup.actor.cpp @@ -37,6 +37,7 @@ #include "fdbclient/BackupAgent.actor.h" #include "fdbclient/Status.h" #include "fdbclient/BackupContainer.h" +#include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/KeyBackedTypes.h" #include "fdbclient/IKnobCollection.h" #include "fdbclient/RunTransaction.actor.h" @@ -3095,7 +3096,7 @@ Optional connectToCluster(std::string const& clusterFile, } catch (Error& e) { if (!quiet) { fprintf(stderr, "ERROR: %s\n", e.what()); - fprintf(stderr, "ERROR: Unable to connect to cluster from `%s'\n", ccf->getFilename().c_str()); + fprintf(stderr, "ERROR: Unable to connect to cluster from `%s'\n", ccf->getLocation().c_str()); } return db; } diff --git a/fdbcli/fdbcli.actor.cpp b/fdbcli/fdbcli.actor.cpp index 1f1525c8b9..6995beab30 100644 --- a/fdbcli/fdbcli.actor.cpp +++ b/fdbcli/fdbcli.actor.cpp @@ -19,6 +19,7 @@ */ #include "boost/lexical_cast.hpp" +#include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/NativeAPI.actor.h" #include "fdbclient/FDBTypes.h" #include "fdbclient/IClientApi.h" @@ -1034,8 +1035,8 @@ ACTOR Future exclude(Database db, locality.c_str()); } + ClusterConnectionString ccs = wait(ccf->getStoredConnectionString()); bool foundCoordinator = false; - auto ccs = ClusterConnectionFile(ccf->getFilename()).getConnectionString(); for (const auto& c : ccs.coordinators()) { if (std::count(exclusionVector.begin(), exclusionVector.end(), AddressExclusion(c.ip, c.port)) || std::count(exclusionVector.begin(), exclusionVector.end(), AddressExclusion(c.ip))) { @@ -1584,12 +1585,12 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { try { localDb = Database::createDatabase(ccf, opt.api_version, IsInternal::False); if (!opt.exec.present()) { - printf("Using cluster file `%s'.\n", ccf->getFilename().c_str()); + printf("Using cluster file `%s'.\n", ccf->getLocation().c_str()); } db = API->createDatabase(opt.clusterFile.c_str()); } catch (Error& e) { fprintf(stderr, "ERROR: %s (%d)\n", e.what(), e.code()); - printf("Unable to connect to cluster from `%s'\n", ccf->getFilename().c_str()); + printf("Unable to connect to cluster from `%s'\n", ccf->getLocation().c_str()); return 1; } @@ -1600,7 +1601,7 @@ ACTOR Future cli(CLIOptions opt, LineNoise* plinenoise) { .detail("Version", FDB_VT_VERSION) .detail("PackageName", FDB_VT_PACKAGE_NAME) .detailf("ActualTime", "%lld", DEBUG_DETERMINISM ? 0 : time(nullptr)) - .detail("ClusterFile", ccf->getFilename().c_str()) + .detail("ClusterFile", ccf->toString()) .detail("ConnectionString", ccf->getConnectionString().toString()) .setMaxFieldLength(10000) .detail("CommandLine", opt.commandLine) diff --git a/fdbclient/CMakeLists.txt b/fdbclient/CMakeLists.txt index e90085e977..c11663b62f 100644 --- a/fdbclient/CMakeLists.txt +++ b/fdbclient/CMakeLists.txt @@ -26,6 +26,12 @@ set(FDBCLIENT_SRCS ClientKnobs.h ClientLogEvents.h ClientWorkerInterface.h + ClusterConnectionFile.actor.cpp + ClusterConnectionFile.h + ClusterConnectionKey.actor.cpp + ClusterConnectionKey.actor.h + ClusterConnectionMemoryRecord.actor.cpp + ClusterConnectionMemoryRecord.h ClusterInterface.h CommitProxyInterface.h CommitTransaction.h diff --git a/fdbclient/ClusterConnectionFile.actor.cpp b/fdbclient/ClusterConnectionFile.actor.cpp new file mode 100644 index 0000000000..9cfb410382 --- /dev/null +++ b/fdbclient/ClusterConnectionFile.actor.cpp @@ -0,0 +1,174 @@ +/* + * ClusterConnectionFile.actor.cpp + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbclient/ClusterConnectionFile.h" +#include "fdbclient/MonitorLeader.h" +#include "flow/actorcompiler.h" // has to be last include + +// Loads and parses the file at 'path', throwing errors if the file cannot be read or the format is invalid. +ClusterConnectionFile::ClusterConnectionFile(std::string const& filename) : IClusterConnectionRecord(false) { + if (!fileExists(filename)) { + throw no_cluster_file_found(); + } + + cs = ClusterConnectionString(readFileBytes(filename, MAX_CLUSTER_FILE_BYTES)); + this->filename = filename; +} + +// Creates a cluster file with a given connection string and saves it to the specified file. +ClusterConnectionFile::ClusterConnectionFile(std::string const& filename, ClusterConnectionString const& contents) + : IClusterConnectionRecord(true) { + this->filename = filename; + cs = contents; +} + +// Returns the connection string currently held in this object. This may not match the string on disk if it hasn't +// been persisted or if the file has been modified externally. +ClusterConnectionString const& ClusterConnectionFile::getConnectionString() const { + return cs; +} + +// Sets the connections string held by this object. Calling this function does not persist the string to disk. +Future ClusterConnectionFile::setConnectionString(ClusterConnectionString const& conn) { + ASSERT(filename.size()); + cs = conn; + return success(persist()); +} + +// Get the connection string stored in the file. +Future ClusterConnectionFile::getStoredConnectionString() { + try { + return ClusterConnectionFile(filename).cs; + } catch (Error& e) { + return e; + } +} + +// Checks whether the connection string in the file matches the connection string stored in memory. The cluster +// string stored in the file is returned via the reference parameter connectionString. +Future ClusterConnectionFile::upToDate(ClusterConnectionString& fileConnectionString) { + try { + // the cluster file hasn't been created yet so there's nothing to check + if (needsToBePersisted()) + return true; + + ClusterConnectionFile temp(filename); + fileConnectionString = temp.getConnectionString(); + return fileConnectionString.toString() == cs.toString(); + } catch (Error& e) { + TraceEvent(SevWarnAlways, "ClusterFileError").error(e).detail("Filename", filename); + return false; // Swallow the error and report that the file is out of date + } +} + +// Returns the specified path of the cluster file. +std::string ClusterConnectionFile::getLocation() const { + return filename; +} + +// Creates a copy of this object with a modified connection string but that isn't persisted. +Reference ClusterConnectionFile::makeIntermediateRecord( + ClusterConnectionString const& connectionString) const { + return makeReference(filename, connectionString); +} + +// Returns a string representation of this cluster connection record. This will include the type of record and the +// filename of the cluster file. +std::string ClusterConnectionFile::toString() const { + return "File: " + filename; +} + +// returns +std::pair ClusterConnectionFile::lookupClusterFileName(std::string const& filename) { + if (filename.length()) + return std::make_pair(filename, false); + + std::string f; + bool isDefaultFile = true; + if (platform::getEnvironmentVar(CLUSTER_FILE_ENV_VAR_NAME, f)) { + // If this is set but points to a file that does not + // exist, we will not fallback to any other methods + isDefaultFile = false; + } else if (fileExists("fdb.cluster")) + f = "fdb.cluster"; + else + f = platform::getDefaultClusterFilePath(); + + return std::make_pair(f, isDefaultFile); +} + +// get a human readable error message describing the error returned from the constructor +std::string ClusterConnectionFile::getErrorString(std::pair const& resolvedClusterFile, + Error const& e) { + bool isDefault = resolvedClusterFile.second; + if (e.code() == error_code_connection_string_invalid) { + return format("Invalid cluster file `%s': %d %s", resolvedClusterFile.first.c_str(), e.code(), e.what()); + } else if (e.code() == error_code_no_cluster_file_found) { + if (isDefault) + return format("Unable to read cluster file `./fdb.cluster' or `%s' and %s unset: %d %s", + platform::getDefaultClusterFilePath().c_str(), + CLUSTER_FILE_ENV_VAR_NAME, + e.code(), + e.what()); + else + return format( + "Unable to read cluster file `%s': %d %s", resolvedClusterFile.first.c_str(), e.code(), e.what()); + } else { + return format( + "Unexpected error loading cluster file `%s': %d %s", resolvedClusterFile.first.c_str(), e.code(), e.what()); + } +} + +// Writes the connection string to the cluster file +Future ClusterConnectionFile::persist() { + setPersisted(); + + if (filename.size()) { + try { + atomicReplace(filename, + "# DO NOT EDIT!\n# This file is auto-generated, it is not to be edited by hand\n" + + cs.toString().append("\n")); + + Future isUpToDate = IClusterConnectionRecord::upToDate(); + + // The implementation of upToDate in this class is synchronous + ASSERT(isUpToDate.isReady()); + + if (!isUpToDate.get()) { + // This should only happen in rare scenarios where multiple processes are updating the same file to + // different values simultaneously In that case, we don't have any guarantees about which file will + // ultimately be written + TraceEvent(SevWarnAlways, "ClusterFileChangedAfterReplace") + .detail("Filename", filename) + .detail("ConnStr", cs.toString()); + return false; + } + + return true; + } catch (Error& e) { + TraceEvent(SevWarnAlways, "UnableToChangeConnectionFile") + .error(e) + .detail("Filename", filename) + .detail("ConnStr", cs.toString()); + } + } + + return false; +} diff --git a/fdbclient/ClusterConnectionFile.h b/fdbclient/ClusterConnectionFile.h new file mode 100644 index 0000000000..22a07bc779 --- /dev/null +++ b/fdbclient/ClusterConnectionFile.h @@ -0,0 +1,81 @@ +/* + * ClusterConnectionFile.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#ifndef FDBCLIENT_CLUSTERCONNECTIONFILE_H +#define FDBCLIENT_CLUSTERCONNECTIONFILE_H + +#include "fdbclient/CoordinationInterface.h" +#include "flow/actorcompiler.h" // has to be last include + +// An implementation of IClusterConnectionRecord backed by a file. +class ClusterConnectionFile : public IClusterConnectionRecord, ReferenceCounted, NonCopyable { +public: + // Loads and parses the file at 'path', throwing errors if the file cannot be read or the format is invalid. + explicit ClusterConnectionFile(std::string const& path); + + // Creates a cluster file with a given connection string and saves it to the specified file. + explicit ClusterConnectionFile(std::string const& filename, ClusterConnectionString const& contents); + + // Returns the connection string currently held in this object. This may not match the string on disk if it hasn't + // been persisted or if the file has been modified externally. + ClusterConnectionString const& getConnectionString() const override; + + // Sets the connections string held by this object. Calling this function does not persist the string to disk. + Future setConnectionString(ClusterConnectionString const&) override; + + // Get the connection string stored in the file. + Future getStoredConnectionString() override; + + // Checks whether the connection string in the file matches the connection string stored in memory. The cluster + // string stored in the file is returned via the reference parameter connectionString. + Future upToDate(ClusterConnectionString& fileConnectionString) override; + + // Returns the specified path of the cluster file. + std::string getLocation() const override; + + // Creates a copy of this object with a modified connection string but that isn't persisted. + Reference makeIntermediateRecord( + ClusterConnectionString const& connectionString) const override; + + // Returns a string representation of this cluster connection record. This will include the type of record and the + // filename of the cluster file. + std::string toString() const override; + + void addref() override { ReferenceCounted::addref(); } + void delref() override { ReferenceCounted::delref(); } + + // returns + static std::pair lookupClusterFileName(std::string const& filename); + + // get a human readable error message describing the error returned from the constructor + static std::string getErrorString(std::pair const& resolvedFile, Error const& e); + +protected: + // Writes the connection string to the cluster file + Future persist() override; + +private: + ClusterConnectionString cs; + std::string filename; +}; + +#include "flow/unactorcompiler.h" +#endif \ No newline at end of file diff --git a/fdbclient/ClusterConnectionKey.actor.cpp b/fdbclient/ClusterConnectionKey.actor.cpp new file mode 100644 index 0000000000..e1864b78ca --- /dev/null +++ b/fdbclient/ClusterConnectionKey.actor.cpp @@ -0,0 +1,144 @@ +/* + * ClusterConnectionKey.actor.cpp + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbclient/ClusterConnectionKey.actor.h" +#include "flow/actorcompiler.h" // has to be last include + +// Creates a cluster connection record with a given connection string and saves it to the specified key. Needs to be +// persisted should be set to true unless this ClusterConnectionKey is being created with the value read from the +// key. +ClusterConnectionKey::ClusterConnectionKey(Database db, + Key connectionStringKey, + ClusterConnectionString const& contents, + bool needsToBePersisted) + : IClusterConnectionRecord(needsToBePersisted), db(db), cs(contents), connectionStringKey(connectionStringKey) {} + +// Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the +// format is invalid. +ACTOR Future> ClusterConnectionKey::loadClusterConnectionKey(Database db, + Key connectionStringKey) { + state Transaction tr(db); + loop { + try { + Optional v = wait(tr.get(connectionStringKey)); + if (!v.present()) { + throw connection_string_invalid(); + } + return makeReference( + db, connectionStringKey, ClusterConnectionString(v.get().toString()), false); + } catch (Error& e) { + wait(tr.onError(e)); + } + } +} + +// Returns the connection string currently held in this object. This may not match the string in the database if it +// hasn't been persisted or if the key has been modified externally. +ClusterConnectionString const& ClusterConnectionKey::getConnectionString() const { + return cs; +} + +// Sets the connections string held by this object. Calling this function does not persist the string to the +// database. +Future ClusterConnectionKey::setConnectionString(ClusterConnectionString const& connectionString) { + cs = connectionString; + return success(persist()); +} + +// Get the connection string stored in the database. +ACTOR Future ClusterConnectionKey::getStoredConnectionStringImpl( + Reference self) { + Reference cck = + wait(ClusterConnectionKey::loadClusterConnectionKey(self->db, self->connectionStringKey)); + return cck->cs; +} + +Future ClusterConnectionKey::getStoredConnectionString() { + return getStoredConnectionStringImpl(Reference::addRef(this)); +} + +ACTOR Future ClusterConnectionKey::upToDateImpl(Reference self, + ClusterConnectionString* connectionString) { + try { + // the cluster file hasn't been created yet so there's nothing to check + if (self->needsToBePersisted()) + return true; + + Reference temp = + wait(ClusterConnectionKey::loadClusterConnectionKey(self->db, self->connectionStringKey)); + *connectionString = temp->getConnectionString(); + return connectionString->toString() == self->cs.toString(); + } catch (Error& e) { + TraceEvent(SevWarnAlways, "ClusterKeyError").error(e).detail("Key", self->connectionStringKey); + return false; // Swallow the error and report that the file is out of date + } +} + +// Checks whether the connection string in the database matches the connection string stored in memory. The cluster +// string stored in the database is returned via the reference parameter connectionString. +Future ClusterConnectionKey::upToDate(ClusterConnectionString& connectionString) { + return upToDateImpl(Reference::addRef(this), &connectionString); +} + +// Returns the key where the connection string is stored. +std::string ClusterConnectionKey::getLocation() const { + return printable(connectionStringKey); +} + +// Creates a copy of this object with a modified connection string but that isn't persisted. +Reference ClusterConnectionKey::makeIntermediateRecord( + ClusterConnectionString const& connectionString) const { + return makeReference(db, connectionStringKey, connectionString); +} + +// Returns a string representation of this cluster connection record. This will include the type of record and the +// key where the record is stored. +std::string ClusterConnectionKey::toString() const { + return "Key: " + printable(connectionStringKey); +} + +ACTOR Future ClusterConnectionKey::persistImpl(Reference self) { + self->setPersisted(); + + try { + state Transaction tr(self->db); + loop { + try { + tr.set(self->connectionStringKey, StringRef(self->cs.toString())); + wait(tr.commit()); + return true; + } catch (Error& e) { + wait(tr.onError(e)); + } + } + } catch (Error& e) { + TraceEvent(SevWarnAlways, "UnableToChangeConnectionKey") + .error(e) + .detail("ConnectionKey", self->connectionStringKey) + .detail("ConnStr", self->cs.toString()); + } + + return false; +}; + +// Writes the connection string to the database +Future ClusterConnectionKey::persist() { + return persistImpl(Reference::addRef(this)); +} \ No newline at end of file diff --git a/fdbclient/ClusterConnectionKey.actor.h b/fdbclient/ClusterConnectionKey.actor.h new file mode 100644 index 0000000000..a4001c26c5 --- /dev/null +++ b/fdbclient/ClusterConnectionKey.actor.h @@ -0,0 +1,97 @@ +/* + * ClusterConnectionKey.actor.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +// When actually compiled (NO_INTELLISENSE), include the generated version of this file. In intellisense use the source +// version. +#if defined(NO_INTELLISENSE) && !defined(FDBCLIENT_CLUSTERCONNECTIONKEY_ACTOR_G_H) +#define FDBCLIENT_CLUSTERCONNECTIONKEY_ACTOR_G_H +#include "fdbclient/ClusterConnectionKey.actor.g.h" +#elif !defined(FDBCLIENT_CLUSTERCONNECTIONKEY_ACTOR_H) +#define FDBCLIENT_CLUSTERCONNECTIONKEY_ACTOR_H + +#include "fdbclient/CoordinationInterface.h" +#include "fdbclient/NativeAPI.actor.h" +#include "flow/actorcompiler.h" // has to be last include + +// An implementation of IClusterConnectionRecord backed by a key in a FoundationDB database. +class ClusterConnectionKey : public IClusterConnectionRecord, ReferenceCounted, NonCopyable { +public: + // Creates a cluster connection record with a given connection string and saves it to the specified key. Needs to be + // persisted should be set to true unless this ClusterConnectionKey is being created with the value read from the + // key. + ClusterConnectionKey(Database db, + Key connectionStringKey, + ClusterConnectionString const& contents, + bool needsToBePersisted = true); + + // Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the + // format is invalid. + ACTOR static Future> loadClusterConnectionKey(Database db, Key connectionStringKey); + + // Returns the connection string currently held in this object. This may not match the string in the database if it + // hasn't been persisted or if the key has been modified externally. + ClusterConnectionString const& getConnectionString() const override; + + // Sets the connections string held by this object. Calling this function does not persist the string to the + // database. + Future setConnectionString(ClusterConnectionString const&) override; + + // Get the connection string stored in the database. + Future getStoredConnectionString() override; + + // Checks whether the connection string in the database matches the connection string stored in memory. The cluster + // string stored in the database is returned via the reference parameter connectionString. + Future upToDate(ClusterConnectionString& connectionString) override; + + // Returns the key where the connection string is stored. + std::string getLocation() const override; + + // Creates a copy of this object with a modified connection string but that isn't persisted. + Reference makeIntermediateRecord( + ClusterConnectionString const& connectionString) const override; + + // Returns a string representation of this cluster connection record. This will include the type of record and the + // key where the record is stored. + std::string toString() const override; + + void addref() override { ReferenceCounted::addref(); } + void delref() override { ReferenceCounted::delref(); } + +protected: + // Writes the connection string to the database + Future persist() override; + +private: + ACTOR static Future getStoredConnectionStringImpl(Reference self); + ACTOR static Future upToDateImpl(Reference self, + ClusterConnectionString* connectionString); + ACTOR static Future persistImpl(Reference self); + + // The database where the connection key is stored. Note that this does not need to be the same database as the one + // that the connection string would connect to. + Database db; + ClusterConnectionString cs; + Key connectionStringKey; +}; + +#include "flow/unactorcompiler.h" +#endif \ No newline at end of file diff --git a/fdbclient/ClusterConnectionMemoryRecord.actor.cpp b/fdbclient/ClusterConnectionMemoryRecord.actor.cpp new file mode 100644 index 0000000000..1a50213bda --- /dev/null +++ b/fdbclient/ClusterConnectionMemoryRecord.actor.cpp @@ -0,0 +1,68 @@ +/* + * ClusterConnectionMemoryRecord.actor.cpp + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "fdbclient/ClusterConnectionMemoryRecord.h" +#include "fdbclient/MonitorLeader.h" +#include "flow/actorcompiler.h" // has to be last include + +// Returns the connection string currently held in this object. +ClusterConnectionString const& ClusterConnectionMemoryRecord::getConnectionString() const { + return cs; +} + +// Sets the connections string held by this object. +Future ClusterConnectionMemoryRecord::setConnectionString(ClusterConnectionString const& conn) { + cs = conn; + return Void(); +} + +// Returns the connection string currently held in this object (there is no persistent storage). +Future ClusterConnectionMemoryRecord::getStoredConnectionString() { + return cs; +} + +// Because the memory record is not persisted, it is always up to date and this returns true. The connection string +// is returned via the reference parameter connectionString. +Future ClusterConnectionMemoryRecord::upToDate(ClusterConnectionString& fileConnectionString) { + fileConnectionString = cs; + return true; +} + +// Returns the ID of the memory record. +std::string ClusterConnectionMemoryRecord::getLocation() const { + return id.toString(); +} + +// Returns a copy of this object with a modified connection string. +Reference ClusterConnectionMemoryRecord::makeIntermediateRecord( + ClusterConnectionString const& connectionString) const { + return makeReference(connectionString); +} + +// Returns a string representation of this cluster connection record. This will include the type and id of the +// record. +std::string ClusterConnectionMemoryRecord::toString() const { + return "Memory: " + id.toString(); +} + +// This is a no-op for memory records. Returns true to indicate success. +Future ClusterConnectionMemoryRecord::persist() { + return true; +} \ No newline at end of file diff --git a/fdbclient/ClusterConnectionMemoryRecord.h b/fdbclient/ClusterConnectionMemoryRecord.h new file mode 100644 index 0000000000..062c1894fb --- /dev/null +++ b/fdbclient/ClusterConnectionMemoryRecord.h @@ -0,0 +1,73 @@ +/* + * ClusterConnectionMemoryRecord.h + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2013-2021 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#ifndef FDBCLIENT_CLUSTERCONNECTIONMEMORYRECORD_H +#define FDBCLIENT_CLUSTERCONNECTIONMEMORYRECORD_H + +#include "fdbclient/CoordinationInterface.h" + +// An implementation of IClusterConnectionRecord that is stored in memory only and not persisted. +class ClusterConnectionMemoryRecord : public IClusterConnectionRecord, + ReferenceCounted, + NonCopyable { +public: + // Creates a cluster file with a given connection string. + explicit ClusterConnectionMemoryRecord(ClusterConnectionString const& cs) + : IClusterConnectionRecord(false), id(deterministicRandom()->randomUniqueID()), cs(cs) {} + + // Returns the connection string currently held in this object. + ClusterConnectionString const& getConnectionString() const override; + + // Sets the connections string held by this object. + Future setConnectionString(ClusterConnectionString const&) override; + + // Returns the connection string currently held in this object (there is no persistent storage). + Future getStoredConnectionString() override; + + // Because the memory record is not persisted, it is always up to date and this returns true. The connection string + // is returned via the reference parameter connectionString. + Future upToDate(ClusterConnectionString& fileConnectionString) override; + + // Returns a location string for the memory record that includes its ID. + std::string getLocation() const override; + + // Returns a copy of this object with a modified connection string. + Reference makeIntermediateRecord( + ClusterConnectionString const& connectionString) const override; + + // Returns a string representation of this cluster connection record. This will include the type and id of the + // record. + std::string toString() const override; + + void addref() override { ReferenceCounted::addref(); } + void delref() override { ReferenceCounted::delref(); } + +protected: + // This is a no-op for memory records. Returns true to indicate success. + Future persist() override; + +private: + // A unique ID for the record + UID id; + ClusterConnectionString cs; +}; + +#endif \ No newline at end of file diff --git a/fdbclient/CoordinationInterface.h b/fdbclient/CoordinationInterface.h index c1873a0b92..27bda0fe7e 100644 --- a/fdbclient/CoordinationInterface.h +++ b/fdbclient/CoordinationInterface.h @@ -45,11 +45,23 @@ struct ClientLeaderRegInterface { } }; +// A string containing the information necessary to connect to a cluster. +// +// The format of the connection string is: description:id@[addrs]+ +// The description and id together are called the "key" +// +// The following is enforced about the format of the file: +// - The key must contain one (and only one) ':' character +// - The description contains only allowed characters (a-z, A-Z, 0-9, _) +// - The ID contains only allowed characters (a-z, A-Z, 0-9) +// - At least one address is specified +// - There is no address present more than once class ClusterConnectionString { public: ClusterConnectionString() {} ClusterConnectionString(std::string const& connectionString); ClusterConnectionString(std::vector, Key); + std::vector const& coordinators() const { return coord; } Key clusterKey() const { return key; } Key clusterKeyName() const { @@ -65,45 +77,68 @@ private: Key key, keyDesc; }; -class ClusterConnectionFile : NonCopyable, public ReferenceCounted { +// A record that stores the connection string used to connect to a cluster. This record can be updated when a cluster +// notifies a connected party that the connection string has changed. +// +// The typically used cluster connection record is a cluster file (implemented in ClusterConnectionFile). This interface +// provides an abstraction over the cluster file so that we can persist the connection string in other locations or have +// one that is only stored in memory. +class IClusterConnectionRecord { public: - ClusterConnectionFile() {} - // Loads and parses the file at 'path', throwing errors if the file cannot be read or the format is invalid. - // - // The format of the file is: description:id@[addrs]+ - // The description and id together are called the "key" - // - // The following is enforced about the format of the file: - // - The key must contain one (and only one) ':' character - // - The description contains only allowed characters (a-z, A-Z, 0-9, _) - // - The ID contains only allowed characters (a-z, A-Z, 0-9) - // - At least one address is specified - // - There is no address present more than once - explicit ClusterConnectionFile(std::string const& path); - explicit ClusterConnectionFile(ClusterConnectionString const& cs) : cs(cs), setConn(false) {} - explicit ClusterConnectionFile(std::string const& filename, ClusterConnectionString const& contents); + IClusterConnectionRecord(bool connectionStringNeedsPersisted) + : connectionStringNeedsPersisted(connectionStringNeedsPersisted) {} + virtual ~IClusterConnectionRecord() {} - // returns - static std::pair lookupClusterFileName(std::string const& filename); - // get a human readable error message describing the error returned from the constructor - static std::string getErrorString(std::pair const& resolvedFile, Error const& e); + // Returns the connection string currently held in this object. This may not match the stored record if it hasn't + // been persisted or if the persistent storage for the record has been modified externally. + virtual ClusterConnectionString const& getConnectionString() const = 0; - ClusterConnectionString const& getConnectionString() const; - bool writeFile(); - void setConnectionString(ClusterConnectionString const&); - std::string const& getFilename() const { - ASSERT(filename.size()); - return filename; - } - bool canGetFilename() const { return filename.size() != 0; } - bool fileContentsUpToDate() const; - bool fileContentsUpToDate(ClusterConnectionString& fileConnectionString) const; + // Sets the connections string held by this object. Calling this function does not persist the record. + virtual Future setConnectionString(ClusterConnectionString const&) = 0; + + // If this record is backed by persistent storage, get the connection string from that storage. Otherwise, return + // the connection string stored in memory. + virtual Future getStoredConnectionString() = 0; + + // Checks whether the connection string in persisten storage matches the connection string stored in memory. + Future upToDate(); + + // Checks whether the connection string in persisten storage matches the connection string stored in memory. The + // cluster string stored in persistent storage is returned via the reference parameter connectionString. + virtual Future upToDate(ClusterConnectionString& connectionString) = 0; + + // Returns a string representing the location of the cluster record. For example, this could be the filename or key + // that stores the connection string. + virtual std::string getLocation() const = 0; + + // Creates a copy of this object with a modified connection string but that isn't persisted. + virtual Reference makeIntermediateRecord( + ClusterConnectionString const& connectionString) const = 0; + + // Returns a string representation of this cluster connection record. This will include the type and location of the + // record. + virtual std::string toString() const = 0; + + // Signals to the connection record that it was successfully used to connect to a cluster. void notifyConnected(); + virtual void addref() = 0; + virtual void delref() = 0; + +protected: + // Writes the connection string to the backing persistent storage, if applicable. + virtual Future persist() = 0; + + // Returns whether the connection record contains a connection string that should be persisted upon connection. + bool needsToBePersisted() const; + + // Clears the flag needs persisted flag. + void setPersisted(); + private: - ClusterConnectionString cs; - std::string filename; - bool setConn; + // A flag that indicates whether this connection record should be persisted when it succesfully establishes a + // connection. + bool connectionStringNeedsPersisted; }; struct LeaderInfo { @@ -199,9 +234,9 @@ class ClientCoordinators { public: std::vector clientLeaderServers; Key clusterKey; - Reference ccf; + Reference ccr; - explicit ClientCoordinators(Reference ccf); + explicit ClientCoordinators(Reference ccr); explicit ClientCoordinators(Key clusterKey, std::vector coordinators); ClientCoordinators() {} }; diff --git a/fdbclient/DatabaseContext.h b/fdbclient/DatabaseContext.h index 418b8681d3..0db71aa895 100644 --- a/fdbclient/DatabaseContext.h +++ b/fdbclient/DatabaseContext.h @@ -167,7 +167,7 @@ public: // Constructs a new copy of this DatabaseContext from the parameters of this DatabaseContext Database clone() const { - return Database(new DatabaseContext(connectionFile, + return Database(new DatabaseContext(connectionRecord, clientInfo, coordinator, clientInfoMonitor, @@ -231,16 +231,16 @@ public: Future onConnected(); // Returns after a majority of coordination servers are available and have reported a // leader. The cluster file therefore is valid, but the database might be unavailable. - Reference getConnectionFile(); + Reference getConnectionRecord(); // Switch the database to use the new connection file, and recreate all pending watches for committed transactions. // // Meant to be used as part of a 'hot standby' solution to switch to the standby. A correct switch will involve // advancing the version on the new cluster sufficiently far that any transaction begun with a read version from the // old cluster will fail to commit. Assuming the above version-advancing is done properly, a call to - // switchConnectionFile guarantees that any read with a version from the old cluster will not be attempted on the + // switchConnectionRecord guarantees that any read with a version from the old cluster will not be attempted on the // new cluster. - Future switchConnectionFile(Reference standby); + Future switchConnectionRecord(Reference standby); Future connectionFileChanged(); IsSwitchable switchable{ false }; @@ -253,7 +253,7 @@ public: Future createSnapshot(StringRef uid, StringRef snapshot_command); // private: - explicit DatabaseContext(Reference>> connectionFile, + explicit DatabaseContext(Reference>> connectionRecord, Reference> clientDBInfo, Reference> const> coordinator, Future clientInfoMonitor, @@ -270,7 +270,7 @@ public: void expireThrottles(); // Key DB-specific information - Reference>> connectionFile; + Reference>> connectionRecord; AsyncTrigger proxiesChangeTrigger; Future monitorProxiesInfoChange; Future monitorTssInfoChange; diff --git a/fdbclient/ManagementAPI.actor.cpp b/fdbclient/ManagementAPI.actor.cpp index cd6ed05dab..8e7e873bcf 100644 --- a/fdbclient/ManagementAPI.actor.cpp +++ b/fdbclient/ManagementAPI.actor.cpp @@ -24,6 +24,7 @@ #include "fdbclient/Knobs.h" #include "flow/Arena.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/FDBOptions.g.h" #include "fdbclient/FDBTypes.h" #include "fdbclient/ReadYourWrites.h" @@ -778,15 +779,18 @@ ACTOR Future> changeQuorumChecker(Transaction* tr, return CoordinatorsResult::BAD_DATABASE_STATE; // Someone deleted this key entirely? state ClusterConnectionString old(currentKey.get().toString()); - if (tr->getDatabase()->getConnectionFile() && + if (tr->getDatabase()->getConnectionRecord() && old.clusterKeyName().toString() != - tr->getDatabase()->getConnectionFile()->getConnectionString().clusterKeyName()) + tr->getDatabase()->getConnectionRecord()->getConnectionString().clusterKeyName()) return CoordinatorsResult::BAD_DATABASE_STATE; // Someone changed the "name" of the database?? state CoordinatorsResult result = CoordinatorsResult::SUCCESS; if (!desiredCoordinators->size()) { std::vector _desiredCoordinators = wait(change->getDesiredCoordinators( - tr, old.coordinators(), Reference(new ClusterConnectionFile(old)), result)); + tr, + old.coordinators(), + Reference(new ClusterConnectionMemoryRecord(old)), + result)); *desiredCoordinators = _desiredCoordinators; } @@ -821,7 +825,7 @@ ACTOR Future> changeQuorumChecker(Transaction* tr, } std::vector>> leaderServers; - ClientCoordinators coord(Reference(new ClusterConnectionFile(conn))); + ClientCoordinators coord(Reference(new ClusterConnectionMemoryRecord(conn))); leaderServers.reserve(coord.clientLeaderServers.size()); for (int i = 0; i < coord.clientLeaderServers.size(); i++) @@ -854,14 +858,17 @@ ACTOR Future changeQuorum(Database cx, ReferencegetConnectionFile() && - old.clusterKeyName().toString() != cx->getConnectionFile()->getConnectionString().clusterKeyName()) + if (cx->getConnectionRecord() && + old.clusterKeyName().toString() != cx->getConnectionRecord()->getConnectionString().clusterKeyName()) return CoordinatorsResult::BAD_DATABASE_STATE; // Someone changed the "name" of the database?? state CoordinatorsResult result = CoordinatorsResult::SUCCESS; if (!desiredCoordinators.size()) { std::vector _desiredCoordinators = wait(change->getDesiredCoordinators( - &tr, old.coordinators(), Reference(new ClusterConnectionFile(old)), result)); + &tr, + old.coordinators(), + Reference(new ClusterConnectionMemoryRecord(old)), + result)); desiredCoordinators = _desiredCoordinators; } @@ -907,7 +914,8 @@ ACTOR Future changeQuorum(Database cx, Reference>> leaderServers; - state ClientCoordinators coord(Reference(new ClusterConnectionFile(conn))); + state ClientCoordinators coord( + Reference(new ClusterConnectionMemoryRecord(conn))); // check if allowed to modify the cluster descriptor if (!change->getDesiredClusterKeyName().empty()) { CheckDescriptorMutableReply mutabilityReply = @@ -942,7 +950,7 @@ struct SpecifiedQuorumChange final : IQuorumChange { explicit SpecifiedQuorumChange(std::vector const& desired) : desired(desired) {} Future> getDesiredCoordinators(Transaction* tr, std::vector oldCoordinators, - Reference, + Reference, CoordinatorsResult&) override { return desired; } @@ -954,7 +962,7 @@ Reference specifiedQuorumChange(std::vector const struct NoQuorumChange final : IQuorumChange { Future> getDesiredCoordinators(Transaction* tr, std::vector oldCoordinators, - Reference, + Reference, CoordinatorsResult&) override { return oldCoordinators; } @@ -970,9 +978,9 @@ struct NameQuorumChange final : IQuorumChange { : newName(newName), otherChange(otherChange) {} Future> getDesiredCoordinators(Transaction* tr, std::vector oldCoordinators, - Reference cf, + Reference ccr, CoordinatorsResult& t) override { - return otherChange->getDesiredCoordinators(tr, oldCoordinators, cf, t); + return otherChange->getDesiredCoordinators(tr, oldCoordinators, ccr, t); } std::string getDesiredClusterKeyName() const override { return newName; } }; @@ -986,9 +994,9 @@ struct AutoQuorumChange final : IQuorumChange { Future> getDesiredCoordinators(Transaction* tr, std::vector oldCoordinators, - Reference ccf, + Reference ccr, CoordinatorsResult& err) override { - return getDesired(Reference::addRef(this), tr, oldCoordinators, ccf, &err); + return getDesired(Reference::addRef(this), tr, oldCoordinators, ccr, &err); } ACTOR static Future getRedundancy(AutoQuorumChange* self, Transaction* tr) { @@ -1006,7 +1014,7 @@ struct AutoQuorumChange final : IQuorumChange { ACTOR static Future isAcceptable(AutoQuorumChange* self, Transaction* tr, std::vector oldCoordinators, - Reference ccf, + Reference ccr, int desiredCount, std::set* excluded) { // Are there enough coordinators for the redundancy level? @@ -1016,7 +1024,7 @@ struct AutoQuorumChange final : IQuorumChange { return false; // Check availability - ClientCoordinators coord(ccf); + ClientCoordinators coord(ccr); std::vector>> leaderServers; leaderServers.reserve(coord.clientLeaderServers.size()); for (int i = 0; i < coord.clientLeaderServers.size(); i++) { @@ -1054,7 +1062,7 @@ struct AutoQuorumChange final : IQuorumChange { ACTOR static Future> getDesired(Reference self, Transaction* tr, std::vector oldCoordinators, - Reference ccf, + Reference ccr, CoordinatorsResult* err) { state int desiredCount = self->desired; @@ -1088,7 +1096,7 @@ struct AutoQuorumChange final : IQuorumChange { } if (checkAcceptable) { - bool ok = wait(isAcceptable(self.getPtr(), tr, oldCoordinators, ccf, desiredCount, &excluded)); + bool ok = wait(isAcceptable(self.getPtr(), tr, oldCoordinators, ccr, desiredCount, &excluded)); if (ok) return oldCoordinators; } @@ -2093,7 +2101,7 @@ ACTOR Future advanceVersion(Database cx, Version v) { } } -ACTOR Future forceRecovery(Reference clusterFile, Key dcId) { +ACTOR Future forceRecovery(Reference clusterFile, Key dcId) { state Reference>> clusterInterface(new AsyncVar>); state Future leaderMon = monitorLeader(clusterFile, clusterInterface); diff --git a/fdbclient/ManagementAPI.actor.h b/fdbclient/ManagementAPI.actor.h index 021dfde2bf..3b5dc5beda 100644 --- a/fdbclient/ManagementAPI.actor.h +++ b/fdbclient/ManagementAPI.actor.h @@ -130,7 +130,7 @@ struct IQuorumChange : ReferenceCounted { virtual ~IQuorumChange() {} virtual Future> getDesiredCoordinators(Transaction* tr, std::vector oldCoordinators, - Reference, + Reference, CoordinatorsResult&) = 0; virtual std::string getDesiredClusterKeyName() const { return std::string(); } }; @@ -211,7 +211,7 @@ ACTOR Future advanceVersion(Database cx, Version v); ACTOR Future setDDMode(Database cx, int mode); -ACTOR Future forceRecovery(Reference clusterFile, Standalone dcId); +ACTOR Future forceRecovery(Reference clusterFile, Standalone dcId); ACTOR Future printHealthyZone(Database cx); ACTOR Future setDDIgnoreRebalanceSwitch(Database cx, bool ignoreRebalance); diff --git a/fdbclient/MonitorLeader.actor.cpp b/fdbclient/MonitorLeader.actor.cpp index 7047935b0c..40274ba7c9 100644 --- a/fdbclient/MonitorLeader.actor.cpp +++ b/fdbclient/MonitorLeader.actor.cpp @@ -18,8 +18,10 @@ * limitations under the License. */ +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/MonitorLeader.h" #include "fdbclient/CoordinationInterface.h" +#include "fdbclient/NativeAPI.actor.h" #include "flow/ActorCollection.h" #include "flow/UnitTest.h" #include "fdbrpc/genericactors.actor.h" @@ -48,124 +50,23 @@ std::string trim(std::string const& connectionString) { } // namespace -std::pair ClusterConnectionFile::lookupClusterFileName(std::string const& filename) { - if (filename.length()) - return std::make_pair(filename, false); - - std::string f; - bool isDefaultFile = true; - if (platform::getEnvironmentVar(CLUSTER_FILE_ENV_VAR_NAME, f)) { - // If this is set but points to a file that does not - // exist, we will not fallback to any other methods - isDefaultFile = false; - } else if (fileExists("fdb.cluster")) - f = "fdb.cluster"; - else - f = platform::getDefaultClusterFilePath(); - - return std::make_pair(f, isDefaultFile); -} - -std::string ClusterConnectionFile::getErrorString(std::pair const& resolvedClusterFile, - Error const& e) { - bool isDefault = resolvedClusterFile.second; - if (e.code() == error_code_connection_string_invalid) { - return format("Invalid cluster file `%s': %d %s", resolvedClusterFile.first.c_str(), e.code(), e.what()); - } else if (e.code() == error_code_no_cluster_file_found) { - if (isDefault) - return format("Unable to read cluster file `./fdb.cluster' or `%s' and %s unset: %d %s", - platform::getDefaultClusterFilePath().c_str(), - CLUSTER_FILE_ENV_VAR_NAME, - e.code(), - e.what()); - else - return format( - "Unable to read cluster file `%s': %d %s", resolvedClusterFile.first.c_str(), e.code(), e.what()); - } else { - return format( - "Unexpected error loading cluster file `%s': %d %s", resolvedClusterFile.first.c_str(), e.code(), e.what()); - } -} - -ClusterConnectionFile::ClusterConnectionFile(std::string const& filename) { - if (!fileExists(filename)) { - throw no_cluster_file_found(); - } - - cs = ClusterConnectionString(readFileBytes(filename, MAX_CLUSTER_FILE_BYTES)); - this->filename = filename; - setConn = false; -} - -ClusterConnectionFile::ClusterConnectionFile(std::string const& filename, ClusterConnectionString const& contents) { - this->filename = filename; - cs = contents; - setConn = true; -} - -ClusterConnectionString const& ClusterConnectionFile::getConnectionString() const { - return cs; -} - -void ClusterConnectionFile::notifyConnected() { - if (setConn) { - this->writeFile(); - } -} - -bool ClusterConnectionFile::fileContentsUpToDate() const { +Future IClusterConnectionRecord::upToDate() { ClusterConnectionString temp; - return fileContentsUpToDate(temp); + return upToDate(temp); } -bool ClusterConnectionFile::fileContentsUpToDate(ClusterConnectionString& fileConnectionString) const { - try { - // the cluster file hasn't been created yet so there's nothing to check - if (setConn) - return true; - - ClusterConnectionFile temp(filename); - fileConnectionString = temp.getConnectionString(); - return fileConnectionString.toString() == cs.toString(); - } catch (Error& e) { - TraceEvent(SevWarnAlways, "ClusterFileError").error(e).detail("Filename", filename); - return false; // Swallow the error and report that the file is out of date +void IClusterConnectionRecord::notifyConnected() { + if (connectionStringNeedsPersisted) { + this->persist(); } } -bool ClusterConnectionFile::writeFile() { - setConn = false; - if (filename.size()) { - try { - atomicReplace(filename, - "# DO NOT EDIT!\n# This file is auto-generated, it is not to be edited by hand\n" + - cs.toString().append("\n")); - if (!fileContentsUpToDate()) { - // This should only happen in rare scenarios where multiple processes are updating the same file to - // different values simultaneously In that case, we don't have any guarantees about which file will - // ultimately be written - TraceEvent(SevWarnAlways, "ClusterFileChangedAfterReplace") - .detail("Filename", filename) - .detail("ConnStr", cs.toString()); - return false; - } - - return true; - } catch (Error& e) { - TraceEvent(SevWarnAlways, "UnableToChangeConnectionFile") - .error(e) - .detail("Filename", filename) - .detail("ConnStr", cs.toString()); - } - } - - return false; +bool IClusterConnectionRecord::needsToBePersisted() const { + return connectionStringNeedsPersisted; } -void ClusterConnectionFile::setConnectionString(ClusterConnectionString const& conn) { - ASSERT(filename.size()); - cs = conn; - writeFile(); +void IClusterConnectionRecord::setPersisted() { + connectionStringNeedsPersisted = false; } std::string ClusterConnectionString::getErrorString(std::string const& source, Error const& e) { @@ -367,8 +268,8 @@ std::string ClusterConnectionString::toString() const { return s; } -ClientCoordinators::ClientCoordinators(Reference ccf) : ccf(ccf) { - ClusterConnectionString cs = ccf->getConnectionString(); +ClientCoordinators::ClientCoordinators(Reference ccr) : ccr(ccr) { + ClusterConnectionString cs = ccr->getConnectionString(); for (auto s = cs.coordinators().begin(); s != cs.coordinators().end(); ++s) clientLeaderServers.push_back(ClientLeaderRegInterface(*s)); clusterKey = cs.clusterKey(); @@ -379,7 +280,7 @@ ClientCoordinators::ClientCoordinators(Key clusterKey, std::vector(ClusterConnectionString(coordinators, clusterKey)); + ccr = makeReference(ClusterConnectionString(coordinators, clusterKey)); } ClientLeaderRegInterface::ClientLeaderRegInterface(NetworkAddress remote) @@ -476,10 +377,10 @@ Optional> getLeader(const std::vector monitorLeaderOneGeneration(Reference connFile, +ACTOR Future monitorLeaderOneGeneration(Reference connRecord, Reference> outSerializedLeaderInfo, MonitorLeaderInfo info) { - state ClientCoordinators coordinators(info.intermediateConnFile); + state ClientCoordinators coordinators(info.intermediateConnRecord); state AsyncTrigger nomineeChange; state std::vector> nominees; state Future allActors; @@ -502,25 +403,26 @@ ACTOR Future monitorLeaderOneGeneration(ReferencegetConnectionString().toString()) + .detail("OldConnStr", info.intermediateConnRecord->getConnectionString().toString()) .trackLatest("MonitorLeaderForwarding"); - info.intermediateConnFile = makeReference( - connFile->getFilename(), ClusterConnectionString(leader.get().first.serializedInfo.toString())); + info.intermediateConnRecord = connRecord->makeIntermediateRecord( + ClusterConnectionString(leader.get().first.serializedInfo.toString())); return info; } - if (connFile != info.intermediateConnFile) { + if (connRecord != info.intermediateConnRecord) { if (!info.hasConnected) { TraceEvent(SevWarnAlways, "IncorrectClusterFileContentsAtConnection") - .detail("Filename", connFile->getFilename()) - .detail("ConnectionStringFromFile", connFile->getConnectionString().toString()) - .detail("CurrentConnectionString", info.intermediateConnFile->getConnectionString().toString()); + .detail("ClusterFile", connRecord->toString()) + .detail("StoredConnectionString", connRecord->getConnectionString().toString()) + .detail("CurrentConnectionString", + info.intermediateConnRecord->getConnectionString().toString()); } - connFile->setConnectionString(info.intermediateConnFile->getConnectionString()); - info.intermediateConnFile = connFile; + connRecord->setConnectionString(info.intermediateConnRecord->getConnectionString()); + info.intermediateConnRecord = connRecord; } info.hasConnected = true; - connFile->notifyConnected(); + connRecord->notifyConnected(); outSerializedLeaderInfo->set(leader.get().first.serializedInfo); } @@ -528,11 +430,11 @@ ACTOR Future monitorLeaderOneGeneration(Reference monitorLeaderInternal(Reference connFile, +ACTOR Future monitorLeaderInternal(Reference connRecord, Reference> outSerializedLeaderInfo) { - state MonitorLeaderInfo info(connFile); + state MonitorLeaderInfo info(connRecord); loop { - MonitorLeaderInfo _info = wait(monitorLeaderOneGeneration(connFile, outSerializedLeaderInfo, info)); + MonitorLeaderInfo _info = wait(monitorLeaderOneGeneration(connRecord, outSerializedLeaderInfo, info)); info = _info; } } @@ -750,13 +652,13 @@ void shrinkProxyList(ClientDBInfo& ni, } ACTOR Future monitorProxiesOneGeneration( - Reference connFile, + Reference connRecord, Reference> clientInfo, Reference>> coordinator, MonitorLeaderInfo info, Reference>>> supportedVersions, Key traceLogGroup) { - state ClusterConnectionString cs = info.intermediateConnFile->getConnectionString(); + state ClusterConnectionString cs = info.intermediateConnRecord->getConnectionString(); state std::vector addrs = cs.coordinators(); state int idx = 0; state int successIndex = 0; @@ -779,20 +681,24 @@ ACTOR Future monitorProxiesOneGeneration( req.supportedVersions = supportedVersions->get(); req.traceLogGroup = traceLogGroup; - ClusterConnectionString fileConnectionString; - if (connFile && !connFile->fileContentsUpToDate(fileConnectionString)) { - req.issues.push_back_deep(req.issues.arena(), LiteralStringRef("incorrect_cluster_file_contents")); - std::string connectionString = connFile->getConnectionString().toString(); - if (!incorrectTime.present()) { - incorrectTime = now(); - } - if (connFile->canGetFilename()) { - // Don't log a SevWarnAlways initially to account for transient issues (e.g. someone else changing the - // file right before us) + state ClusterConnectionString storedConnectionString; + if (connRecord) { + bool upToDate = wait(connRecord->upToDate(storedConnectionString)); + if (!upToDate) { + req.issues.push_back_deep(req.issues.arena(), LiteralStringRef("incorrect_cluster_file_contents")); + std::string connectionString = connRecord->getConnectionString().toString(); + if (!incorrectTime.present()) { + incorrectTime = now(); + } + + // Don't log a SevWarnAlways initially to account for transient issues (e.g. someone else changing + // the file right before us) TraceEvent(now() - incorrectTime.get() > 300 ? SevWarnAlways : SevWarn, "IncorrectClusterFileContents") - .detail("Filename", connFile->getFilename()) - .detail("ConnectionStringFromFile", fileConnectionString.toString()) + .detail("ClusterFile", connRecord->toString()) + .detail("StoredConnectionString", storedConnectionString.toString()) .detail("CurrentConnectionString", connectionString); + } else { + incorrectTime = Optional(); } } else { incorrectTime = Optional(); @@ -804,24 +710,25 @@ ACTOR Future monitorProxiesOneGeneration( if (rep.get().read().forward.present()) { TraceEvent("MonitorProxiesForwarding") .detail("NewConnStr", rep.get().read().forward.get().toString()) - .detail("OldConnStr", info.intermediateConnFile->getConnectionString().toString()); - info.intermediateConnFile = Reference(new ClusterConnectionFile( - connFile->getFilename(), ClusterConnectionString(rep.get().read().forward.get().toString()))); + .detail("OldConnStr", info.intermediateConnRecord->getConnectionString().toString()); + info.intermediateConnRecord = connRecord->makeIntermediateRecord( + ClusterConnectionString(rep.get().read().forward.get().toString())); return info; } - if (connFile != info.intermediateConnFile) { + if (connRecord != info.intermediateConnRecord) { if (!info.hasConnected) { TraceEvent(SevWarnAlways, "IncorrectClusterFileContentsAtConnection") - .detail("Filename", connFile->getFilename()) - .detail("ConnectionStringFromFile", connFile->getConnectionString().toString()) - .detail("CurrentConnectionString", info.intermediateConnFile->getConnectionString().toString()); + .detail("ClusterFile", connRecord->toString()) + .detail("StoredConnectionString", connRecord->getConnectionString().toString()) + .detail("CurrentConnectionString", + info.intermediateConnRecord->getConnectionString().toString()); } - connFile->setConnectionString(info.intermediateConnFile->getConnectionString()); - info.intermediateConnFile = connFile; + connRecord->setConnectionString(info.intermediateConnRecord->getConnectionString()); + info.intermediateConnRecord = connRecord; } info.hasConnected = true; - connFile->notifyConnected(); + connRecord->notifyConnected(); auto& ni = rep.get().mutate(); shrinkProxyList(ni, lastCommitProxyUIDs, lastCommitProxies, lastGrvProxyUIDs, lastGrvProxies); @@ -838,21 +745,21 @@ ACTOR Future monitorProxiesOneGeneration( } ACTOR Future monitorProxies( - Reference>> connFile, + Reference>> connRecord, Reference> clientInfo, Reference>> coordinator, Reference>>> supportedVersions, Key traceLogGroup) { - state MonitorLeaderInfo info(connFile->get()); + state MonitorLeaderInfo info(connRecord->get()); loop { choose { when(MonitorLeaderInfo _info = wait(monitorProxiesOneGeneration( - connFile->get(), clientInfo, coordinator, info, supportedVersions, traceLogGroup))) { + connRecord->get(), clientInfo, coordinator, info, supportedVersions, traceLogGroup))) { info = _info; } - when(wait(connFile->onChange())) { + when(wait(connRecord->onChange())) { info.hasConnected = false; - info.intermediateConnFile = connFile->get(); + info.intermediateConnRecord = connRecord->get(); } } } diff --git a/fdbclient/MonitorLeader.h b/fdbclient/MonitorLeader.h index b9c58de422..2903ff51b9 100644 --- a/fdbclient/MonitorLeader.h +++ b/fdbclient/MonitorLeader.h @@ -54,11 +54,11 @@ struct ClientData { struct MonitorLeaderInfo { bool hasConnected; - Reference intermediateConnFile; + Reference intermediateConnRecord; MonitorLeaderInfo() : hasConnected(false) {} - explicit MonitorLeaderInfo(Reference intermediateConnFile) - : hasConnected(false), intermediateConnFile(intermediateConnFile) {} + explicit MonitorLeaderInfo(Reference intermediateConnRecord) + : hasConnected(false), intermediateConnRecord(intermediateConnRecord) {} }; Optional> getLeader(const std::vector>& nominees); @@ -68,7 +68,7 @@ Optional> getLeader(const std::vector -Future monitorLeader(Reference const& connFile, +Future monitorLeader(Reference const& connFile, Reference>> const& outKnownLeader); // This is one place where the leader election algorithm is run. The coodinator contacts all coodinators to collect @@ -80,7 +80,7 @@ Future monitorLeaderAndGetClientInfo(Value const& key, Reference>> const& leaderInfo); Future monitorProxies( - Reference>> const& connFile, + Reference>> const& connRecord, Reference> const& clientInfo, Reference>> const& coordinator, Reference>>> const& supportedVersions, @@ -96,7 +96,7 @@ void shrinkProxyList(ClientDBInfo& ni, #pragma region Implementation #endif -Future monitorLeaderInternal(Reference const& connFile, +Future monitorLeaderInternal(Reference const& connRecord, Reference> const& outSerializedLeaderInfo); template @@ -119,11 +119,11 @@ struct LeaderDeserializer { }; template -Future monitorLeader(Reference const& connFile, +Future monitorLeader(Reference const& connRecord, Reference>> const& outKnownLeader) { LeaderDeserializer deserializer; auto serializedInfo = makeReference>(); - Future m = monitorLeaderInternal(connFile, serializedInfo); + Future m = monitorLeaderInternal(connRecord, serializedInfo); return m || deserializer(serializedInfo, outKnownLeader); } diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 1b72a2e0c0..c07675c180 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -36,6 +36,7 @@ #include "fdbclient/AnnotateActor.h" #include "fdbclient/Atomic.h" #include "fdbclient/ClusterInterface.h" +#include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/CoordinationInterface.h" #include "fdbclient/DatabaseContext.h" #include "fdbclient/GlobalConfig.actor.h" @@ -376,8 +377,9 @@ ACTOR Future databaseLogger(DatabaseContext* cx) { ev.detail("Elapsed", (lastLogged == 0) ? 0 : now() - lastLogged) .detail("Cluster", - cx->getConnectionFile() ? cx->getConnectionFile()->getConnectionString().clusterKeyName().toString() - : "") + cx->getConnectionRecord() + ? cx->getConnectionRecord()->getConnectionString().clusterKeyName().toString() + : "") .detail("Internal", cx->internal); cx->cc.logToTraceEvent(ev); @@ -1027,14 +1029,14 @@ void DatabaseContext::registerSpecialKeySpaceModule(SpecialKeySpace::MODULE modu specialKeySpaceModules.push_back(std::move(impl)); } -ACTOR Future getWorkerInterfaces(Reference clusterFile); +ACTOR Future getWorkerInterfaces(Reference clusterRecord); ACTOR Future> getJSON(Database db); struct WorkerInterfacesSpecialKeyImpl : SpecialKeyRangeReadImpl { Future getRange(ReadYourWritesTransaction* ryw, KeyRangeRef kr) const override { - if (ryw->getDatabase().getPtr() && ryw->getDatabase()->getConnectionFile()) { + if (ryw->getDatabase().getPtr() && ryw->getDatabase()->getConnectionRecord()) { Key prefix = Key(getKeyRange().begin); - return map(getWorkerInterfaces(ryw->getDatabase()->getConnectionFile()), + return map(getWorkerInterfaces(ryw->getDatabase()->getConnectionRecord()), [prefix = prefix, kr = KeyRange(kr)](const RangeResult& in) { RangeResult result; for (const auto& [k_, v] : in) { @@ -1166,7 +1168,7 @@ Future HealthMetricsRangeImpl::getRange(ReadYourWritesTransaction* return healthMetricsGetRangeActor(ryw, kr); } -DatabaseContext::DatabaseContext(Reference>> connectionFile, +DatabaseContext::DatabaseContext(Reference>> connectionRecord, Reference> clientInfo, Reference> const> coordinator, Future clientInfoMonitor, @@ -1177,7 +1179,7 @@ DatabaseContext::DatabaseContext(Reference(LiteralStringRef("\xff\xff/status/json"), [](ReadYourWritesTransaction* ryw) -> Future> { if (ryw->getDatabase().getPtr() && - ryw->getDatabase()->getConnectionFile()) { + ryw->getDatabase()->getConnectionRecord()) { ++ryw->getDatabase()->transactionStatusRequests; return getJSON(ryw->getDatabase()); } else { @@ -1397,8 +1399,9 @@ DatabaseContext::DatabaseContext(Reference Future> { try { - if (ryw->getDatabase().getPtr() && ryw->getDatabase()->getConnectionFile()) { - Optional output = StringRef(ryw->getDatabase()->getConnectionFile()->getFilename()); + if (ryw->getDatabase().getPtr() && ryw->getDatabase()->getConnectionRecord()) { + Optional output = + StringRef(ryw->getDatabase()->getConnectionRecord()->getLocation()); return output; } } catch (Error& e) { @@ -1414,8 +1417,8 @@ DatabaseContext::DatabaseContext(Reference Future> { try { - if (ryw->getDatabase().getPtr() && ryw->getDatabase()->getConnectionFile()) { - Reference f = ryw->getDatabase()->getConnectionFile(); + if (ryw->getDatabase().getPtr() && ryw->getDatabase()->getConnectionRecord()) { + Reference f = ryw->getDatabase()->getConnectionRecord(); Optional output = StringRef(f->getConnectionString().toString()); return output; } @@ -1475,7 +1478,7 @@ Database DatabaseContext::create(Reference> clientInfo, LockAware lockAware, int apiVersion, IsSwitchable switchable) { - return Database(new DatabaseContext(Reference>>(), + return Database(new DatabaseContext(Reference>>(), clientInfo, makeReference>>(), clientInfoMonitor, @@ -1699,11 +1702,12 @@ Future DatabaseContext::onConnected() { return connected; } -ACTOR static Future switchConnectionFileImpl(Reference connFile, DatabaseContext* self) { +ACTOR static Future switchConnectionRecordImpl(Reference connRecord, + DatabaseContext* self) { TEST(true); // Switch connection file - TraceEvent("SwitchConnectionFile") - .detail("ConnectionFile", connFile->canGetFilename() ? connFile->getFilename() : "") - .detail("ConnectionString", connFile->getConnectionString().toString()); + TraceEvent("SwitchConnectionRecord") + .detail("ClusterFile", connRecord->toString()) + .detail("ConnectionString", connRecord->getConnectionString().toString()); // Reset state from former cluster. self->commitProxies.clear(); @@ -1716,38 +1720,38 @@ ACTOR static Future switchConnectionFileImpl(ReferencerandomUniqueID(); self->clientInfo->set(clearedClientInfo); - self->connectionFile->set(connFile); + self->connectionRecord->set(connRecord); state Database db(Reference::addRef(self)); state Transaction tr(db); loop { tr.setOption(FDBTransactionOptions::READ_LOCK_AWARE); try { - TraceEvent("SwitchConnectionFileAttemptingGRV").log(); + TraceEvent("SwitchConnectionRecordAttemptingGRV").log(); Version v = wait(tr.getReadVersion()); - TraceEvent("SwitchConnectionFileGotRV") + TraceEvent("SwitchConnectionRecordGotRV") .detail("ReadVersion", v) .detail("MinAcceptableReadVersion", self->minAcceptableReadVersion); ASSERT(self->minAcceptableReadVersion != std::numeric_limits::max()); self->connectionFileChangedTrigger.trigger(); return Void(); } catch (Error& e) { - TraceEvent("SwitchConnectionFileError").detail("Error", e.what()); + TraceEvent("SwitchConnectionRecordError").detail("Error", e.what()); wait(tr.onError(e)); } } } -Reference DatabaseContext::getConnectionFile() { - if (connectionFile) { - return connectionFile->get(); +Reference DatabaseContext::getConnectionRecord() { + if (connectionRecord) { + return connectionRecord->get(); } - return Reference(); + return Reference(); } -Future DatabaseContext::switchConnectionFile(Reference standby) { +Future DatabaseContext::switchConnectionRecord(Reference standby) { ASSERT(switchable); - return switchConnectionFileImpl(standby, this); + return switchConnectionRecordImpl(standby, this); } Future DatabaseContext::connectionFileChanged() { @@ -1772,7 +1776,7 @@ extern IPAddress determinePublicIPAutomatically(ClusterConnectionString const& c // Creates a database object that represents a connection to a cluster // This constructor uses a preallocated DatabaseContext that may have been created // on another thread -Database Database::createDatabase(Reference connFile, +Database Database::createDatabase(Reference connRecord, int apiVersion, IsInternal internal, LocalityData const& clientLocality, @@ -1780,13 +1784,13 @@ Database Database::createDatabase(Reference connFile, if (!g_network) throw network_not_setup(); - if (connFile) { + if (connRecord) { if (networkOptions.traceDirectory.present() && !traceFileIsOpen()) { g_network->initMetrics(); FlowTransport::transport().initMetrics(); initTraceEventMetrics(); - auto publicIP = determinePublicIPAutomatically(connFile->getConnectionString()); + auto publicIP = determinePublicIPAutomatically(connRecord->getConnectionString()); selectTraceFormatter(networkOptions.traceFormat); selectTraceClockSource(networkOptions.traceClockSource); openTraceFile(NetworkAddress(publicIP, ::getpid()), @@ -1802,8 +1806,8 @@ Database Database::createDatabase(Reference connFile, .detail("SourceVersion", getSourceVersion()) .detail("Version", FDB_VT_VERSION) .detail("PackageName", FDB_VT_PACKAGE_NAME) - .detail("ClusterFile", connFile->getFilename().c_str()) - .detail("ConnectionString", connFile->getConnectionString().toString()) + .detail("ClusterFile", connRecord->toString()) + .detail("ConnectionString", connRecord->getConnectionString().toString()) .detailf("ActualTime", "%lld", DEBUG_DETERMINISM ? 0 : time(nullptr)) .detail("ApiVersion", apiVersion) .detailf("ImageOffset", "%p", platform::getImageOffset()) @@ -1820,9 +1824,9 @@ Database Database::createDatabase(Reference connFile, auto clientInfo = makeReference>(); auto coordinator = makeReference>>(); - auto connectionFile = makeReference>>(); - connectionFile->set(connFile); - Future clientInfoMonitor = monitorProxies(connectionFile, + auto connectionRecord = makeReference>>(); + connectionRecord->set(connRecord); + Future clientInfoMonitor = monitorProxies(connectionRecord, clientInfo, coordinator, networkOptions.supportedVersions, @@ -1830,7 +1834,7 @@ Database Database::createDatabase(Reference connFile, DatabaseContext* db; if (preallocatedDb) { - db = new (preallocatedDb) DatabaseContext(connectionFile, + db = new (preallocatedDb) DatabaseContext(connectionRecord, clientInfo, coordinator, clientInfoMonitor, @@ -1842,7 +1846,7 @@ Database Database::createDatabase(Reference connFile, apiVersion, IsSwitchable::True); } else { - db = new DatabaseContext(connectionFile, + db = new DatabaseContext(connectionRecord, clientInfo, coordinator, clientInfoMonitor, @@ -1867,9 +1871,9 @@ Database Database::createDatabase(std::string connFileName, int apiVersion, IsInternal internal, LocalityData const& clientLocality) { - Reference rccf = Reference( + Reference rccr = Reference( new ClusterConnectionFile(ClusterConnectionFile::lookupClusterFileName(connFileName).first)); - return Database::createDatabase(rccf, apiVersion, internal, clientLocality); + return Database::createDatabase(rccr, apiVersion, internal, clientLocality); } Reference DatabaseContext::getWatchMetadata(KeyRef key) const { @@ -2831,7 +2835,7 @@ ACTOR Future watchValue(Future version, TaskPriority::DefaultPromiseEndpoint))) { resp = r; } - when(wait(cx->connectionFile ? cx->connectionFile->onChange() : Never())) { wait(Never()); } + when(wait(cx->connectionRecord ? cx->connectionRecord->onChange() : Never())) { wait(Never()); } } if (info.debugID.present()) { g_traceBatch.addEvent("WatchValueDebug", @@ -6537,7 +6541,7 @@ ACTOR Future checkSafeExclusions(Database cx, std::vectorgetConnectionFile()); + state ClientCoordinators coordinatorList(cx->getConnectionRecord()); state std::vector>> leaderServers; leaderServers.reserve(coordinatorList.clientLeaderServers.size()); for (int i = 0; i < coordinatorList.clientLeaderServers.size(); i++) { @@ -6616,9 +6620,9 @@ ACTOR static Future rebootWorkerActor(DatabaseContext* cx, ValueRef add duration = 0; // fetch the addresses of all workers state std::map> address_interface; - if (!cx->getConnectionFile()) + if (!cx->getConnectionRecord()) return 0; - RangeResult kvs = wait(getWorkerInterfaces(cx->getConnectionFile())); + RangeResult kvs = wait(getWorkerInterfaces(cx->getConnectionRecord())); ASSERT(!kvs.more); // Note: reuse this knob from fdbcli, change it if necessary Reference connectLock(new FlowLock(CLIENT_KNOBS->CLI_CONNECT_PARALLELISM)); @@ -6640,7 +6644,7 @@ Future DatabaseContext::rebootWorker(StringRef addr, bool check, int du } Future DatabaseContext::forceRecoveryWithDataLoss(StringRef dcId) { - return forceRecovery(getConnectionFile(), dcId); + return forceRecovery(getConnectionRecord(), dcId); } ACTOR static Future createSnapshotActor(DatabaseContext* cx, UID snapUID, StringRef snapCmd) { diff --git a/fdbclient/NativeAPI.actor.h b/fdbclient/NativeAPI.actor.h index 6175205481..6bd5ab892e 100644 --- a/fdbclient/NativeAPI.actor.h +++ b/fdbclient/NativeAPI.actor.h @@ -83,7 +83,7 @@ public: // Creates a database object that represents a connection to a cluster // This constructor uses a preallocated DatabaseContext that may have been created // on another thread - static Database createDatabase(Reference connFile, + static Database createDatabase(Reference connRecord, int apiVersion, IsInternal internal = IsInternal::True, LocalityData const& clientLocality = LocalityData(), diff --git a/fdbclient/PaxosConfigTransaction.actor.cpp b/fdbclient/PaxosConfigTransaction.actor.cpp index 1b22cf0d5a..0c887ae67c 100644 --- a/fdbclient/PaxosConfigTransaction.actor.cpp +++ b/fdbclient/PaxosConfigTransaction.actor.cpp @@ -317,7 +317,7 @@ public: Future commit() { return commit(this); } PaxosConfigTransactionImpl(Database const& cx) : cx(cx) { - auto coordinators = cx->getConnectionFile()->getConnectionString().coordinators(); + auto coordinators = cx->getConnectionRecord()->getConnectionString().coordinators(); ctis.reserve(coordinators.size()); for (const auto& coordinator : coordinators) { ctis.emplace_back(coordinator); diff --git a/fdbclient/ReadYourWrites.actor.cpp b/fdbclient/ReadYourWrites.actor.cpp index dffb84db62..f156a36c85 100644 --- a/fdbclient/ReadYourWrites.actor.cpp +++ b/fdbclient/ReadYourWrites.actor.cpp @@ -1336,9 +1336,9 @@ ACTOR Future> getJSON(Database db) { return getValueFromJSON(statusObj); } -ACTOR Future getWorkerInterfaces(Reference clusterFile) { +ACTOR Future getWorkerInterfaces(Reference connRecord) { state Reference>> clusterInterface(new AsyncVar>); - state Future leaderMon = monitorLeader(clusterFile, clusterInterface); + state Future leaderMon = monitorLeader(connRecord, clusterInterface); loop { choose { @@ -1371,7 +1371,7 @@ Future> ReadYourWritesTransaction::get(const Key& key, Snapshot } } else { if (key == LiteralStringRef("\xff\xff/status/json")) { - if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionFile()) { + if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionRecord()) { ++tr.getDatabase()->transactionStatusRequests; return getJSON(tr.getDatabase()); } else { @@ -1381,8 +1381,8 @@ Future> ReadYourWritesTransaction::get(const Key& key, Snapshot if (key == LiteralStringRef("\xff\xff/cluster_file_path")) { try { - if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionFile()) { - Optional output = StringRef(tr.getDatabase()->getConnectionFile()->getFilename()); + if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionRecord()) { + Optional output = StringRef(tr.getDatabase()->getConnectionRecord()->getLocation()); return output; } } catch (Error& e) { @@ -1393,8 +1393,8 @@ Future> ReadYourWritesTransaction::get(const Key& key, Snapshot if (key == LiteralStringRef("\xff\xff/connection_string")) { try { - if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionFile()) { - Reference f = tr.getDatabase()->getConnectionFile(); + if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionRecord()) { + Reference f = tr.getDatabase()->getConnectionRecord(); Optional output = StringRef(f->getConnectionString().toString()); return output; } @@ -1454,8 +1454,8 @@ Future ReadYourWritesTransaction::getRange(KeySelector begin, } } else { if (begin.getKey() == LiteralStringRef("\xff\xff/worker_interfaces")) { - if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionFile()) { - return getWorkerInterfaces(tr.getDatabase()->getConnectionFile()); + if (tr.getDatabase().getPtr() && tr.getDatabase()->getConnectionRecord()) { + return getWorkerInterfaces(tr.getDatabase()->getConnectionRecord()); } else { return RangeResult(); } diff --git a/fdbclient/SimpleConfigTransaction.actor.cpp b/fdbclient/SimpleConfigTransaction.actor.cpp index 77f72fb9f2..f1bc1aebb9 100644 --- a/fdbclient/SimpleConfigTransaction.actor.cpp +++ b/fdbclient/SimpleConfigTransaction.actor.cpp @@ -126,7 +126,7 @@ class SimpleConfigTransactionImpl { public: SimpleConfigTransactionImpl(Database const& cx) : cx(cx) { - auto coordinators = cx->getConnectionFile()->getConnectionString().coordinators(); + auto coordinators = cx->getConnectionRecord()->getConnectionString().coordinators(); std::sort(coordinators.begin(), coordinators.end()); cti = ConfigTransactionInterface(coordinators[0]); } diff --git a/fdbclient/SpecialKeySpace.actor.cpp b/fdbclient/SpecialKeySpace.actor.cpp index d2fcbf96ec..bea850dea5 100644 --- a/fdbclient/SpecialKeySpace.actor.cpp +++ b/fdbclient/SpecialKeySpace.actor.cpp @@ -27,6 +27,7 @@ #include #include "fdbclient/ActorLineageProfiler.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/Knobs.h" #include "fdbclient/ProcessInterface.h" #include "fdbclient/GlobalConfig.actor.h" @@ -1590,8 +1591,7 @@ CoordinatorsImpl::CoordinatorsImpl(KeyRangeRef kr) : SpecialKeyRangeRWImpl(kr) { Future CoordinatorsImpl::getRange(ReadYourWritesTransaction* ryw, KeyRangeRef kr) const { RangeResult result; KeyRef prefix(getKeyRange().begin); - // the constructor of ClusterConnectionFile already checks whether the file is valid - auto cs = ClusterConnectionFile(ryw->getDatabase()->getConnectionFile()->getFilename()).getConnectionString(); + auto cs = ryw->getDatabase()->getConnectionRecord()->getConnectionString(); auto coordinator_processes = cs.coordinators(); Key cluster_decription_key = prefix.withSuffix(LiteralStringRef("cluster_description")); if (kr.contains(cluster_decription_key)) { @@ -1737,7 +1737,10 @@ ACTOR static Future CoordinatorsAutoImplActor(ReadYourWritesTransac state CoordinatorsResult result = CoordinatorsResult::SUCCESS; std::vector _desiredCoordinators = wait(autoQuorumChange()->getDesiredCoordinators( - &tr, old.coordinators(), Reference(new ClusterConnectionFile(old)), result)); + &tr, + old.coordinators(), + Reference(new ClusterConnectionMemoryRecord(old)), + result)); if (result == CoordinatorsResult::NOT_ENOUGH_MACHINES) { // we could get not_enough_machines if we happen to see the database while the cluster controller is updating diff --git a/fdbclient/StatusClient.actor.cpp b/fdbclient/StatusClient.actor.cpp index 01f8bedf0b..2829771ba8 100644 --- a/fdbclient/StatusClient.actor.cpp +++ b/fdbclient/StatusClient.actor.cpp @@ -302,11 +302,11 @@ void JSONDoc::mergeValueInto(json_spirit::mValue& dst, const json_spirit::mValue // Check if a quorum of coordination servers is reachable // Will not throw, will just return non-present Optional if error -ACTOR Future> clientCoordinatorsStatusFetcher(Reference f, +ACTOR Future> clientCoordinatorsStatusFetcher(Reference connRecord, bool* quorum_reachable, int* coordinatorsFaultTolerance) { try { - state ClientCoordinators coord(f); + state ClientCoordinators coord(connRecord); state StatusObject statusObj; state std::vector>> leaderServers; @@ -365,14 +365,16 @@ ACTOR Future> clientCoordinatorsStatusFetcher(Reference clientStatusFetcher(Reference f, +ACTOR Future clientStatusFetcher(Reference connRecord, StatusArray* messages, bool* quorum_reachable, int* coordinatorsFaultTolerance) { state StatusObject statusObj; - Optional coordsStatusObj = - wait(clientCoordinatorsStatusFetcher(f, quorum_reachable, coordinatorsFaultTolerance)); + state Optional coordsStatusObj = + wait(clientCoordinatorsStatusFetcher(connRecord, quorum_reachable, coordinatorsFaultTolerance)); + state bool contentsUpToDate = wait(connRecord->upToDate()); + if (coordsStatusObj.present()) { statusObj["coordinators"] = coordsStatusObj.get(); if (!*quorum_reachable) @@ -381,17 +383,17 @@ ACTOR Future clientStatusFetcher(Reference messages->push_back(makeMessage("status_incomplete_coordinators", "Could not fetch coordinator info.")); StatusObject statusObjClusterFile; - statusObjClusterFile["path"] = f->getFilename(); - bool contentsUpToDate = f->fileContentsUpToDate(); + statusObjClusterFile["path"] = connRecord->getLocation(); statusObjClusterFile["up_to_date"] = contentsUpToDate; statusObj["cluster_file"] = statusObjClusterFile; if (!contentsUpToDate) { + ClusterConnectionString storedConnectionString = wait(connRecord->getStoredConnectionString()); std::string description = "Cluster file contents do not match current cluster connection string."; description += "\nThe file contains the connection string: "; - description += ClusterConnectionFile(f->getFilename()).getConnectionString().toString().c_str(); + description += storedConnectionString.toString().c_str(); description += "\nThe current connection string is: "; - description += f->getConnectionString().toString().c_str(); + description += connRecord->getConnectionString().toString().c_str(); description += "\nVerify the cluster file and its parent directory are writable and that the cluster file has " "not been overwritten externally. To change coordinators without manual intervention, the " "cluster file and its containing folder must be writable by all servers and clients. If a " @@ -491,7 +493,7 @@ StatusObject getClientDatabaseStatus(StatusObjectReader client, StatusObjectRead return databaseStatus; } -ACTOR Future statusFetcherImpl(Reference f, +ACTOR Future statusFetcherImpl(Reference connRecord, Reference>> clusterInterface) { if (!g_network) throw network_not_setup(); @@ -508,7 +510,7 @@ ACTOR Future statusFetcherImpl(Reference f, state int64_t clientTime = g_network->timer(); StatusObject _statusObjClient = - wait(clientStatusFetcher(f, &clientMessages, &quorum_reachable, &coordinatorsFaultTolerance)); + wait(clientStatusFetcher(connRecord, &clientMessages, &quorum_reachable, &coordinatorsFaultTolerance)); statusObjClient = _statusObjClient; if (clientTime != -1) @@ -598,7 +600,7 @@ ACTOR Future statusFetcherImpl(Reference f, } ACTOR Future timeoutMonitorLeader(Database db) { - state Future leadMon = monitorLeader(db->getConnectionFile(), db->statusClusterInterface); + state Future leadMon = monitorLeader(db->getConnectionRecord(), db->statusClusterInterface); loop { wait(delay(CLIENT_KNOBS->STATUS_IDLE_TIMEOUT + 0.00001 + db->lastStatusFetch - now())); if (now() - db->lastStatusFetch > CLIENT_KNOBS->STATUS_IDLE_TIMEOUT) { @@ -615,5 +617,5 @@ Future StatusClient::statusFetcher(Database db) { db->statusLeaderMon = timeoutMonitorLeader(db); } - return statusFetcherImpl(db->getConnectionFile(), db->statusClusterInterface); + return statusFetcherImpl(db->getConnectionRecord(), db->statusClusterInterface); } diff --git a/fdbclient/ThreadSafeTransaction.cpp b/fdbclient/ThreadSafeTransaction.cpp index ee7e0b75e2..3810d08191 100644 --- a/fdbclient/ThreadSafeTransaction.cpp +++ b/fdbclient/ThreadSafeTransaction.cpp @@ -18,6 +18,7 @@ * limitations under the License. */ +#include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/ThreadSafeTransaction.h" #include "fdbclient/DatabaseContext.h" #include "fdbclient/versions.h" diff --git a/fdbserver/ClusterController.actor.cpp b/fdbserver/ClusterController.actor.cpp index 55831de579..cada429dee 100644 --- a/fdbserver/ClusterController.actor.cpp +++ b/fdbserver/ClusterController.actor.cpp @@ -27,6 +27,7 @@ #include "fdbrpc/FailureMonitor.h" #include "flow/ActorCollection.h" #include "flow/SystemMonitor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/NativeAPI.actor.h" #include "fdbserver/BackupInterface.h" #include "fdbserver/CoordinationInterface.h" @@ -5299,7 +5300,7 @@ ACTOR Future clusterController(ServerCoordinators coordinators, } } -ACTOR Future clusterController(Reference connFile, +ACTOR Future clusterController(Reference connRecord, Reference>> currentCC, Reference> asyncPriorityInfo, Future recoveredDiskFiles, @@ -5309,7 +5310,7 @@ ACTOR Future clusterController(Reference connFile, state bool hasConnected = false; loop { try { - ServerCoordinators coordinators(connFile); + ServerCoordinators coordinators(connRecord); wait(clusterController(coordinators, currentCC, hasConnected, asyncPriorityInfo, locality, configDBType)); } catch (Error& e) { if (e.code() != error_code_coordinators_changed) @@ -5328,7 +5329,8 @@ TEST_CASE("/fdbserver/clustercontroller/updateWorkerHealth") { // Create a testing ClusterControllerData. Most of the internal states do not matter in this test. state ClusterControllerData data(ClusterControllerFullInterface(), LocalityData(), - ServerCoordinators(Reference(new ClusterConnectionFile()))); + ServerCoordinators(Reference( + new ClusterConnectionMemoryRecord(ClusterConnectionString())))); state NetworkAddress workerAddress(IPAddress(0x01010101), 1); state NetworkAddress badPeer1(IPAddress(0x02020202), 1); state NetworkAddress badPeer2(IPAddress(0x03030303), 1); @@ -5386,7 +5388,8 @@ TEST_CASE("/fdbserver/clustercontroller/updateRecoveredWorkers") { // Create a testing ClusterControllerData. Most of the internal states do not matter in this test. ClusterControllerData data(ClusterControllerFullInterface(), LocalityData(), - ServerCoordinators(Reference(new ClusterConnectionFile()))); + ServerCoordinators(Reference( + new ClusterConnectionMemoryRecord(ClusterConnectionString())))); NetworkAddress worker1(IPAddress(0x01010101), 1); NetworkAddress worker2(IPAddress(0x11111111), 1); NetworkAddress badPeer1(IPAddress(0x02020202), 1); @@ -5422,7 +5425,8 @@ TEST_CASE("/fdbserver/clustercontroller/getServersWithDegradedLink") { // Create a testing ClusterControllerData. Most of the internal states do not matter in this test. ClusterControllerData data(ClusterControllerFullInterface(), LocalityData(), - ServerCoordinators(Reference(new ClusterConnectionFile()))); + ServerCoordinators(Reference( + new ClusterConnectionMemoryRecord(ClusterConnectionString())))); NetworkAddress worker(IPAddress(0x01010101), 1); NetworkAddress badPeer1(IPAddress(0x02020202), 1); NetworkAddress badPeer2(IPAddress(0x03030303), 1); @@ -5523,7 +5527,8 @@ TEST_CASE("/fdbserver/clustercontroller/recentRecoveryCountDueToHealth") { // Create a testing ClusterControllerData. Most of the internal states do not matter in this test. ClusterControllerData data(ClusterControllerFullInterface(), LocalityData(), - ServerCoordinators(Reference(new ClusterConnectionFile()))); + ServerCoordinators(Reference( + new ClusterConnectionMemoryRecord(ClusterConnectionString())))); ASSERT_EQ(data.recentRecoveryCountDueToHealth(), 0); @@ -5543,7 +5548,8 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerRecoveryDueToDegradedServer // Create a testing ClusterControllerData. Most of the internal states do not matter in this test. ClusterControllerData data(ClusterControllerFullInterface(), LocalityData(), - ServerCoordinators(Reference(new ClusterConnectionFile()))); + ServerCoordinators(Reference( + new ClusterConnectionMemoryRecord(ClusterConnectionString())))); NetworkAddress master(IPAddress(0x01010101), 1); NetworkAddress tlog(IPAddress(0x02020202), 1); NetworkAddress satelliteTlog(IPAddress(0x03030303), 1); @@ -5647,7 +5653,8 @@ TEST_CASE("/fdbserver/clustercontroller/shouldTriggerFailoverDueToDegradedServer // Create a testing ClusterControllerData. Most of the internal states do not matter in this test. ClusterControllerData data(ClusterControllerFullInterface(), LocalityData(), - ServerCoordinators(Reference(new ClusterConnectionFile()))); + ServerCoordinators(Reference( + new ClusterConnectionMemoryRecord(ClusterConnectionString())))); NetworkAddress master(IPAddress(0x01010101), 1); NetworkAddress tlog(IPAddress(0x02020202), 1); NetworkAddress satelliteTlog(IPAddress(0x03030303), 1); diff --git a/fdbserver/CoordinatedState.actor.cpp b/fdbserver/CoordinatedState.actor.cpp index 02a3529132..4ffdd057a2 100644 --- a/fdbserver/CoordinatedState.actor.cpp +++ b/fdbserver/CoordinatedState.actor.cpp @@ -18,6 +18,7 @@ * limitations under the License. */ +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbserver/CoordinatedState.h" #include "fdbserver/CoordinationInterface.h" #include "fdbserver/Knobs.h" @@ -288,8 +289,7 @@ struct MovableCoordinatedStateImpl { // reached the point where a leader elected by the new coordinators should be doing the rest of the work // (and therefore the caller should die). state CoordinatedState cs(self->coordinators); - state CoordinatedState nccs( - ServerCoordinators(Reference(new ClusterConnectionFile(nc)))); + state CoordinatedState nccs(ServerCoordinators(makeReference(nc))); state Future creationTimeout = delay(30); ASSERT(self->lastValue.present() && self->lastCSValue.present()); TraceEvent("StartMove").detail("ConnectionString", nc.toString()); @@ -306,7 +306,7 @@ struct MovableCoordinatedStateImpl { when(wait(nccs.setExclusive( BinaryWriter::toValue(MovableValue(self->lastValue.get(), MovableValue::MovingFrom, - self->coordinators.ccf->getConnectionString().toString()), + self->coordinators.ccr->getConnectionString().toString()), IncludeVersion(ProtocolVersion::withMovableCoordinatedStateV2()))))) {} } diff --git a/fdbserver/Coordination.actor.cpp b/fdbserver/Coordination.actor.cpp index 9d5455b477..a4fe8b4b1a 100644 --- a/fdbserver/Coordination.actor.cpp +++ b/fdbserver/Coordination.actor.cpp @@ -95,8 +95,8 @@ LeaderElectionRegInterface::LeaderElectionRegInterface(INetwork* local) : Client forward.makeWellKnownEndpoint(WLTOKEN_LEADERELECTIONREG_FORWARD, TaskPriority::Coordination); } -ServerCoordinators::ServerCoordinators(Reference cf) : ClientCoordinators(cf) { - ClusterConnectionString cs = ccf->getConnectionString(); +ServerCoordinators::ServerCoordinators(Reference ccr) : ClientCoordinators(ccr) { + ClusterConnectionString cs = ccr->getConnectionString(); for (auto s = cs.coordinators().begin(); s != cs.coordinators().end(); ++s) { leaderElectionServers.emplace_back(*s); stateServers.emplace_back(*s); @@ -588,7 +588,7 @@ StringRef getClusterDescriptor(Key key) { ACTOR Future leaderServer(LeaderElectionRegInterface interf, OnDemandStore* pStore, UID id, - Reference ccf) { + Reference ccr) { state LeaderRegisterCollection regs(pStore); state ActorCollection forwarders(false); @@ -609,12 +609,12 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, info.forward = forward.get().serializedInfo; req.reply.send(CachedSerialization(info)); } else { - StringRef clusterName = ccf->getConnectionString().clusterKeyName(); + StringRef clusterName = ccr->getConnectionString().clusterKeyName(); if (!SERVER_KNOBS->ENABLE_CROSS_CLUSTER_SUPPORT && getClusterDescriptor(req.clusterKey).compare(clusterName)) { - TraceEvent(SevWarn, "CCFMismatch") + TraceEvent(SevWarn, "CCRMismatch") .detail("RequestType", "OpenDatabaseCoordRequest") - .detail("LocalCS", ccf->getConnectionString().toString()) + .detail("LocalCS", ccr->getConnectionString().toString()) .detail("IncomingClusterKey", req.clusterKey) .detail("IncomingCoordinators", describeList(req.coordinators, req.coordinators.size())); req.reply.sendError(wrong_connection_file()); @@ -628,13 +628,13 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, if (forward.present()) { req.reply.send(forward.get()); } else { - StringRef clusterName = ccf->getConnectionString().clusterKeyName(); + StringRef clusterName = ccr->getConnectionString().clusterKeyName(); if (!SERVER_KNOBS->ENABLE_CROSS_CLUSTER_SUPPORT && getClusterDescriptor(req.key).compare(clusterName)) { - TraceEvent(SevWarn, "CCFMismatch") + TraceEvent(SevWarn, "CCRMismatch") .detail("RequestType", "ElectionResultRequest") - .detail("LocalCS", ccf->getConnectionString().toString()) + .detail("LocalCS", ccr->getConnectionString().toString()) .detail("IncomingClusterKey", req.key) - .detail("ClusterKey", ccf->getConnectionString().clusterKey()) + .detail("ClusterKey", ccr->getConnectionString().clusterKey()) .detail("IncomingCoordinators", describeList(req.coordinators, req.coordinators.size())); req.reply.sendError(wrong_connection_file()); } else { @@ -647,13 +647,13 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, if (forward.present()) req.reply.send(forward.get()); else { - StringRef clusterName = ccf->getConnectionString().clusterKeyName(); + StringRef clusterName = ccr->getConnectionString().clusterKeyName(); if (!SERVER_KNOBS->ENABLE_CROSS_CLUSTER_SUPPORT && getClusterDescriptor(req.key).compare(clusterName)) { - TraceEvent(SevWarn, "CCFMismatch") + TraceEvent(SevWarn, "CCRMismatch") .detail("RequestType", "GetLeaderRequest") - .detail("LocalCS", ccf->getConnectionString().toString()) + .detail("LocalCS", ccr->getConnectionString().toString()) .detail("IncomingClusterKey", req.key) - .detail("ClusterKey", ccf->getConnectionString().clusterKey()); + .detail("ClusterKey", ccr->getConnectionString().clusterKey()); req.reply.sendError(wrong_connection_file()); } else { regs.getInterface(req.key, id).getLeader.send(req); @@ -665,11 +665,11 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, if (forward.present()) req.reply.send(forward.get()); else { - StringRef clusterName = ccf->getConnectionString().clusterKeyName(); + StringRef clusterName = ccr->getConnectionString().clusterKeyName(); if (!SERVER_KNOBS->ENABLE_CROSS_CLUSTER_SUPPORT && getClusterDescriptor(req.key).compare(clusterName)) { - TraceEvent(SevWarn, "CCFMismatch") + TraceEvent(SevWarn, "CCRMismatch") .detail("RequestType", "CandidacyRequest") - .detail("LocalCS", ccf->getConnectionString().toString()) + .detail("LocalCS", ccr->getConnectionString().toString()) .detail("IncomingClusterKey", req.key); req.reply.sendError(wrong_connection_file()); } else { @@ -682,11 +682,11 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, if (forward.present()) req.reply.send(LeaderHeartbeatReply{ false }); else { - StringRef clusterName = ccf->getConnectionString().clusterKeyName(); + StringRef clusterName = ccr->getConnectionString().clusterKeyName(); if (!SERVER_KNOBS->ENABLE_CROSS_CLUSTER_SUPPORT && getClusterDescriptor(req.key).compare(clusterName)) { - TraceEvent(SevWarn, "CCFMismatch") + TraceEvent(SevWarn, "CCRMismatch") .detail("RequestType", "LeaderHeartbeatRequest") - .detail("LocalCS", ccf->getConnectionString().toString()) + .detail("LocalCS", ccr->getConnectionString().toString()) .detail("IncomingClusterKey", req.key); req.reply.sendError(wrong_connection_file()); } else { @@ -699,11 +699,11 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, if (forward.present()) req.reply.send(Void()); else { - StringRef clusterName = ccf->getConnectionString().clusterKeyName(); + StringRef clusterName = ccr->getConnectionString().clusterKeyName(); if (!SERVER_KNOBS->ENABLE_CROSS_CLUSTER_SUPPORT && getClusterDescriptor(req.key).compare(clusterName)) { - TraceEvent(SevWarn, "CCFMismatch") + TraceEvent(SevWarn, "CCRMismatch") .detail("RequestType", "ForwardRequest") - .detail("LocalCS", ccf->getConnectionString().toString()) + .detail("LocalCS", ccr->getConnectionString().toString()) .detail("IncomingClusterKey", req.key); req.reply.sendError(wrong_connection_file()); } else { @@ -721,7 +721,7 @@ ACTOR Future leaderServer(LeaderElectionRegInterface interf, } ACTOR Future coordinationServer(std::string dataFolder, - Reference ccf, + Reference ccr, ConfigDBType configDBType) { state UID myID = deterministicRandom()->randomUniqueID(); state LeaderElectionRegInterface myLeaderInterface(g_network); @@ -744,7 +744,7 @@ ACTOR Future coordinationServer(std::string dataFolder, } try { - wait(localGenerationReg(myInterface, &store) || leaderServer(myLeaderInterface, &store, myID, ccf) || + wait(localGenerationReg(myInterface, &store) || leaderServer(myLeaderInterface, &store, myID, ccr) || store.getError() || configDatabaseServer); throw internal_error(); } catch (Error& e) { diff --git a/fdbserver/CoordinationInterface.h b/fdbserver/CoordinationInterface.h index 3631495dd6..f6238cc208 100644 --- a/fdbserver/CoordinationInterface.h +++ b/fdbserver/CoordinationInterface.h @@ -214,7 +214,7 @@ struct ForwardRequest { class ServerCoordinators : public ClientCoordinators { public: - explicit ServerCoordinators(Reference); + explicit ServerCoordinators(Reference); std::vector leaderElectionServers; std::vector stateServers; @@ -222,7 +222,7 @@ public: }; Future coordinationServer(std::string const& dataFolder, - Reference const& ccf, + Reference const& ccf, ConfigDBType const&); #endif diff --git a/fdbserver/LeaderElection.actor.cpp b/fdbserver/LeaderElection.actor.cpp index 40136fd45b..94476569ee 100644 --- a/fdbserver/LeaderElection.actor.cpp +++ b/fdbserver/LeaderElection.actor.cpp @@ -137,21 +137,21 @@ ACTOR Future tryBecomeLeaderInternal(ServerCoordinators coordinators, if (!hasConnected) { TraceEvent(SevWarnAlways, "IncorrectClusterFileContentsAtConnection") - .detail("Filename", coordinators.ccf->getFilename()) - .detail("ConnectionStringFromFile", coordinators.ccf->getConnectionString().toString()) + .detail("ClusterFile", coordinators.ccr->toString()) + .detail("StoredConnectionString", coordinators.ccr->getConnectionString().toString()) .detail("CurrentConnectionString", leader.get().first.serializedInfo.toString()); } - coordinators.ccf->setConnectionString( + coordinators.ccr->setConnectionString( ClusterConnectionString(leader.get().first.serializedInfo.toString())); TraceEvent("LeaderForwarding") - .detail("ConnStr", coordinators.ccf->getConnectionString().toString()) + .detail("ConnStr", coordinators.ccr->getConnectionString().toString()) .trackLatest("LeaderForwarding"); throw coordinators_changed(); } if (leader.present() && leader.get().second) { hasConnected = true; - coordinators.ccf->notifyConnected(); + coordinators.ccr->notifyConnected(); } if (leader.present() && leader.get().second && leader.get().first.equalInternalId(myInfo)) { diff --git a/fdbserver/RestoreWorker.actor.cpp b/fdbserver/RestoreWorker.actor.cpp index 43224fe318..7bf4010999 100644 --- a/fdbserver/RestoreWorker.actor.cpp +++ b/fdbserver/RestoreWorker.actor.cpp @@ -406,11 +406,11 @@ ACTOR Future _restoreWorker(Database cx, LocalityData locality) { return Void(); } -ACTOR Future restoreWorker(Reference connFile, +ACTOR Future restoreWorker(Reference connRecord, LocalityData locality, std::string coordFolder) { try { - Database cx = Database::createDatabase(connFile, Database::API_VERSION_LATEST, IsInternal::True, locality); + Database cx = Database::createDatabase(connRecord, Database::API_VERSION_LATEST, IsInternal::True, locality); wait(reportErrors(_restoreWorker(cx, locality), "RestoreWorker")); } catch (Error& e) { TraceEvent("FastRestoreWorker").detail("Error", e.what()); diff --git a/fdbserver/RestoreWorkerInterface.actor.h b/fdbserver/RestoreWorkerInterface.actor.h index d2d681a0e4..02b84af808 100644 --- a/fdbserver/RestoreWorkerInterface.actor.h +++ b/fdbserver/RestoreWorkerInterface.actor.h @@ -711,7 +711,9 @@ std::string getRoleStr(RestoreRole role); ////--- Interface functions ACTOR Future _restoreWorker(Database cx, LocalityData locality); -ACTOR Future restoreWorker(Reference ccf, LocalityData locality, std::string coordFolder); +ACTOR Future restoreWorker(Reference ccr, + LocalityData locality, + std::string coordFolder); extern const KeyRef restoreLeaderKey; extern const KeyRangeRef restoreWorkersKeys; diff --git a/fdbserver/SimulatedCluster.actor.cpp b/fdbserver/SimulatedCluster.actor.cpp index 47c356ed86..6381140aa5 100644 --- a/fdbserver/SimulatedCluster.actor.cpp +++ b/fdbserver/SimulatedCluster.actor.cpp @@ -26,6 +26,8 @@ #include #include "fdbrpc/Locality.h" #include "fdbrpc/simulator.h" +#include "fdbclient/ClusterConnectionFile.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/DatabaseContext.h" #include "fdbserver/TesterInterface.actor.h" #include "fdbserver/WorkerInterface.actor.h" @@ -387,7 +389,7 @@ T simulate(const T& in) { return out; } -ACTOR Future runBackup(Reference connFile) { +ACTOR Future runBackup(Reference connRecord) { state std::vector> agentFutures; while (g_simulator.backupAgents == ISimulator::BackupAgentType::WaitForType) { @@ -395,7 +397,7 @@ ACTOR Future runBackup(Reference connFile) { } if (g_simulator.backupAgents == ISimulator::BackupAgentType::BackupToFile) { - Database cx = Database::createDatabase(connFile, -1); + Database cx = Database::createDatabase(connRecord, -1); state FileBackupAgent fileAgent; agentFutures.push_back(fileAgent.run( @@ -414,7 +416,7 @@ ACTOR Future runBackup(Reference connFile) { throw internal_error(); } -ACTOR Future runDr(Reference connFile) { +ACTOR Future runDr(Reference connRecord) { state std::vector> agentFutures; while (g_simulator.drAgents == ISimulator::BackupAgentType::WaitForType) { @@ -422,13 +424,13 @@ ACTOR Future runDr(Reference connFile) { } if (g_simulator.drAgents == ISimulator::BackupAgentType::BackupToDB) { - Database cx = Database::createDatabase(connFile, -1); + Database cx = Database::createDatabase(connRecord, -1); - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); state Database extraDB = Database::createDatabase(extraFile, -1); TraceEvent("StartingDrAgents") - .detail("ConnFile", connFile->getConnectionString().toString()) + .detail("ConnectionString", connRecord->getConnectionString().toString()) .detail("ExtraString", extraFile->getConnectionString().toString()); state DatabaseBackupAgent dbAgent = DatabaseBackupAgent(cx); @@ -459,7 +461,7 @@ enum AgentMode { AgentNone = 0, AgentOnly = 1, AgentAddition = 2 }; // SOMEDAY: when a process can be rebooted in isolation from the other on that machine, // a loop{} will be needed around the waiting on simulatedFDBD(). For now this simply // takes care of house-keeping such as context switching and file closing. -ACTOR Future simulatedFDBDRebooter(Reference connFile, +ACTOR Future simulatedFDBDRebooter(Reference connRecord, IPAddress ip, bool sslEnabled, uint16_t port, @@ -525,7 +527,7 @@ ACTOR Future simulatedFDBDRebooter(ReferencegetConnectionString().toString() : "") + .detail("ConnectionString", connRecord ? connRecord->getConnectionString().toString() : "") .detailf("ActualTime", "%lld", DEBUG_DETERMINISM ? 0 : time(nullptr)) .detail("CommandLine", "fdbserver -r simulation") .detail("BuggifyEnabled", isBuggifyEnabled(BuggifyType::General)) @@ -546,7 +548,7 @@ ACTOR Future simulatedFDBDRebooter(Reference simulatedFDBDRebooter(Reference simulatedFDBDRebooter(Reference(joinPath(*dataFolder, "fdb.cluster")); + connRecord = makeReference(joinPath(*dataFolder, "fdb.cluster")); } else { - connFile = + connRecord = makeReference(joinPath(*dataFolder, "fdb.cluster"), connStr.toString()); } } else { @@ -747,9 +749,9 @@ ACTOR Future simulatedMachine(ClusterConnectionString connStr, state std::vector> processes; for (int i = 0; i < ips.size(); i++) { std::string path = joinPath(myFolders[i], "fdb.cluster"); - Reference clusterFile(useSeedFile - ? new ClusterConnectionFile(path, connStr.toString()) - : new ClusterConnectionFile(path)); + Reference clusterFile( + useSeedFile ? new ClusterConnectionFile(path, connStr.toString()) + : new ClusterConnectionFile(path)); const int listenPort = i * listenPerProcess + 1; AgentMode agentMode = runBackupAgents == AgentOnly ? (i == ips.size() - 1 ? AgentOnly : AgentNone) : runBackupAgents; @@ -2196,7 +2198,7 @@ ACTOR void setupAndRun(std::string dataFolder, bool restoring, std::string whitelistBinPaths) { state std::vector> systemActors; - state Optional connFile; + state Optional connectionString; state Standalone startingConfiguration; state int testerCount = 1; state TestConfig testConfig; @@ -2258,7 +2260,7 @@ ACTOR void setupAndRun(std::string dataFolder, wait(timeoutError(restartSimulatedSystem(&systemActors, dataFolder, &testerCount, - &connFile, + &connectionString, &startingConfiguration, testConfig, whitelistBinPaths, @@ -2273,7 +2275,7 @@ ACTOR void setupAndRun(std::string dataFolder, setupSimulatedSystem(&systemActors, dataFolder, &testerCount, - &connFile, + &connectionString, &startingConfiguration, whitelistBinPaths, testConfig, @@ -2282,7 +2284,7 @@ ACTOR void setupAndRun(std::string dataFolder, } std::string clusterFileDir = joinPath(dataFolder, deterministicRandom()->randomUniqueID().toString()); platform::createDirectory(clusterFileDir); - writeFile(joinPath(clusterFileDir, "fdb.cluster"), connFile.get().toString()); + writeFile(joinPath(clusterFileDir, "fdb.cluster"), connectionString.get().toString()); wait(timeoutError(runTests(makeReference(joinPath(clusterFileDir, "fdb.cluster")), TEST_TYPE_FROM_FILE, TEST_ON_TESTERS, diff --git a/fdbserver/Status.actor.cpp b/fdbserver/Status.actor.cpp index 3ba311b8bf..1f4c4f25db 100644 --- a/fdbserver/Status.actor.cpp +++ b/fdbserver/Status.actor.cpp @@ -813,7 +813,7 @@ ACTOR static Future processStatusFetcher( } } - for (auto& coordinator : coordinators.ccf->getConnectionString().coordinators()) { + for (auto& coordinator : coordinators.ccr->getConnectionString().coordinators()) { roles.addCoordinatorRole(coordinator); } @@ -2423,7 +2423,7 @@ static JsonBuilderObject faultToleranceStatusFetcher(DatabaseConfiguration confi workerZones[worker.interf.address()] = worker.interf.locality.zoneId().orDefault(LiteralStringRef("")); } std::map coordinatorZoneCounts; - for (auto& coordinator : coordinators.ccf->getConnectionString().coordinators()) { + for (auto& coordinator : coordinators.ccr->getConnectionString().coordinators()) { auto zone = workerZones[coordinator]; coordinatorZoneCounts[zone] += 1; } @@ -2806,7 +2806,7 @@ ACTOR Future clusterGetStatus( state JsonBuilderObject data_overlay; statusObj["protocol_version"] = format("%" PRIx64, g_network->protocolVersion().version()); - statusObj["connection_string"] = coordinators.ccf->getConnectionString().toString(); + statusObj["connection_string"] = coordinators.ccr->getConnectionString().toString(); statusObj["bounce_impact"] = getBounceImpactInfo(statusCode); state Optional configuration; diff --git a/fdbserver/TesterInterface.actor.h b/fdbserver/TesterInterface.actor.h index 3874ff134d..0e73217a10 100644 --- a/fdbserver/TesterInterface.actor.h +++ b/fdbserver/TesterInterface.actor.h @@ -113,14 +113,14 @@ struct TesterInterface { }; ACTOR Future testerServerCore(TesterInterface interf, - Reference ccf, + Reference ccr, Reference const> serverDBInfo, LocalityData locality); enum test_location_t { TEST_HERE, TEST_ON_SERVERS, TEST_ON_TESTERS }; enum test_type_t { TEST_TYPE_FROM_FILE, TEST_TYPE_CONSISTENCY_CHECK, TEST_TYPE_UNIT_TESTS }; -ACTOR Future runTests(Reference connFile, +ACTOR Future runTests(Reference connRecord, test_type_t whatToRun, test_location_t whereToRun, int minTestersExpected, diff --git a/fdbserver/WorkerInterface.actor.h b/fdbserver/WorkerInterface.actor.h index 6d93202b01..fc904b5718 100644 --- a/fdbserver/WorkerInterface.actor.h +++ b/fdbserver/WorkerInterface.actor.h @@ -884,7 +884,7 @@ ACTOR Future extractClusterInterface( Reference> const> in, Reference>> out); -ACTOR Future fdbd(Reference ccf, +ACTOR Future fdbd(Reference ccr, LocalityData localities, ProcessClass processClass, std::string dataFolder, @@ -898,7 +898,7 @@ ACTOR Future fdbd(Reference ccf, std::map manualKnobOverrides, ConfigDBType configDBType); -ACTOR Future clusterController(Reference ccf, +ACTOR Future clusterController(Reference ccr, Reference>> currentCC, Reference> asyncPriorityInfo, Future recoveredDiskFiles, @@ -922,8 +922,8 @@ ACTOR Future storageServer( Reference const> db, std::string folder, Promise recovered, - Reference - connFile); // changes pssi->id() to be the recovered ID); // changes pssi->id() to be the recovered ID + Reference + connRecord); // changes pssi->id() to be the recovered ID); // changes pssi->id() to be the recovered ID ACTOR Future masterServer(MasterInterface mi, Reference const> db, Reference> const> ccInterface, diff --git a/fdbserver/fdbserver.actor.cpp b/fdbserver/fdbserver.actor.cpp index fd753e82bf..1889b9525b 100644 --- a/fdbserver/fdbserver.actor.cpp +++ b/fdbserver/fdbserver.actor.cpp @@ -36,6 +36,7 @@ #include #include "fdbclient/ActorLineageProfiler.h" +#include "fdbclient/ClusterConnectionFile.h" #include "fdbclient/IKnobCollection.h" #include "fdbclient/NativeAPI.actor.h" #include "fdbclient/SystemData.h" @@ -805,7 +806,7 @@ Optional checkBuggifyOverride(const char* testFile) { // Takes a vector of public and listen address strings given via command line, and returns vector of NetworkAddress // objects. std::pair buildNetworkAddresses( - const ClusterConnectionFile& connectionFile, + const IClusterConnectionRecord& connectionRecord, const std::vector& publicAddressStrs, std::vector& listenAddressStrs) { if (listenAddressStrs.size() > 0 && publicAddressStrs.size() != listenAddressStrs.size()) { @@ -823,7 +824,7 @@ std::pair buildNetworkAddresses( NetworkAddressList publicNetworkAddresses; NetworkAddressList listenNetworkAddresses; - auto& coordinators = connectionFile.getConnectionString().coordinators(); + auto& coordinators = connectionRecord.getConnectionString().coordinators(); ASSERT(coordinators.size() > 0); for (int ii = 0; ii < publicAddressStrs.size(); ++ii) { @@ -833,7 +834,7 @@ std::pair buildNetworkAddresses( if (autoPublicAddress) { try { const NetworkAddress& parsedAddress = NetworkAddress::parse("0.0.0.0:" + publicAddressStr.substr(5)); - const IPAddress publicIP = determinePublicIPAutomatically(connectionFile.getConnectionString()); + const IPAddress publicIP = determinePublicIPAutomatically(connectionRecord.getConnectionString()); currentPublicAddress = NetworkAddress(publicIP, parsedAddress.port, true, parsedAddress.isTLS()); } catch (Error& e) { fprintf(stderr, @@ -998,7 +999,7 @@ struct CLIOptions { std::string configPath; ConfigDBType configDBType{ ConfigDBType::DISABLED }; - Reference connectionFile; + Reference connectionFile; Standalone machineId; UnitTestParameters testParams; @@ -1849,7 +1850,7 @@ int main(int argc, char* argv[]) { .detail("FileSystem", opts.fileSystemPath) .detail("DataFolder", opts.dataFolder) .detail("WorkingDirectory", cwd) - .detail("ClusterFile", opts.connectionFile ? opts.connectionFile->getFilename().c_str() : "") + .detail("ClusterFile", opts.connectionFile ? opts.connectionFile->toString() : "") .detail("ConnectionString", opts.connectionFile ? opts.connectionFile->getConnectionString().toString() : "") .detailf("ActualTime", "%lld", DEBUG_DETERMINISM ? 0 : time(nullptr)) diff --git a/fdbserver/masterserver.actor.cpp b/fdbserver/masterserver.actor.cpp index 428425e6c5..292a758f42 100644 --- a/fdbserver/masterserver.actor.cpp +++ b/fdbserver/masterserver.actor.cpp @@ -1867,7 +1867,7 @@ ACTOR Future masterCore(Reference self) { tr.set( recoveryCommitRequest.arena, primaryLocalityKey, BinaryWriter::toValue(self->primaryLocality, Unversioned())); tr.set(recoveryCommitRequest.arena, backupVersionKey, backupVersionValue); - tr.set(recoveryCommitRequest.arena, coordinatorsKey, self->coordinators.ccf->getConnectionString().toString()); + tr.set(recoveryCommitRequest.arena, coordinatorsKey, self->coordinators.ccr->getConnectionString().toString()); tr.set(recoveryCommitRequest.arena, logsKey, self->logSystem->getLogsValue()); tr.set(recoveryCommitRequest.arena, primaryDatacenterKey, diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 43d780980a..c2a1faf577 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -5211,13 +5211,13 @@ bool storageServerTerminated(StorageServer& self, IKeyValueStore* persistentData return false; } -ACTOR Future memoryStoreRecover(IKeyValueStore* store, Reference connFile, UID id) { - if (store->getType() != KeyValueStoreType::MEMORY || connFile.getPtr() == nullptr) { +ACTOR Future memoryStoreRecover(IKeyValueStore* store, Reference connRecord, UID id) { + if (store->getType() != KeyValueStoreType::MEMORY || connRecord.getPtr() == nullptr) { return Never(); } // create a temp client connect to DB - Database cx = Database::createDatabase(connFile, Database::API_VERSION_LATEST); + Database cx = Database::createDatabase(connRecord, Database::API_VERSION_LATEST); state Reference tr = makeReference(cx); state int noCanRemoveCount = 0; @@ -5451,7 +5451,7 @@ ACTOR Future storageServer(IKeyValueStore* persistentData, Reference const> db, std::string folder, Promise recovered, - Reference connFile) { + Reference connRecord) { state StorageServer self(persistentData, db, ssi); self.folder = folder; @@ -5465,7 +5465,7 @@ ACTOR Future storageServer(IKeyValueStore* persistentData, // for memory storage engine type, wait until recovery is done before commit when(wait(self.storage.commit())) {} - when(wait(memoryStoreRecover(persistentData, connFile, self.thisServerID))) { + when(wait(memoryStoreRecover(persistentData, connRecord, self.thisServerID))) { TraceEvent("DisposeStorageServer", self.thisServerID).log(); throw worker_removed(); } diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index 3184ba3ef4..4bfaf2c722 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -599,7 +599,7 @@ ACTOR Future runWorkloadAsync(Database cx, } ACTOR Future testerServerWorkload(WorkloadRequest work, - Reference ccf, + Reference ccr, Reference const> dbInfo, LocalityData locality) { state WorkloadInterface workIface; @@ -614,7 +614,7 @@ ACTOR Future testerServerWorkload(WorkloadRequest work, startRole(Role::TESTER, workIface.id(), UID(), details); if (work.useDatabase) { - cx = Database::createDatabase(ccf, -1, IsInternal::True, locality); + cx = Database::createDatabase(ccr, -1, IsInternal::True, locality); wait(delay(1.0)); } @@ -658,7 +658,7 @@ ACTOR Future testerServerWorkload(WorkloadRequest work, } ACTOR Future testerServerCore(TesterInterface interf, - Reference ccf, + Reference ccr, Reference const> dbInfo, LocalityData locality) { state PromiseStream> addWorkload; @@ -668,7 +668,7 @@ ACTOR Future testerServerCore(TesterInterface interf, loop choose { when(wait(workerFatalError)) {} when(WorkloadRequest work = waitNext(interf.recruitments.getFuture())) { - addWorkload.send(testerServerWorkload(work, ccf, dbInfo, locality)); + addWorkload.send(testerServerWorkload(work, ccr, dbInfo, locality)); } } } @@ -1583,8 +1583,8 @@ ACTOR Future runTests(Reference runTests(Reference runTests(Reference connFile, +ACTOR Future runTests(Reference connRecord, test_type_t whatToRun, test_location_t at, int minTestersExpected, @@ -1612,8 +1612,8 @@ ACTOR Future runTests(Reference connFile, auto cc = makeReference>>(); auto ci = makeReference>>(); std::vector> actors; - if (connFile) { - actors.push_back(reportErrors(monitorLeader(connFile, cc), "MonitorLeader")); + if (connRecord) { + actors.push_back(reportErrors(monitorLeader(connRecord, cc), "MonitorLeader")); actors.push_back(reportErrors(extractClusterInterface(cc, ci), "ExtractClusterInterface")); } @@ -1688,7 +1688,7 @@ ACTOR Future runTests(Reference connFile, std::vector iTesters(1); actors.push_back( reportErrors(monitorServerDBInfo(cc, LocalityData(), db), "MonitorServerDBInfo")); // FIXME: Locality - actors.push_back(reportErrors(testerServerCore(iTesters[0], connFile, db, locality), "TesterServerCore")); + actors.push_back(reportErrors(testerServerCore(iTesters[0], connRecord, db, locality), "TesterServerCore")); tests = runTests(cc, ci, iTesters, testSpecs, startingConfiguration, locality); } else { tests = reportErrors(runTests(cc, ci, testSpecs, at, minTestersExpected, startingConfiguration, locality), diff --git a/fdbserver/worker.actor.cpp b/fdbserver/worker.actor.cpp index 3d912737ff..ffb5f95b25 100644 --- a/fdbserver/worker.actor.cpp +++ b/fdbserver/worker.actor.cpp @@ -518,7 +518,7 @@ ACTOR Future registrationClient(Reference> const> ddInterf, Reference> const> rkInterf, Reference const> degraded, - Reference connFile, + Reference connRecord, Reference> const> issues, Reference localConfig) { // Keeps the cluster controller (as it may be re-elected) informed that this worker exists @@ -532,6 +532,16 @@ ACTOR Future registrationClient(Reference cacheErrorsFuture; state Optional incorrectTime; loop { + state ClusterConnectionString storedConnectionString; + state bool upToDate = true; + if (connRecord) { + bool upToDateResult = wait(connRecord->upToDate(storedConnectionString)); + upToDate = upToDateResult; + } + if (upToDate) { + incorrectTime = Optional(); + } + RegisterWorkerRequest request(interf, initialClass, processClass, @@ -542,28 +552,25 @@ ACTOR Future registrationClient(Referenceget(), localConfig->lastSeenVersion(), localConfig->configClassSet()); + for (auto const& i : issues->get()) { request.issues.push_back_deep(request.issues.arena(), i); } - ClusterConnectionString fileConnectionString; - if (connFile && !connFile->fileContentsUpToDate(fileConnectionString)) { + + if (!upToDate) { request.issues.push_back_deep(request.issues.arena(), LiteralStringRef("incorrect_cluster_file_contents")); - std::string connectionString = connFile->getConnectionString().toString(); + std::string connectionString = connRecord->getConnectionString().toString(); if (!incorrectTime.present()) { incorrectTime = now(); } - if (connFile->canGetFilename()) { - // Don't log a SevWarnAlways initially to account for transient issues (e.g. someone else changing the - // file right before us) - TraceEvent(now() - incorrectTime.get() > 300 ? SevWarnAlways : SevWarn, "IncorrectClusterFileContents") - .detail("Filename", connFile->getFilename()) - .detail("ConnectionStringFromFile", fileConnectionString.toString()) - .detail("CurrentConnectionString", connectionString); - } - } else { - incorrectTime = Optional(); - } + // Don't log a SevWarnAlways initially to account for transient issues (e.g. someone else changing + // the file right before us) + TraceEvent(now() - incorrectTime.get() > 300 ? SevWarnAlways : SevWarn, "IncorrectClusterFileContents") + .detail("ClusterFile", connRecord->toString()) + .detail("StoredConnectionString", storedConnectionString.toString()) + .detail("CurrentConnectionString", connectionString); + } auto peers = FlowTransport::transport().getIncompatiblePeers(); for (auto it = peers->begin(); it != peers->end();) { if (now() - it->second.second > FLOW_KNOBS->INCOMPATIBLE_PEER_DELAY_BEFORE_LOGGING) { @@ -1095,7 +1102,7 @@ ACTOR Future storageServerRollbackRebooter(std::set(), Reference(nullptr)); + storageServer(store, recruited, db, folder, Promise(), Reference(nullptr)); prevStorageServer = handleIOErrors(prevStorageServer, store, id, store->onClosed()); } } @@ -1308,7 +1315,7 @@ struct SharedLogsValue { : actor(actor), uid(uid), requests(requests) {} }; -ACTOR Future workerServer(Reference connFile, +ACTOR Future workerServer(Reference connRecord, Reference> const> ccInterface, LocalityData locality, Reference> asyncPriorityInfo, @@ -1383,7 +1390,7 @@ ACTOR Future workerServer(Reference connFile, errorForwarders.add(loadedPonger(interf.debugPing.getFuture())); errorForwarders.add(waitFailureServer(interf.waitFailure.getFuture())); errorForwarders.add(monitorTraceLogIssues(issues)); - errorForwarders.add(testerServerCore(interf.testerInterface, connFile, dbInfo, locality)); + errorForwarders.add(testerServerCore(interf.testerInterface, connRecord, dbInfo, locality)); errorForwarders.add(monitorHighMemory(memoryProfileThreshold)); filesClosed.add(stopping.getFuture()); @@ -1464,7 +1471,7 @@ ACTOR Future workerServer(Reference connFile, DUMPTOKEN(recruited.getKeyValuesStream); Promise recovery; - Future f = storageServer(kv, recruited, dbInfo, folder, recovery, connFile); + Future f = storageServer(kv, recruited, dbInfo, folder, recovery, connRecord); recoveries.push_back(recovery.getFuture()); f = handleIOErrors(f, kv, s.storeID, kvClosed); f = storageServerRollbackRebooter(&runningStorages, @@ -1590,7 +1597,7 @@ ACTOR Future workerServer(Reference connFile, ddInterf, rkInterf, degraded, - connFile, + connRecord, issues, localConfig)); @@ -1705,7 +1712,7 @@ ACTOR Future workerServer(Reference connFile, // printf("Recruited as masterServer\n"); Future masterProcess = masterServer( - recruited, dbInfo, ccInterface, ServerCoordinators(connFile), req.lifetime, req.forceRecovery); + recruited, dbInfo, ccInterface, ServerCoordinators(connRecord), req.lifetime, req.forceRecovery); errorForwarders.add( zombie(recruited, forwardError(errors, Role::MASTER, recruited.id(), masterProcess))); req.reply.send(recruited); @@ -2293,10 +2300,10 @@ ACTOR Future createAndLockProcessIdFile(std::string folder) { } ACTOR Future monitorLeaderWithDelayedCandidacyImplOneGeneration( - Reference connFile, + Reference connRecord, Reference> result, MonitorLeaderInfo info) { - state ClusterConnectionString ccf = info.intermediateConnFile->getConnectionString(); + state ClusterConnectionString ccf = info.intermediateConnRecord->getConnectionString(); state std::vector addrs = ccf.coordinators(); state ElectionResultRequest request; state int index = 0; @@ -2314,24 +2321,24 @@ ACTOR Future monitorLeaderWithDelayedCandidacyImplOneGenerati if (leader.present()) { if (leader.get().present()) { if (leader.get().get().forward) { - info.intermediateConnFile = makeReference( - connFile->getFilename(), ClusterConnectionString(leader.get().get().serializedInfo.toString())); + info.intermediateConnRecord = connRecord->makeIntermediateRecord( + ClusterConnectionString(leader.get().get().serializedInfo.toString())); return info; } - if (connFile != info.intermediateConnFile) { + if (connRecord != info.intermediateConnRecord) { if (!info.hasConnected) { TraceEvent(SevWarnAlways, "IncorrectClusterFileContentsAtConnection") - .detail("Filename", connFile->getFilename()) - .detail("ConnectionStringFromFile", connFile->getConnectionString().toString()) + .detail("ClusterFile", connRecord->toString()) + .detail("StoredConnectionString", connRecord->getConnectionString().toString()) .detail("CurrentConnectionString", - info.intermediateConnFile->getConnectionString().toString()); + info.intermediateConnRecord->getConnectionString().toString()); } - connFile->setConnectionString(info.intermediateConnFile->getConnectionString()); - info.intermediateConnFile = connFile; + connRecord->setConnectionString(info.intermediateConnRecord->getConnectionString()); + info.intermediateConnRecord = connRecord; } info.hasConnected = true; - connFile->notifyConnected(); + connRecord->notifyConnected(); request.knownLeader = leader.get().get().changeID; ClusterControllerPriorityInfo info = leader.get().get().getPriorityInfo(); @@ -2354,35 +2361,35 @@ ACTOR Future monitorLeaderWithDelayedCandidacyImplOneGenerati } } -ACTOR Future monitorLeaderWithDelayedCandidacyImplInternal(Reference connFile, +ACTOR Future monitorLeaderWithDelayedCandidacyImplInternal(Reference connRecord, Reference> outSerializedLeaderInfo) { - state MonitorLeaderInfo info(connFile); + state MonitorLeaderInfo info(connRecord); loop { MonitorLeaderInfo _info = - wait(monitorLeaderWithDelayedCandidacyImplOneGeneration(connFile, outSerializedLeaderInfo, info)); + wait(monitorLeaderWithDelayedCandidacyImplOneGeneration(connRecord, outSerializedLeaderInfo, info)); info = _info; } } template Future monitorLeaderWithDelayedCandidacyImpl( - Reference const& connFile, + Reference const& connRecord, Reference>> const& outKnownLeader) { LeaderDeserializer deserializer; auto serializedInfo = makeReference>(); - Future m = monitorLeaderWithDelayedCandidacyImplInternal(connFile, serializedInfo); + Future m = monitorLeaderWithDelayedCandidacyImplInternal(connRecord, serializedInfo); return m || deserializer(serializedInfo, outKnownLeader); } ACTOR Future monitorLeaderWithDelayedCandidacy( - Reference connFile, + Reference connRecord, Reference>> currentCC, Reference> asyncPriorityInfo, Future recoveredDiskFiles, LocalityData locality, Reference> dbInfo, ConfigDBType configDBType) { - state Future monitor = monitorLeaderWithDelayedCandidacyImpl(connFile, currentCC); + state Future monitor = monitorLeaderWithDelayedCandidacyImpl(connRecord, currentCC); state Future timeout; wait(recoveredDiskFiles); @@ -2408,7 +2415,7 @@ ACTOR Future monitorLeaderWithDelayedCandidacy( when(wait(timeout.isValid() ? timeout : Never())) { monitor.cancel(); wait(clusterController( - connFile, currentCC, asyncPriorityInfo, recoveredDiskFiles, locality, configDBType)); + connRecord, currentCC, asyncPriorityInfo, recoveredDiskFiles, locality, configDBType)); return Void(); } } @@ -2458,7 +2465,7 @@ ACTOR Future serveProcess() { } } -ACTOR Future fdbd(Reference connFile, +ACTOR Future fdbd(Reference connRecord, LocalityData localities, ProcessClass processClass, std::string dataFolder, @@ -2489,7 +2496,7 @@ ACTOR Future fdbd(Reference connFile, actors.push_back(serveProcess()); try { - ServerCoordinators coordinators(connFile); + ServerCoordinators coordinators(connRecord); if (g_network->isSimulated()) { whitelistBinPaths = ",, random_path, /bin/snap_create.sh,,"; } @@ -2506,7 +2513,7 @@ ACTOR Future fdbd(Reference connFile, if (coordFolder.size()) { // SOMEDAY: remove the fileNotFound wrapper and make DiskQueue construction safe from errors setting up // their files - actors.push_back(fileNotFoundToNever(coordinationServer(coordFolder, coordinators.ccf, configDBType))); + actors.push_back(fileNotFoundToNever(coordinationServer(coordFolder, coordinators.ccr, configDBType))); } state UID processIDUid = wait(createAndLockProcessIdFile(dataFolder)); @@ -2523,21 +2530,25 @@ ACTOR Future fdbd(Reference connFile, actors.push_back(reportErrors(monitorAndWriteCCPriorityInfo(fitnessFilePath, asyncPriorityInfo), "MonitorAndWriteCCPriorityInfo")); if (processClass.machineClassFitness(ProcessClass::ClusterController) == ProcessClass::NeverAssign) { - actors.push_back(reportErrors(monitorLeader(connFile, cc), "ClusterController")); + actors.push_back(reportErrors(monitorLeader(connRecord, cc), "ClusterController")); } else if (processClass.machineClassFitness(ProcessClass::ClusterController) == ProcessClass::WorstFit && SERVER_KNOBS->MAX_DELAY_CC_WORST_FIT_CANDIDACY_SECONDS > 0) { - actors.push_back(reportErrors( - monitorLeaderWithDelayedCandidacy( - connFile, cc, asyncPriorityInfo, recoveredDiskFiles.getFuture(), localities, dbInfo, configDBType), - "ClusterController")); + actors.push_back(reportErrors(monitorLeaderWithDelayedCandidacy(connRecord, + cc, + asyncPriorityInfo, + recoveredDiskFiles.getFuture(), + localities, + dbInfo, + configDBType), + "ClusterController")); } else { actors.push_back(reportErrors( clusterController( - connFile, cc, asyncPriorityInfo, recoveredDiskFiles.getFuture(), localities, configDBType), + connRecord, cc, asyncPriorityInfo, recoveredDiskFiles.getFuture(), localities, configDBType), "ClusterController")); } actors.push_back(reportErrors(extractClusterInterface(cc, ci), "ExtractClusterInterface")); - actors.push_back(reportErrorsExcept(workerServer(connFile, + actors.push_back(reportErrorsExcept(workerServer(connRecord, cc, localities, asyncPriorityInfo, diff --git a/fdbserver/workloads/ApiWorkload.h b/fdbserver/workloads/ApiWorkload.h index b3992f33f3..c345f95226 100644 --- a/fdbserver/workloads/ApiWorkload.h +++ b/fdbserver/workloads/ApiWorkload.h @@ -23,6 +23,7 @@ #pragma once #include "fdbserver/workloads/workloads.actor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/ReadYourWrites.h" #include "fdbclient/ThreadSafeTransaction.h" #include "fdbserver/workloads/MemoryKeyValueStore.h" @@ -239,7 +240,7 @@ struct ApiWorkload : TestWorkload { useExtraDB = g_simulator.extraDB != nullptr; if (useExtraDB) { - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); } } diff --git a/fdbserver/workloads/AtomicSwitchover.actor.cpp b/fdbserver/workloads/AtomicSwitchover.actor.cpp index f60156ddf7..aedd4f0dae 100644 --- a/fdbserver/workloads/AtomicSwitchover.actor.cpp +++ b/fdbserver/workloads/AtomicSwitchover.actor.cpp @@ -20,6 +20,7 @@ #include "fdbrpc/simulator.h" #include "fdbclient/BackupAgent.actor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbserver/workloads/workloads.actor.h" #include "fdbserver/workloads/BulkSetup.actor.h" #include "flow/actorcompiler.h" // This must be the last #include. @@ -38,7 +39,7 @@ struct AtomicSwitchoverWorkload : TestWorkload { backupRanges.push_back_deep(backupRanges.arena(), normalKeys); - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); } diff --git a/fdbserver/workloads/BackupToDBAbort.actor.cpp b/fdbserver/workloads/BackupToDBAbort.actor.cpp index 1cad65dc7b..1a6a23eca6 100644 --- a/fdbserver/workloads/BackupToDBAbort.actor.cpp +++ b/fdbserver/workloads/BackupToDBAbort.actor.cpp @@ -19,6 +19,7 @@ */ #include "fdbclient/BackupAgent.actor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/ManagementAPI.actor.h" #include "fdbclient/NativeAPI.actor.h" #include "fdbserver/workloads/workloads.actor.h" @@ -35,7 +36,7 @@ struct BackupToDBAbort : TestWorkload { backupRanges.push_back_deep(backupRanges.arena(), normalKeys); - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); lockid = UID(0xbeeffeed, 0xdecaf00d); diff --git a/fdbserver/workloads/BackupToDBCorrectness.actor.cpp b/fdbserver/workloads/BackupToDBCorrectness.actor.cpp index c2d0ecd1ed..b80952f352 100644 --- a/fdbserver/workloads/BackupToDBCorrectness.actor.cpp +++ b/fdbserver/workloads/BackupToDBCorrectness.actor.cpp @@ -20,6 +20,7 @@ #include "fdbrpc/simulator.h" #include "fdbclient/BackupAgent.actor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbserver/workloads/workloads.actor.h" #include "fdbserver/workloads/BulkSetup.actor.h" #include "flow/actorcompiler.h" // This must be the last #include. @@ -127,7 +128,7 @@ struct BackupToDBCorrectnessWorkload : TestWorkload { } } - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); TraceEvent("BARW_Start").detail("Locked", locked); diff --git a/fdbserver/workloads/BackupToDBUpgrade.actor.cpp b/fdbserver/workloads/BackupToDBUpgrade.actor.cpp index 414bcfabb3..91c56166a1 100644 --- a/fdbserver/workloads/BackupToDBUpgrade.actor.cpp +++ b/fdbserver/workloads/BackupToDBUpgrade.actor.cpp @@ -20,6 +20,7 @@ #include "fdbrpc/simulator.h" #include "fdbclient/BackupAgent.actor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbserver/workloads/workloads.actor.h" #include "fdbserver/workloads/BulkSetup.actor.h" #include "fdbclient/ManagementAPI.actor.h" @@ -75,7 +76,7 @@ struct BackupToDBUpgradeWorkload : TestWorkload { } } - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); TraceEvent("DRU_Start").log(); diff --git a/fdbserver/workloads/ChangeConfig.actor.cpp b/fdbserver/workloads/ChangeConfig.actor.cpp index 50073f29a0..c19a97782d 100644 --- a/fdbserver/workloads/ChangeConfig.actor.cpp +++ b/fdbserver/workloads/ChangeConfig.actor.cpp @@ -19,6 +19,7 @@ */ #include "fdbclient/NativeAPI.actor.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/ClusterInterface.h" #include "fdbserver/TesterInterface.actor.h" #include "fdbclient/ManagementAPI.actor.h" @@ -56,7 +57,7 @@ struct ChangeConfigWorkload : TestWorkload { // for the extra cluster. ACTOR Future extraDatabaseConfigure(ChangeConfigWorkload* self) { if (g_network->isSimulated() && g_simulator.extraDB) { - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); state Database extraDB = Database::createDatabase(extraFile, -1); wait(delay(5 * deterministicRandom()->random01())); diff --git a/fdbserver/workloads/DifferentClustersSameRV.actor.cpp b/fdbserver/workloads/DifferentClustersSameRV.actor.cpp index de5fd7549b..197f924b91 100644 --- a/fdbserver/workloads/DifferentClustersSameRV.actor.cpp +++ b/fdbserver/workloads/DifferentClustersSameRV.actor.cpp @@ -18,6 +18,7 @@ * limitations under the License. */ +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/ManagementAPI.actor.h" #include "fdbclient/RunTransaction.actor.h" #include "fdbrpc/simulator.h" @@ -37,7 +38,7 @@ struct DifferentClustersSameRVWorkload : TestWorkload { DifferentClustersSameRVWorkload(WorkloadContext const& wcx) : TestWorkload(wcx) { ASSERT(g_simulator.extraDB != nullptr); - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); testDuration = getOption(options, LiteralStringRef("testDuration"), 100.0); switchAfter = getOption(options, LiteralStringRef("switchAfter"), 50.0); @@ -53,7 +54,7 @@ struct DifferentClustersSameRVWorkload : TestWorkload { if (clientId != 0) { return Void(); } - auto switchConnFileDb = Database::createDatabase(cx->getConnectionFile(), -1); + auto switchConnFileDb = Database::createDatabase(cx->getConnectionRecord(), -1); originalDB = cx; std::vector> clients = { readerClientSeparateDBs(cx, this), doSwitch(switchConnFileDb, this), @@ -141,8 +142,8 @@ struct DifferentClustersSameRVWorkload : TestWorkload { TraceEvent("DifferentClusters_CopiedDatabase").log(); wait(advanceVersion(self->extraDB, rv)); TraceEvent("DifferentClusters_AdvancedVersion").log(); - wait(cx->switchConnectionFile( - makeReference(self->extraDB->getConnectionFile()->getConnectionString()))); + wait(cx->switchConnectionRecord( + makeReference(self->extraDB->getConnectionRecord()->getConnectionString()))); TraceEvent("DifferentClusters_SwitchedConnectionFile").log(); state Transaction tr(cx); tr.setVersion(rv); @@ -156,7 +157,7 @@ struct DifferentClustersSameRVWorkload : TestWorkload { TraceEvent("DifferentClusters_ReadError").error(e); wait(tr.onError(e)); } - // In an actual switch we would call switchConnectionFile after unlocking the database. But it's possible + // In an actual switch we would call switchConnectionRecord after unlocking the database. But it's possible // that a storage server serves a read at |rv| even after the recovery caused by unlocking the database, and we // want to make that more likely for this test. So read at |rv| then unlock. wait(unlockDatabase(self->extraDB, lockUid)); diff --git a/fdbserver/workloads/KillRegion.actor.cpp b/fdbserver/workloads/KillRegion.actor.cpp index a6b9f10618..6490c2c4bd 100644 --- a/fdbserver/workloads/KillRegion.actor.cpp +++ b/fdbserver/workloads/KillRegion.actor.cpp @@ -101,7 +101,7 @@ struct KillRegionWorkload : TestWorkload { TraceEvent("ForceRecovery_Begin").log(); - wait(forceRecovery(cx->getConnectionFile(), LiteralStringRef("1"))); + wait(forceRecovery(cx->getConnectionRecord(), LiteralStringRef("1"))); TraceEvent("ForceRecovery_UsableRegions").log(); diff --git a/fdbserver/workloads/VersionStamp.actor.cpp b/fdbserver/workloads/VersionStamp.actor.cpp index d2576002fa..a5055d2e4d 100644 --- a/fdbserver/workloads/VersionStamp.actor.cpp +++ b/fdbserver/workloads/VersionStamp.actor.cpp @@ -19,6 +19,7 @@ */ #include "fdbrpc/ContinuousSample.h" +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/NativeAPI.actor.h" #include "fdbserver/TesterInterface.actor.h" #include "fdbserver/workloads/BulkSetup.actor.h" @@ -150,7 +151,7 @@ struct VersionStampWorkload : TestWorkload { ACTOR Future _check(Database cx, VersionStampWorkload* self) { if (self->validateExtraDB) { - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); cx = Database::createDatabase(extraFile, -1); } state ReadYourWritesTransaction tr(cx); @@ -309,7 +310,7 @@ struct VersionStampWorkload : TestWorkload { state Database extraDB; if (g_simulator.extraDB != nullptr) { - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); } diff --git a/fdbserver/workloads/WriteDuringRead.actor.cpp b/fdbserver/workloads/WriteDuringRead.actor.cpp index 79dd30eb48..e0d47d68e4 100644 --- a/fdbserver/workloads/WriteDuringRead.actor.cpp +++ b/fdbserver/workloads/WriteDuringRead.actor.cpp @@ -18,6 +18,7 @@ * limitations under the License. */ +#include "fdbclient/ClusterConnectionMemoryRecord.h" #include "fdbclient/NativeAPI.actor.h" #include "fdbserver/TesterInterface.actor.h" #include "fdbclient/ReadYourWrites.h" @@ -81,7 +82,7 @@ struct WriteDuringReadWorkload : TestWorkload { useExtraDB = g_simulator.extraDB != nullptr; if (useExtraDB) { - auto extraFile = makeReference(*g_simulator.extraDB); + auto extraFile = makeReference(*g_simulator.extraDB); extraDB = Database::createDatabase(extraFile, -1); useSystemKeys = false; } From 9358adcf49726cd21a72375a23789f363a026011 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 11 Oct 2021 14:39:05 -0700 Subject: [PATCH 33/42] Address some review comments. --- fdbclient/ClusterConnectionFile.actor.cpp | 9 +++++---- fdbclient/ClusterConnectionFile.h | 6 +++--- fdbclient/ClusterConnectionKey.actor.cpp | 11 ++++++----- fdbclient/ClusterConnectionKey.actor.h | 5 ++--- fdbclient/ClusterConnectionMemoryRecord.h | 3 ++- fdbclient/CoordinationInterface.h | 10 ++++++---- fdbclient/MonitorLeader.actor.cpp | 2 ++ 7 files changed, 26 insertions(+), 20 deletions(-) diff --git a/fdbclient/ClusterConnectionFile.actor.cpp b/fdbclient/ClusterConnectionFile.actor.cpp index 9cfb410382..22f49bacf5 100644 --- a/fdbclient/ClusterConnectionFile.actor.cpp +++ b/fdbclient/ClusterConnectionFile.actor.cpp @@ -22,8 +22,9 @@ #include "fdbclient/MonitorLeader.h" #include "flow/actorcompiler.h" // has to be last include -// Loads and parses the file at 'path', throwing errors if the file cannot be read or the format is invalid. -ClusterConnectionFile::ClusterConnectionFile(std::string const& filename) : IClusterConnectionRecord(false) { +// Loads and parses the file at 'filename', throwing errors if the file cannot be read or the format is invalid. +ClusterConnectionFile::ClusterConnectionFile(std::string const& filename) + : IClusterConnectionRecord(ConnectionStringNeedsPersisted::False) { if (!fileExists(filename)) { throw no_cluster_file_found(); } @@ -34,7 +35,7 @@ ClusterConnectionFile::ClusterConnectionFile(std::string const& filename) : IClu // Creates a cluster file with a given connection string and saves it to the specified file. ClusterConnectionFile::ClusterConnectionFile(std::string const& filename, ClusterConnectionString const& contents) - : IClusterConnectionRecord(true) { + : IClusterConnectionRecord(ConnectionStringNeedsPersisted::True) { this->filename = filename; cs = contents; } @@ -45,7 +46,7 @@ ClusterConnectionString const& ClusterConnectionFile::getConnectionString() cons return cs; } -// Sets the connections string held by this object. Calling this function does not persist the string to disk. +// Sets the connections string held by this object and persists it. Future ClusterConnectionFile::setConnectionString(ClusterConnectionString const& conn) { ASSERT(filename.size()); cs = conn; diff --git a/fdbclient/ClusterConnectionFile.h b/fdbclient/ClusterConnectionFile.h index 22a07bc779..b12f6baf9e 100644 --- a/fdbclient/ClusterConnectionFile.h +++ b/fdbclient/ClusterConnectionFile.h @@ -28,8 +28,8 @@ // An implementation of IClusterConnectionRecord backed by a file. class ClusterConnectionFile : public IClusterConnectionRecord, ReferenceCounted, NonCopyable { public: - // Loads and parses the file at 'path', throwing errors if the file cannot be read or the format is invalid. - explicit ClusterConnectionFile(std::string const& path); + // Loads and parses the file at 'filename', throwing errors if the file cannot be read or the format is invalid. + explicit ClusterConnectionFile(std::string const& filename); // Creates a cluster file with a given connection string and saves it to the specified file. explicit ClusterConnectionFile(std::string const& filename, ClusterConnectionString const& contents); @@ -38,7 +38,7 @@ public: // been persisted or if the file has been modified externally. ClusterConnectionString const& getConnectionString() const override; - // Sets the connections string held by this object. Calling this function does not persist the string to disk. + // Sets the connections string held by this object and persists it. Future setConnectionString(ClusterConnectionString const&) override; // Get the connection string stored in the file. diff --git a/fdbclient/ClusterConnectionKey.actor.cpp b/fdbclient/ClusterConnectionKey.actor.cpp index e1864b78ca..bf08e42b24 100644 --- a/fdbclient/ClusterConnectionKey.actor.cpp +++ b/fdbclient/ClusterConnectionKey.actor.cpp @@ -27,7 +27,7 @@ ClusterConnectionKey::ClusterConnectionKey(Database db, Key connectionStringKey, ClusterConnectionString const& contents, - bool needsToBePersisted) + ConnectionStringNeedsPersisted needsToBePersisted) : IClusterConnectionRecord(needsToBePersisted), db(db), cs(contents), connectionStringKey(connectionStringKey) {} // Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the @@ -41,8 +41,10 @@ ACTOR Future> ClusterConnectionKey::loadClusterC if (!v.present()) { throw connection_string_invalid(); } - return makeReference( - db, connectionStringKey, ClusterConnectionString(v.get().toString()), false); + return makeReference(db, + connectionStringKey, + ClusterConnectionString(v.get().toString()), + ConnectionStringNeedsPersisted::False); } catch (Error& e) { wait(tr.onError(e)); } @@ -55,8 +57,7 @@ ClusterConnectionString const& ClusterConnectionKey::getConnectionString() const return cs; } -// Sets the connections string held by this object. Calling this function does not persist the string to the -// database. +// Sets the connections string held by this object and persists it. Future ClusterConnectionKey::setConnectionString(ClusterConnectionString const& connectionString) { cs = connectionString; return success(persist()); diff --git a/fdbclient/ClusterConnectionKey.actor.h b/fdbclient/ClusterConnectionKey.actor.h index a4001c26c5..5417a2fda6 100644 --- a/fdbclient/ClusterConnectionKey.actor.h +++ b/fdbclient/ClusterConnectionKey.actor.h @@ -41,7 +41,7 @@ public: ClusterConnectionKey(Database db, Key connectionStringKey, ClusterConnectionString const& contents, - bool needsToBePersisted = true); + ConnectionStringNeedsPersisted needsToBePersisted = ConnectionStringNeedsPersisted::True); // Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the // format is invalid. @@ -51,8 +51,7 @@ public: // hasn't been persisted or if the key has been modified externally. ClusterConnectionString const& getConnectionString() const override; - // Sets the connections string held by this object. Calling this function does not persist the string to the - // database. + // Sets the connections string held by this object and persists it. Future setConnectionString(ClusterConnectionString const&) override; // Get the connection string stored in the database. diff --git a/fdbclient/ClusterConnectionMemoryRecord.h b/fdbclient/ClusterConnectionMemoryRecord.h index 062c1894fb..2d30855f01 100644 --- a/fdbclient/ClusterConnectionMemoryRecord.h +++ b/fdbclient/ClusterConnectionMemoryRecord.h @@ -31,7 +31,8 @@ class ClusterConnectionMemoryRecord : public IClusterConnectionRecord, public: // Creates a cluster file with a given connection string. explicit ClusterConnectionMemoryRecord(ClusterConnectionString const& cs) - : IClusterConnectionRecord(false), id(deterministicRandom()->randomUniqueID()), cs(cs) {} + : IClusterConnectionRecord(ConnectionStringNeedsPersisted::False), id(deterministicRandom()->randomUniqueID()), + cs(cs) {} // Returns the connection string currently held in this object. ClusterConnectionString const& getConnectionString() const override; diff --git a/fdbclient/CoordinationInterface.h b/fdbclient/CoordinationInterface.h index 27bda0fe7e..b5e3b50bad 100644 --- a/fdbclient/CoordinationInterface.h +++ b/fdbclient/CoordinationInterface.h @@ -77,6 +77,8 @@ private: Key key, keyDesc; }; +FDB_DECLARE_BOOLEAN_PARAM(ConnectionStringNeedsPersisted); + // A record that stores the connection string used to connect to a cluster. This record can be updated when a cluster // notifies a connected party that the connection string has changed. // @@ -85,7 +87,7 @@ private: // one that is only stored in memory. class IClusterConnectionRecord { public: - IClusterConnectionRecord(bool connectionStringNeedsPersisted) + IClusterConnectionRecord(ConnectionStringNeedsPersisted connectionStringNeedsPersisted) : connectionStringNeedsPersisted(connectionStringNeedsPersisted) {} virtual ~IClusterConnectionRecord() {} @@ -93,7 +95,7 @@ public: // been persisted or if the persistent storage for the record has been modified externally. virtual ClusterConnectionString const& getConnectionString() const = 0; - // Sets the connections string held by this object. Calling this function does not persist the record. + // Sets the connections string held by this object and persists it. virtual Future setConnectionString(ClusterConnectionString const&) = 0; // If this record is backed by persistent storage, get the connection string from that storage. Otherwise, return @@ -129,14 +131,14 @@ protected: // Writes the connection string to the backing persistent storage, if applicable. virtual Future persist() = 0; - // Returns whether the connection record contains a connection string that should be persisted upon connection. + // Returns whether the connection record contains a connection string that needs to be persisted upon connection. bool needsToBePersisted() const; // Clears the flag needs persisted flag. void setPersisted(); private: - // A flag that indicates whether this connection record should be persisted when it succesfully establishes a + // A flag that indicates whether this connection record needs to be persisted when it succesfully establishes a // connection. bool connectionStringNeedsPersisted; }; diff --git a/fdbclient/MonitorLeader.actor.cpp b/fdbclient/MonitorLeader.actor.cpp index 40274ba7c9..b2b4069cc9 100644 --- a/fdbclient/MonitorLeader.actor.cpp +++ b/fdbclient/MonitorLeader.actor.cpp @@ -50,6 +50,8 @@ std::string trim(std::string const& connectionString) { } // namespace +FDB_DEFINE_BOOLEAN_PARAM(ConnectionStringNeedsPersisted); + Future IClusterConnectionRecord::upToDate() { ClusterConnectionString temp; return upToDate(temp); From 4f64f98746002514a74c6d00ac4ed6c68a7cac91 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Thu, 21 Oct 2021 13:44:17 -0700 Subject: [PATCH 34/42] Update the toString method to return URIs. Persisting a cluster connection key makes sure that its not overwriting other changes. --- fdbclient/ClusterConnectionFile.actor.cpp | 6 ++-- fdbclient/ClusterConnectionKey.actor.cpp | 35 ++++++++++++++++--- fdbclient/ClusterConnectionKey.actor.h | 1 + .../ClusterConnectionMemoryRecord.actor.cpp | 2 +- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/fdbclient/ClusterConnectionFile.actor.cpp b/fdbclient/ClusterConnectionFile.actor.cpp index 22f49bacf5..f17674184a 100644 --- a/fdbclient/ClusterConnectionFile.actor.cpp +++ b/fdbclient/ClusterConnectionFile.actor.cpp @@ -93,7 +93,7 @@ Reference ClusterConnectionFile::makeIntermediateRecor // Returns a string representation of this cluster connection record. This will include the type of record and the // filename of the cluster file. std::string ClusterConnectionFile::toString() const { - return "File: " + filename; + return "file://" + filename; } // returns @@ -158,7 +158,7 @@ Future ClusterConnectionFile::persist() { // ultimately be written TraceEvent(SevWarnAlways, "ClusterFileChangedAfterReplace") .detail("Filename", filename) - .detail("ConnStr", cs.toString()); + .detail("ConnectionString", cs.toString()); return false; } @@ -167,7 +167,7 @@ Future ClusterConnectionFile::persist() { TraceEvent(SevWarnAlways, "UnableToChangeConnectionFile") .error(e) .detail("Filename", filename) - .detail("ConnStr", cs.toString()); + .detail("ConnectionString", cs.toString()); } } diff --git a/fdbclient/ClusterConnectionKey.actor.cpp b/fdbclient/ClusterConnectionKey.actor.cpp index bf08e42b24..0a29d9d075 100644 --- a/fdbclient/ClusterConnectionKey.actor.cpp +++ b/fdbclient/ClusterConnectionKey.actor.cpp @@ -28,7 +28,11 @@ ClusterConnectionKey::ClusterConnectionKey(Database db, Key connectionStringKey, ClusterConnectionString const& contents, ConnectionStringNeedsPersisted needsToBePersisted) - : IClusterConnectionRecord(needsToBePersisted), db(db), cs(contents), connectionStringKey(connectionStringKey) {} + : IClusterConnectionRecord(needsToBePersisted), db(db), cs(contents), connectionStringKey(connectionStringKey) { + if (!needsToBePersisted) { + lastPersistedConnectionString = ValueRef(contents.toString()); + } +} // Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the // format is invalid. @@ -112,18 +116,41 @@ Reference ClusterConnectionKey::makeIntermediateRecord // Returns a string representation of this cluster connection record. This will include the type of record and the // key where the record is stored. std::string ClusterConnectionKey::toString() const { - return "Key: " + printable(connectionStringKey); + return "fdbkey://" + printable(connectionStringKey); } ACTOR Future ClusterConnectionKey::persistImpl(Reference self) { self->setPersisted(); + state Value newConnectionString = ValueRef(self->cs.toString()); try { state Transaction tr(self->db); loop { try { - tr.set(self->connectionStringKey, StringRef(self->cs.toString())); + Optional existingConnectionString = wait(tr.get(self->connectionStringKey)); + // Someone has already updated the connection string to what we want + if (existingConnectionString.present() && existingConnectionString.get() == newConnectionString) { + self->lastPersistedConnectionString = newConnectionString; + return true; + } + // Someone has updated the connection string to something we didn't expect, in which case we leave it + // alone. It's possible this could result in the stored string getting stuck if the connection string + // changes twice and only the first change is recorded. If the process that wrote the first change dies + // and no other process attempts to write the intermediate state, then only a newly opened connection + // key would be able to update the state. + else if (existingConnectionString.present() && + existingConnectionString != self->lastPersistedConnectionString) { + TraceEvent(SevWarnAlways, "UnableToChangeConnectionKeyDueToMismatch") + .detail("ConnectionKey", self->connectionStringKey) + .detail("NewConnectionString", newConnectionString) + .detail("ExpectedStoredConnectionString", self->lastPersistedConnectionString) + .detail("ActualStoredConnectionString", existingConnectionString); + return false; + } + tr.set(self->connectionStringKey, newConnectionString); wait(tr.commit()); + + self->lastPersistedConnectionString = newConnectionString; return true; } catch (Error& e) { wait(tr.onError(e)); @@ -133,7 +160,7 @@ ACTOR Future ClusterConnectionKey::persistImpl(ReferenceconnectionStringKey) - .detail("ConnStr", self->cs.toString()); + .detail("ConnectionString", self->cs.toString()); } return false; diff --git a/fdbclient/ClusterConnectionKey.actor.h b/fdbclient/ClusterConnectionKey.actor.h index 5417a2fda6..e60ebf185a 100644 --- a/fdbclient/ClusterConnectionKey.actor.h +++ b/fdbclient/ClusterConnectionKey.actor.h @@ -90,6 +90,7 @@ private: Database db; ClusterConnectionString cs; Key connectionStringKey; + Optional lastPersistedConnectionString; }; #include "flow/unactorcompiler.h" diff --git a/fdbclient/ClusterConnectionMemoryRecord.actor.cpp b/fdbclient/ClusterConnectionMemoryRecord.actor.cpp index 1a50213bda..b3afc0ef96 100644 --- a/fdbclient/ClusterConnectionMemoryRecord.actor.cpp +++ b/fdbclient/ClusterConnectionMemoryRecord.actor.cpp @@ -59,7 +59,7 @@ Reference ClusterConnectionMemoryRecord::makeIntermedi // Returns a string representation of this cluster connection record. This will include the type and id of the // record. std::string ClusterConnectionMemoryRecord::toString() const { - return "Memory: " + id.toString(); + return "memory://" + id.toString(); } // This is a no-op for memory records. Returns true to indicate success. From 020b02ea4c2af1e28d6087d36c84316d817fb5f8 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Thu, 21 Oct 2021 13:58:17 -0700 Subject: [PATCH 35/42] Add a comment about the limitations of the URI-based encoding. --- fdbclient/ClusterConnectionFile.actor.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fdbclient/ClusterConnectionFile.actor.cpp b/fdbclient/ClusterConnectionFile.actor.cpp index f17674184a..82431de62e 100644 --- a/fdbclient/ClusterConnectionFile.actor.cpp +++ b/fdbclient/ClusterConnectionFile.actor.cpp @@ -93,6 +93,9 @@ Reference ClusterConnectionFile::makeIntermediateRecor // Returns a string representation of this cluster connection record. This will include the type of record and the // filename of the cluster file. std::string ClusterConnectionFile::toString() const { + // This is a fairly naive attempt to generate a URI-like string. It will not account for characters like spaces, it + // may use backslashes in windows paths, etc. + // SOMEDAY: we should encode this string as a proper URI. return "file://" + filename; } From 4039dbd8da953623b135364739950280d21ddf5b Mon Sep 17 00:00:00 2001 From: Evan Tschannen Date: Sun, 24 Oct 2021 13:46:45 -0700 Subject: [PATCH 36/42] fix: combine mutations from the same version into a single MutationsAndVesionRef --- fdbclient/NativeAPI.actor.cpp | 7 ++++++- fdbclient/StorageServerInterface.h | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index ce15e08214..6b9a56e844 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -6746,7 +6746,12 @@ ACTOR Future mergeChangeFeedStream(std::vector res = waitNext(nextStream.results.getFuture()); nextStream.next = res; diff --git a/fdbclient/StorageServerInterface.h b/fdbclient/StorageServerInterface.h index c47f985a0b..ac8e9c344f 100644 --- a/fdbclient/StorageServerInterface.h +++ b/fdbclient/StorageServerInterface.h @@ -637,8 +637,8 @@ struct SplitRangeRequest { struct MutationsAndVersionRef { VectorRef mutations; - Version version; - Version knownCommittedVersion; + Version version = invalidVersion; + Version knownCommittedVersion = invalidVersion; MutationsAndVersionRef() {} explicit MutationsAndVersionRef(Version version, Version knownCommittedVersion) From 0e327d3d0a2141a09cf593a90720f0d004f69d61 Mon Sep 17 00:00:00 2001 From: Evan Tschannen Date: Sun, 24 Oct 2021 19:17:11 -0700 Subject: [PATCH 37/42] fix: do not duplicate lastEpochEnd from different servers --- fdbclient/NativeAPI.actor.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fdbclient/NativeAPI.actor.cpp b/fdbclient/NativeAPI.actor.cpp index 6b9a56e844..73f276fa7e 100644 --- a/fdbclient/NativeAPI.actor.cpp +++ b/fdbclient/NativeAPI.actor.cpp @@ -6747,8 +6747,11 @@ ACTOR Future mergeChangeFeedStream(std::vector Date: Sun, 24 Oct 2021 19:18:03 -0700 Subject: [PATCH 38/42] fix: disconnectTriggers cannot be yielded because we could send a reply to a replyPromiseStream while waiting on the delay --- fdbrpc/FailureMonitor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdbrpc/FailureMonitor.h b/fdbrpc/FailureMonitor.h index 04154fa103..795a6e9953 100644 --- a/fdbrpc/FailureMonitor.h +++ b/fdbrpc/FailureMonitor.h @@ -157,7 +157,7 @@ public: private: std::unordered_map addressStatus; YieldedAsyncMap endpointKnownFailed; - YieldedAsyncMap disconnectTriggers; + AsyncMap disconnectTriggers; std::unordered_set failedEndpoints; friend class OnStateChangedActorActor; From 9a6384fc266eefe9f1893eaffc51778621238481 Mon Sep 17 00:00:00 2001 From: Evan Tschannen Date: Sun, 24 Oct 2021 21:18:49 -0700 Subject: [PATCH 39/42] fixed merge conflicts --- fdbserver/storageserver.actor.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index 6502c75b1e..dd01146a54 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -892,8 +892,8 @@ public: fetchKeysParallelismLock(SERVER_KNOBS->FETCH_KEYS_PARALLELISM), fetchKeysBytesBudget(SERVER_KNOBS->STORAGE_FETCH_BYTES), fetchKeysBudgetUsed(false), instanceID(deterministicRandom()->randomUniqueID().first()), shuttingDown(false), behind(false), - versionBehind(false), debug_inApplyUpdate(false), debug_lastValidateTime(0), maxQueryQueue(0), - lastBytesInputEBrake(0), lastDurableVersionEBrake(0), transactionTagCounter(ssi.id()), counters(this), + versionBehind(false), debug_inApplyUpdate(false), debug_lastValidateTime(0), lastBytesInputEBrake(0), + lastDurableVersionEBrake(0), maxQueryQueue(0), transactionTagCounter(ssi.id()), counters(this), storageServerSourceTLogIDEventHolder( makeReference(ssi.id().toString() + "/StorageServerSourceTLogID")) { version.initMetric(LiteralStringRef("StorageServer.Version"), counters.cc.id); @@ -4011,7 +4011,7 @@ void changeServerKeys(StorageServer* data, // Clear the moving-in empty range, and set it available at the latestVersion. for (const auto& range : newEmptyRanges) { MutationRef clearRange(MutationRef::ClearRange, range.begin, range.end); - data->addMutation(data->data().getLatestVersion(), clearRange, range, data->updateEagerReads); + data->addMutation(data->data().getLatestVersion(), true, clearRange, range, data->updateEagerReads); data->newestAvailableVersion.insert(range, latestVersion); setAvailableStatus(data, range, true); } From 118c307b571163f3bcf4e391a6ea31c028f56082 Mon Sep 17 00:00:00 2001 From: Evan Tschannen Date: Sun, 24 Oct 2021 22:26:11 -0700 Subject: [PATCH 40/42] fixed formatting --- fdbserver/storageserver.actor.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index dd01146a54..bacbbb165d 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -1832,8 +1832,7 @@ ACTOR Future localChangeFeedStream(StorageServer* data, } } -ACTOR Future changeFeedStreamQ(StorageServer* data, ChangeFeedStreamRequest req) -{ +ACTOR Future changeFeedStreamQ(StorageServer* data, ChangeFeedStreamRequest req) { state Span span("SS:getChangeFeedStream"_loc, { req.spanContext }); req.reply.setByteLimit(SERVER_KNOBS->RANGESTREAM_LIMIT_BYTES); From a8b6f25b3eb2b74421caa210956460dd6e7bf6db Mon Sep 17 00:00:00 2001 From: Josh Slocum Date: Fri, 22 Oct 2021 16:22:05 -0500 Subject: [PATCH 41/42] Fix ss initialization order --- fdbserver/storageserver.actor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdbserver/storageserver.actor.cpp b/fdbserver/storageserver.actor.cpp index c2a1faf577..752c6c410d 100644 --- a/fdbserver/storageserver.actor.cpp +++ b/fdbserver/storageserver.actor.cpp @@ -871,8 +871,8 @@ public: fetchKeysParallelismLock(SERVER_KNOBS->FETCH_KEYS_PARALLELISM), fetchKeysBytesBudget(SERVER_KNOBS->STORAGE_FETCH_BYTES), fetchKeysBudgetUsed(false), instanceID(deterministicRandom()->randomUniqueID().first()), shuttingDown(false), behind(false), - versionBehind(false), debug_inApplyUpdate(false), debug_lastValidateTime(0), maxQueryQueue(0), - lastBytesInputEBrake(0), lastDurableVersionEBrake(0), transactionTagCounter(ssi.id()), counters(this), + versionBehind(false), debug_inApplyUpdate(false), debug_lastValidateTime(0), lastBytesInputEBrake(0), + lastDurableVersionEBrake(0), maxQueryQueue(0), transactionTagCounter(ssi.id()), counters(this), storageServerSourceTLogIDEventHolder( makeReference(ssi.id().toString() + "/StorageServerSourceTLogID")) { version.initMetric(LiteralStringRef("StorageServer.Version"), counters.cc.id); From aebd12a9aa5848be674d66a8db0eb5617ddb8b19 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 25 Oct 2021 10:36:37 -0700 Subject: [PATCH 42/42] Fix indentation issue with actor compiler (#5828) --- flow/actorcompiler/ActorCompiler.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flow/actorcompiler/ActorCompiler.cs b/flow/actorcompiler/ActorCompiler.cs index 3cf1853ab6..3961ead11b 100644 --- a/flow/actorcompiler/ActorCompiler.cs +++ b/flow/actorcompiler/ActorCompiler.cs @@ -110,7 +110,8 @@ namespace actorcompiler name = name, returnType = returnType, endIsUnreachable = endIsUnreachable, - formalParameters = formalParameters + formalParameters = formalParameters, + indentation = indentation } ); }