From cd676da0cc934ac17b81b3cd520868d02bed9469 Mon Sep 17 00:00:00 2001 From: Yuval Ariel Date: Mon, 3 Feb 2025 08:32:13 +0200 Subject: [PATCH 1/6] Add option to print all histograms instead of only BEST and WORST This functionallity is needed since sometimes theres a need to see a specific run's results. Controlled by a new flag: --print-all-hists yes --- memtier_benchmark.cpp | 60 ++++++++++++++++++++++++++++--------------- memtier_benchmark.h | 1 + 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/memtier_benchmark.cpp b/memtier_benchmark.cpp index 61be983d..6074fa8b 100755 --- a/memtier_benchmark.cpp +++ b/memtier_benchmark.cpp @@ -159,7 +159,8 @@ static void config_print(FILE *file, struct benchmark_config *cfg) "wait-ratio = %u:%u\n" "num-slaves = %u-%u\n" "wait-timeout = %u-%u\n" - "json-out-file = %s\n", + "json-out-file = %s\n" + "print-all-hists = %s\n", cfg->server, cfg->port, cfg->unix_socket, @@ -209,7 +210,8 @@ static void config_print(FILE *file, struct benchmark_config *cfg) cfg->wait_ratio.a, cfg->wait_ratio.b, cfg->num_slaves.min, cfg->num_slaves.max, cfg->wait_timeout.min, cfg->wait_timeout.max, - cfg->json_out_file); + cfg->json_out_file, + cfg->print_all_hists ? "yes" : "no"); } static void config_print_to_json(json_handler * jsonhandler, struct benchmark_config *cfg) @@ -267,6 +269,7 @@ static void config_print_to_json(json_handler * jsonhandler, struct benchmark_co jsonhandler->write_obj("wait-ratio" ,"\"%u:%u\"", cfg->wait_ratio.a, cfg->wait_ratio.b); jsonhandler->write_obj("num-slaves" ,"\"%u:%u\"", cfg->num_slaves.min, cfg->num_slaves.max); jsonhandler->write_obj("wait-timeout" ,"\"%u-%u\"", cfg->wait_timeout.min, cfg->wait_timeout.max); + jsonhandler->write_obj("print-all-hists" ,"\"%s\"", cfg->print_all_hists ? "true" : "false"); jsonhandler->close_nesting(); } @@ -403,6 +406,7 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf o_show_config, o_hide_histogram, o_print_percentiles, + o_print_all_hists, o_distinct_client_seed, o_randomize, o_client_stats, @@ -456,6 +460,7 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf { "show-config", 0, 0, o_show_config }, { "hide-histogram", 0, 0, o_hide_histogram }, { "print-percentiles", 1, 0, o_print_percentiles }, + { "print-all-hists", 0, 0, o_print_all_hists }, { "distinct-client-seed", 0, 0, o_distinct_client_seed }, { "randomize", 0, 0, o_randomize }, { "requests", 1, 0, 'n' }, @@ -587,6 +592,9 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf return -1; } break; + case o_print_all_hists: + cfg->print_all_hists = true; + break; case o_distinct_client_seed: cfg->distinct_client_seed++; break; @@ -977,6 +985,7 @@ void usage() { " --show-config Print detailed configuration before running\n" " --hide-histogram Don't print detailed latency histogram\n" " --print-percentiles Specify which percentiles info to print on the results table (by default prints percentiles: 50,99,99.9)\n" + " --print-all-hists When performing multiple test iterations, print histograms for all iterations\n" " --cluster-mode Run client in cluster mode\n" " -h, --help Display this help\n" " -v, --version Display version information\n" @@ -1651,29 +1660,38 @@ int main(int argc, char *argv[]) jsonhandler->close_nesting(); } - // If more than 1 run was used, compute best, worst and average + // If more than 1 run was used display appropriate info if (cfg.run_count > 1) { - unsigned int min_ops_sec = (unsigned int) -1; - unsigned int max_ops_sec = 0; - run_stats* worst = NULL; - run_stats* best = NULL; - for (std::vector::iterator i = all_stats.begin(); i != all_stats.end(); i++) { - unsigned long usecs = i->get_duration_usec(); - unsigned int ops_sec = (int)(((double)i->get_total_ops() / (usecs > 0 ? usecs : 1)) * 1000000); - if (ops_sec < min_ops_sec || worst == NULL) { - min_ops_sec = ops_sec; - worst = &(*i); + // User wants the best and worst + if (!cfg.print_all_hists) { + unsigned int min_ops_sec = (unsigned int) -1; + unsigned int max_ops_sec = 0; + run_stats* worst = NULL; + run_stats* best = NULL; + for (std::vector::iterator i = all_stats.begin(); i != all_stats.end(); i++) { + unsigned long usecs = i->get_duration_usec(); + unsigned int ops_sec = (int)(((double)i->get_total_ops() / (usecs > 0 ? usecs : 1)) * 1000000); + if (ops_sec < min_ops_sec || worst == NULL) { + min_ops_sec = ops_sec; + worst = &(*i); + } + if (ops_sec > max_ops_sec || best == NULL) { + max_ops_sec = ops_sec; + best = &(*i); + } } - if (ops_sec > max_ops_sec || best == NULL) { - max_ops_sec = ops_sec; - best = &(*i); + + // Best results: + best->print(outfile, &cfg, "BEST RUN RESULTS", jsonhandler); + // worst results: + worst->print(outfile, &cfg, "WORST RUN RESULTS", jsonhandler); + // User wants to see a separate histogram per run + } else { + for (auto i = 0U; i < all_stats.size(); i++) { + auto run_title = std::string("RUN #") + std::to_string(i + 1) + " RESULTS"; + all_stats[i].print(outfile, &cfg, run_title.c_str(), jsonhandler); } } - - // Best results: - best->print(outfile, &cfg, "BEST RUN RESULTS", jsonhandler); - // worst results: - worst->print(outfile, &cfg, "WORST RUN RESULTS", jsonhandler); // average results: run_stats average(&cfg); average.aggregate_average(all_stats); diff --git a/memtier_benchmark.h b/memtier_benchmark.h index b7a71903..f753eb9b 100644 --- a/memtier_benchmark.h +++ b/memtier_benchmark.h @@ -63,6 +63,7 @@ struct benchmark_config { int show_config; int hide_histogram; config_quantiles print_percentiles; + bool print_all_hists; int distinct_client_seed; int randomize; int next_client_idx; From f96e5322ba9fece72dfa94de8aa83b328a893395 Mon Sep 17 00:00:00 2001 From: "Filipe Oliveira (Personal)" Date: Tue, 11 Feb 2025 14:22:38 +0000 Subject: [PATCH 2/6] Fixed release action steps with minimal changes (#299) --- .github/workflows/release.yml | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fcea488e..dcd9e23e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,9 +17,15 @@ on: description: "Docker images for smoke testing (comma-separated, e.g., ubuntu:20.04,ubuntu:22.04,ubuntu:24.04)" required: false default: "ubuntu:20.04,ubuntu:22.04,ubuntu:24.04" + build_runner: + description: "os in which build steps run on" + required: false + default: "ubuntu-22.04" + type: string jobs: build-source-package: - runs-on: ubuntu-latest + runs-on: ${{ inputs.build_runner }} + continue-on-error: true strategy: matrix: dist: ${{ fromJSON(vars.BUILD_DISTS) }} @@ -27,6 +33,24 @@ jobs: - uses: actions/checkout@v4 with: path: sources + - name: Validate configure.ac version matches GitHub Release (only on release) + if: github.event.release.tag_name != '' + env: + VERSION: ${{ github.event.release.tag_name }} + run: | + # Extract the current version from configure.ac + CURRENT_VERSION=$(awk -F'[(),]' '/AC_INIT/ {print $3}' sources/configure.ac | tr -d ' ') + + echo "Current configure.ac version: $CURRENT_VERSION" + echo "GitHub Release version: $VERSION" + + # Check if versions match + if [ "$CURRENT_VERSION" != "$VERSION" ]; then + echo "❌ Version mismatch! configure.ac: $CURRENT_VERSION, GitHub Release: $VERSION" + exit 1 # Fail the build + else + echo "Version match. Proceeding with the build." + fi - name: Install dependencies run: | sudo apt-get update && \ @@ -63,7 +87,8 @@ jobs: memtier-benchmark_*.tar.* build-binary-package: - runs-on: ubuntu-latest + runs-on: ${{ inputs.build_runner }} + continue-on-error: true environment: build strategy: matrix: @@ -121,7 +146,7 @@ jobs: *.deb smoke-test-packages: - runs-on: ubuntu-latest + runs-on: ${{ inputs.build_runner }} needs: build-binary-package env: ARCH: amd64 From 161d9de80af77eaa8602d49783807eb2707c39b8 Mon Sep 17 00:00:00 2001 From: "Filipe Oliveira (Personal)" Date: Tue, 11 Feb 2025 14:47:03 +0000 Subject: [PATCH 3/6] using ubuntu-22.04 on runner of release (#300) --- .github/workflows/release.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index dcd9e23e..840e1af1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -24,7 +24,7 @@ on: type: string jobs: build-source-package: - runs-on: ${{ inputs.build_runner }} + runs-on: ubuntu-22.04 continue-on-error: true strategy: matrix: @@ -87,7 +87,7 @@ jobs: memtier-benchmark_*.tar.* build-binary-package: - runs-on: ${{ inputs.build_runner }} + runs-on: ubuntu-22.04 continue-on-error: true environment: build strategy: @@ -146,7 +146,7 @@ jobs: *.deb smoke-test-packages: - runs-on: ${{ inputs.build_runner }} + runs-on: ubuntu-22.04 needs: build-binary-package env: ARCH: amd64 @@ -187,7 +187,7 @@ jobs: publish-to-apt: env: DEB_S3_VERSION: "0.11.3" - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 environment: build needs: smoke-test-packages steps: From dc13cd1664d3046f65f7bd4201d59dd00bdb9e8e Mon Sep 17 00:00:00 2001 From: "Filipe Oliveira (Personal)" Date: Tue, 11 Feb 2025 16:07:08 +0000 Subject: [PATCH 4/6] Remove ubuntu:bionic logic from smoke-test-packages and removed usage of actions/download-artifact@v3. (#301) --- .github/workflows/release.yml | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 840e1af1..5a575f74 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -150,9 +150,6 @@ jobs: needs: build-binary-package env: ARCH: amd64 - # required by ubuntu:bionic - # https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/ - ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true strategy: matrix: image: ${{ fromJSON(vars.SMOKE_TEST_IMAGES) }} @@ -166,15 +163,7 @@ jobs: exit 1 fi echo "BUILD_ARCH=$BUILD_ARCH" >> $GITHUB_ENV - - name: Get binary packages for ubuntu:bionic - if: matrix.image == 'ubuntu:bionic' - uses: actions/download-artifact@v3 - with: - name: binary-${{ env.BUILD_ARCH }}-${{ env.ARCH }} - path: binary-${{ env.BUILD_ARCH }}-${{ env.ARCH }} - - - name: Get binary packages for other versions - if: matrix.image != 'ubuntu:bionic' + - name: Get binary packages uses: actions/download-artifact@v4 with: name: binary-${{ env.BUILD_ARCH }}-${{ env.ARCH }} From 8d60948a9702356abb601a719f864d26a704e8e9 Mon Sep 17 00:00:00 2001 From: filipecosta90 Date: Fri, 14 Feb 2025 12:22:58 +0000 Subject: [PATCH 5/6] Renamed --print-all-hists to --print-all-runs. Ensured the change is retro-compatible. Added test to confirm it. --- memtier_benchmark.cpp | 65 +++++++++++++++++----------------- memtier_benchmark.h | 2 +- tests/tests_oss_simple_flow.py | 45 +++++++++++++++++++++++ 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/memtier_benchmark.cpp b/memtier_benchmark.cpp index 6074fa8b..6037b182 100755 --- a/memtier_benchmark.cpp +++ b/memtier_benchmark.cpp @@ -160,7 +160,7 @@ static void config_print(FILE *file, struct benchmark_config *cfg) "num-slaves = %u-%u\n" "wait-timeout = %u-%u\n" "json-out-file = %s\n" - "print-all-hists = %s\n", + "print-all-runs = %s\n", cfg->server, cfg->port, cfg->unix_socket, @@ -211,7 +211,7 @@ static void config_print(FILE *file, struct benchmark_config *cfg) cfg->num_slaves.min, cfg->num_slaves.max, cfg->wait_timeout.min, cfg->wait_timeout.max, cfg->json_out_file, - cfg->print_all_hists ? "yes" : "no"); + cfg->print_all_runs ? "yes" : "no"); } static void config_print_to_json(json_handler * jsonhandler, struct benchmark_config *cfg) @@ -269,7 +269,7 @@ static void config_print_to_json(json_handler * jsonhandler, struct benchmark_co jsonhandler->write_obj("wait-ratio" ,"\"%u:%u\"", cfg->wait_ratio.a, cfg->wait_ratio.b); jsonhandler->write_obj("num-slaves" ,"\"%u:%u\"", cfg->num_slaves.min, cfg->num_slaves.max); jsonhandler->write_obj("wait-timeout" ,"\"%u-%u\"", cfg->wait_timeout.min, cfg->wait_timeout.max); - jsonhandler->write_obj("print-all-hists" ,"\"%s\"", cfg->print_all_hists ? "true" : "false"); + jsonhandler->write_obj("print-all-runs" ,"\"%s\"", cfg->print_all_runs ? "true" : "false"); jsonhandler->close_nesting(); } @@ -406,7 +406,7 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf o_show_config, o_hide_histogram, o_print_percentiles, - o_print_all_hists, + o_print_all_runs, o_distinct_client_seed, o_randomize, o_client_stats, @@ -460,7 +460,7 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf { "show-config", 0, 0, o_show_config }, { "hide-histogram", 0, 0, o_hide_histogram }, { "print-percentiles", 1, 0, o_print_percentiles }, - { "print-all-hists", 0, 0, o_print_all_hists }, + { "print-all-runs", 0, 0, o_print_all_runs }, { "distinct-client-seed", 0, 0, o_distinct_client_seed }, { "randomize", 0, 0, o_randomize }, { "requests", 1, 0, 'n' }, @@ -592,8 +592,8 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf return -1; } break; - case o_print_all_hists: - cfg->print_all_hists = true; + case o_print_all_runs: + cfg->print_all_runs = true; break; case o_distinct_client_seed: cfg->distinct_client_seed++; @@ -985,7 +985,7 @@ void usage() { " --show-config Print detailed configuration before running\n" " --hide-histogram Don't print detailed latency histogram\n" " --print-percentiles Specify which percentiles info to print on the results table (by default prints percentiles: 50,99,99.9)\n" - " --print-all-hists When performing multiple test iterations, print histograms for all iterations\n" + " --print-all-runs When performing multiple test iterations, print and save results for all iterations\n" " --cluster-mode Run client in cluster mode\n" " -h, --help Display this help\n" " -v, --version Display version information\n" @@ -1660,38 +1660,37 @@ int main(int argc, char *argv[]) jsonhandler->close_nesting(); } - // If more than 1 run was used display appropriate info + // If more than 1 run was used, compute best, worst and average + // Furthermore, if print_all_runs is enabled we save separate histograms per run if (cfg.run_count > 1) { - // User wants the best and worst - if (!cfg.print_all_hists) { - unsigned int min_ops_sec = (unsigned int) -1; - unsigned int max_ops_sec = 0; - run_stats* worst = NULL; - run_stats* best = NULL; - for (std::vector::iterator i = all_stats.begin(); i != all_stats.end(); i++) { - unsigned long usecs = i->get_duration_usec(); - unsigned int ops_sec = (int)(((double)i->get_total_ops() / (usecs > 0 ? usecs : 1)) * 1000000); - if (ops_sec < min_ops_sec || worst == NULL) { - min_ops_sec = ops_sec; - worst = &(*i); - } - if (ops_sec > max_ops_sec || best == NULL) { - max_ops_sec = ops_sec; - best = &(*i); - } - } - - // Best results: - best->print(outfile, &cfg, "BEST RUN RESULTS", jsonhandler); - // worst results: - worst->print(outfile, &cfg, "WORST RUN RESULTS", jsonhandler); // User wants to see a separate histogram per run - } else { + if (cfg.print_all_runs) { for (auto i = 0U; i < all_stats.size(); i++) { auto run_title = std::string("RUN #") + std::to_string(i + 1) + " RESULTS"; all_stats[i].print(outfile, &cfg, run_title.c_str(), jsonhandler); } } + // User wants the best and worst + unsigned int min_ops_sec = (unsigned int) -1; + unsigned int max_ops_sec = 0; + run_stats* worst = NULL; + run_stats* best = NULL; + for (std::vector::iterator i = all_stats.begin(); i != all_stats.end(); i++) { + unsigned long usecs = i->get_duration_usec(); + unsigned int ops_sec = (int)(((double)i->get_total_ops() / (usecs > 0 ? usecs : 1)) * 1000000); + if (ops_sec < min_ops_sec || worst == NULL) { + min_ops_sec = ops_sec; + worst = &(*i); + } + if (ops_sec > max_ops_sec || best == NULL) { + max_ops_sec = ops_sec; + best = &(*i); + } + } + // Best results: + best->print(outfile, &cfg, "BEST RUN RESULTS", jsonhandler); + // worst results: + worst->print(outfile, &cfg, "WORST RUN RESULTS", jsonhandler); // average results: run_stats average(&cfg); average.aggregate_average(all_stats); diff --git a/memtier_benchmark.h b/memtier_benchmark.h index f753eb9b..3192cf71 100644 --- a/memtier_benchmark.h +++ b/memtier_benchmark.h @@ -63,7 +63,7 @@ struct benchmark_config { int show_config; int hide_histogram; config_quantiles print_percentiles; - bool print_all_hists; + bool print_all_runs; int distinct_client_seed; int randomize; int next_client_idx; diff --git a/tests/tests_oss_simple_flow.py b/tests/tests_oss_simple_flow.py index 2c276bc6..883ceb2c 100644 --- a/tests/tests_oss_simple_flow.py +++ b/tests/tests_oss_simple_flow.py @@ -407,6 +407,51 @@ def test_default_set_get_3_runs(env): assert_minimum_memtier_outcomes(config, env, memtier_ok, overall_expected_request_count, overall_request_count) + +# run each test on different env +def test_print_all_runs(env): + run_count = 5 + benchmark_specs = {"name": env.testName, "args": ['--print-all-runs','--run-count={}'.format(run_count)]} + addTLSArgs(benchmark_specs, env) + config = get_default_memtier_config() + master_nodes_list = env.getMasterNodesList() + overall_expected_request_count = get_expected_request_count(config) * run_count + + add_required_env_arguments(benchmark_specs, config, env, master_nodes_list) + + # Create a temporary directory + test_dir = tempfile.mkdtemp() + + config = RunConfig(test_dir, env.testName, config, {}) + ensure_clean_benchmark_folder(config.results_dir) + + benchmark = Benchmark.from_json(config, benchmark_specs) + + # benchmark.run() returns True if the return code of memtier_benchmark was 0 + memtier_ok = benchmark.run() + + master_nodes_connections = env.getOSSMasterNodesConnectionList() + merged_command_stats = {'cmdstat_set': {'calls': 0}, 'cmdstat_get': {'calls': 0}} + overall_request_count = agg_info_commandstats(master_nodes_connections, merged_command_stats) + assert_minimum_memtier_outcomes(config, env, memtier_ok, overall_expected_request_count, overall_request_count) + + json_filename = '{0}/mb.json'.format(config.results_dir) + ## Assert that all BW metrics are properly stored and calculated + with open(json_filename) as results_json: + results_dict = json.load(results_json) + print_all_runs = results_dict["configuration"]["print-all-runs"] + env.assertTrue(print_all_runs) + for run_count in range(1, run_count+1): + # assert the run infomation exists + env.assertTrue(f"RUN # {run_count} RESULTS" in results_dict) + + # ensure best, worst, and aggregate results are present + env.assertTrue("BEST RUN RESULTS" in results_dict) + env.assertTrue("WORST RUN RESULTS" in results_dict) + env.assertTrue(f"AGGREGATED AVERAGE RESULTS ({run_count} runs)" in results_dict) + # all stats should only exist on a single run json + env.assertTrue("ALL STATS" not in results_dict) + def test_default_arbitrary_command_pubsub(env): benchmark_specs = {"name": env.testName, "args": []} addTLSArgs(benchmark_specs, env) From 88a637b17a922f042189d64173e8e0b2f8e1937f Mon Sep 17 00:00:00 2001 From: filipecosta90 Date: Fri, 14 Feb 2025 12:29:31 +0000 Subject: [PATCH 6/6] Fixed extra space on run check --- tests/tests_oss_simple_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_oss_simple_flow.py b/tests/tests_oss_simple_flow.py index 883ceb2c..5205960c 100644 --- a/tests/tests_oss_simple_flow.py +++ b/tests/tests_oss_simple_flow.py @@ -443,7 +443,7 @@ def test_print_all_runs(env): env.assertTrue(print_all_runs) for run_count in range(1, run_count+1): # assert the run infomation exists - env.assertTrue(f"RUN # {run_count} RESULTS" in results_dict) + env.assertTrue(f"RUN #{run_count} RESULTS" in results_dict) # ensure best, worst, and aggregate results are present env.assertTrue("BEST RUN RESULTS" in results_dict)