From 7ba5063b7a6c80ea07ec30b473e329bf11c93879 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 12 Jun 2020 00:24:56 +0300 Subject: [PATCH 01/13] Add concurrent benchmark to performance test After the main test, run queries from `website.xml` in parallel using `clickhouse-benchmark`. This can be useful to test the effects of concurrency on performance. Comparison test can miss some effects because it always runs queries sequentially, and many of them are even single-threaded. --- docker/test/performance-comparison/compare.sh | 17 +++ docker/test/performance-comparison/perf.py | 100 ++++++++---------- docker/test/performance-comparison/report.py | 31 ++++++ 3 files changed, 90 insertions(+), 58 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index a2760907cb3..3d49e9e841a 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -161,6 +161,20 @@ function run_tests wait } +# Run some queries concurrently and report the resulting TPS. This additional +# (relatively) short test helps detect concurrency-related effects, because the +# main performance comparison testing is done query-by-query. +function run_benchmark +{ + rm -rf benchmark ||: + mkdir bencmhark ||: + + # TODO disable this when there is an explicit list of tests to run + "$script_dir/perf.py" --print right/performance/website.xml > benchmark/website-queries.tsv + clickhouse-benchmark --port 9001 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-left.json < benchmark/website-queries.tsv + clickhouse-benchmark --port 9002 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-right.json < benchmark/website-queries.tsv +} + function get_profiles_watchdog { sleep 6000 @@ -716,6 +730,9 @@ case "$stage" in # Ignore the errors to collect the log and build at least some report, anyway time run_tests ||: ;& +"run_benchmark") + time run_benchmark 2> >(tee -a run-errors.tsv 1>&2) ||: + ;& "get_profiles") # Getting profiles inexplicably hangs sometimes, so try to save some logs if # this happens again. Give the servers some time to collect all info, then diff --git a/docker/test/performance-comparison/perf.py b/docker/test/performance-comparison/perf.py index 308d4760b48..74d0300b074 100755 --- a/docker/test/performance-comparison/perf.py +++ b/docker/test/performance-comparison/perf.py @@ -14,22 +14,14 @@ import traceback def tsv_escape(s): return s.replace('\\', '\\\\').replace('\t', '\\t').replace('\n', '\\n').replace('\r','') -stage_start_seconds = time.perf_counter() - -def report_stage_end(stage_name): - global stage_start_seconds - print('{}\t{}'.format(stage_name, time.perf_counter() - stage_start_seconds)) - stage_start_seconds = time.perf_counter() - -report_stage_end('start') - parser = argparse.ArgumentParser(description='Run performance test.') # Explicitly decode files as UTF-8 because sometimes we have Russian characters in queries, and LANG=C is set. parser.add_argument('file', metavar='FILE', type=argparse.FileType('r', encoding='utf-8'), nargs=1, help='test description file') parser.add_argument('--host', nargs='*', default=['localhost'], help="Server hostname(s). Corresponds to '--port' options.") parser.add_argument('--port', nargs='*', default=[9000], help="Server port(s). Corresponds to '--host' options.") -parser.add_argument('--runs', type=int, default=int(os.environ.get('CHPC_RUNS', 13)), help='Number of query runs per server. Defaults to CHPC_RUNS environment variable.') -parser.add_argument('--no-long', type=bool, default=True, help='Skip the tests tagged as long.') +parser.add_argument('--runs', type=int, default=int(os.environ.get('CHPC_RUNS', 17)), help='Number of query runs per server. Defaults to CHPC_RUNS environment variable.') +parser.add_argument('--long', action='store_true', help='Do not skip the tests tagged as long.') +parser.add_argument('--print', action='store_true', help='Print test queries and exit.') args = parser.parse_args() test_name = os.path.splitext(os.path.basename(args.file[0].name))[0] @@ -37,35 +29,6 @@ test_name = os.path.splitext(os.path.basename(args.file[0].name))[0] tree = et.parse(args.file[0]) root = tree.getroot() -# Skip long tests -for tag in root.findall('.//tag'): - if tag.text == 'long': - print('skipped\tTest is tagged as long.') - sys.exit(0) - -# Check main metric -main_metric_element = root.find('main_metric/*') -if main_metric_element is not None and main_metric_element.tag != 'min_time': - raise Exception('Only the min_time main metric is supported. This test uses \'{}\''.format(main_metric_element.tag)) - -# FIXME another way to detect infinite tests. They should have an appropriate main_metric but sometimes they don't. -infinite_sign = root.find('.//average_speed_not_changing_for_ms') -if infinite_sign is not None: - raise Exception('Looks like the test is infinite (sign 1)') - -# Print report threshold for the test if it is set. -if 'max_ignored_relative_change' in root.attrib: - print(f'report-threshold\t{root.attrib["max_ignored_relative_change"]}') - -# Open connections -servers = [{'host': host, 'port': port} for (host, port) in zip(args.host, args.port)] -connections = [clickhouse_driver.Client(**server) for server in servers] - -for s in servers: - print('server\t{}\t{}'.format(s['host'], s['port'])) - -report_stage_end('connect') - # Process query parameters subst_elems = root.findall('substitutions/substitution') available_parameters = {} # { 'table': ['hits_10m', 'hits_100m'], ... } @@ -84,7 +47,45 @@ def substitute_parameters(query_templates): for values_combo in itertools.product(*values)]) return result -report_stage_end('substitute') +# Build a list of test queries, processing all substitutions +test_query_templates = [q.text for q in root.findall('query')] +test_queries = substitute_parameters(test_query_templates) + +# If we're only asked to print the queries, do that and exit +if args.print: + for q in test_queries: + print(q) + exit(0) + +# Skip long tests +if not args.long: + for tag in root.findall('.//tag'): + if tag.text == 'long': + print('skipped\tTest is tagged as long.') + sys.exit(0) + +# Check main metric to detect infinite tests. We shouldn't have such tests anymore, +# but we did in the past, and it is convenient to be able to process old tests. +main_metric_element = root.find('main_metric/*') +if main_metric_element is not None and main_metric_element.tag != 'min_time': + raise Exception('Only the min_time main metric is supported. This test uses \'{}\''.format(main_metric_element.tag)) + +# Another way to detect infinite tests. They should have an appropriate main_metric +# but sometimes they don't. +infinite_sign = root.find('.//average_speed_not_changing_for_ms') +if infinite_sign is not None: + raise Exception('Looks like the test is infinite (sign 1)') + +# Print report threshold for the test if it is set. +if 'max_ignored_relative_change' in root.attrib: + print(f'report-threshold\t{root.attrib["max_ignored_relative_change"]}') + +# Open connections +servers = [{'host': host, 'port': port} for (host, port) in zip(args.host, args.port)] +connections = [clickhouse_driver.Client(**server) for server in servers] + +for s in servers: + print('server\t{}\t{}'.format(s['host'], s['port'])) # Run drop queries, ignoring errors. Do this before all other activity, because # clickhouse_driver disconnects on error (this is not configurable), and the new @@ -98,8 +99,6 @@ for c in connections: except: pass -report_stage_end('drop1') - # Apply settings. # If there are errors, report them and continue -- maybe a new test uses a setting # that is not in master, but the queries can still run. If we have multiple @@ -115,8 +114,6 @@ for c in connections: except: print(traceback.format_exc(), file=sys.stderr) -report_stage_end('settings') - # Check tables that should exist. If they don't exist, just skip this test. tables = [e.text for e in root.findall('preconditions/table_exists')] for t in tables: @@ -129,8 +126,6 @@ for t in tables: print(f'skipped\t{tsv_escape(skipped_message)}') sys.exit(0) -report_stage_end('preconditions') - # Run create queries create_query_templates = [q.text for q in root.findall('create_query')] create_queries = substitute_parameters(create_query_templates) @@ -145,14 +140,7 @@ for c in connections: for q in fill_queries: c.execute(q) -report_stage_end('fill') - # Run test queries -test_query_templates = [q.text for q in root.findall('query')] -test_queries = substitute_parameters(test_query_templates) - -report_stage_end('substitute2') - for query_index, q in enumerate(test_queries): query_prefix = f'{test_name}.query{query_index}' @@ -199,13 +187,9 @@ for query_index, q in enumerate(test_queries): client_seconds = time.perf_counter() - start_seconds print(f'client-time\t{query_index}\t{client_seconds}\t{server_seconds}') -report_stage_end('benchmark') - # Run drop queries drop_query_templates = [q.text for q in root.findall('drop_query')] drop_queries = substitute_parameters(drop_query_templates) for c in connections: for q in drop_queries: c.execute(q) - -report_stage_end('drop2') diff --git a/docker/test/performance-comparison/report.py b/docker/test/performance-comparison/report.py index 9db37932aea..d7e30190aef 100755 --- a/docker/test/performance-comparison/report.py +++ b/docker/test/performance-comparison/report.py @@ -5,6 +5,7 @@ import ast import collections import csv import itertools +import json import os import sys import traceback @@ -321,6 +322,36 @@ if args.report == 'main': print_test_times() + def print_benchmark_results(): + left_json = json.load(open('benchmark/website-left.json')); + right_json = json.load(open('benchmark/website-right.json')); + left_qps = left_json["statistics"]["QPS"] + right_qps = right_json["statistics"]["QPS"] + relative_diff = (right_qps - left_qps) / left_qps; + times_diff = max(right_qps, left_qps) / max(0.01, min(right_qps, left_qps)) + print(tableStart('Concurrent benchmarks')) + print(tableHeader(['Benchmark', 'Old, queries/s', 'New, queries/s', 'Relative difference', 'Times difference'])) + row = ['website', f'{left_qps:.3f}', f'{right_qps:.3f}', f'{relative_diff:.3f}', f'x{times_diff:.3f}'] + attrs = ['' for r in row] + if abs(relative_diff) > 0.1: + # More queries per second is better. + if relative_diff > 0.: + attrs[3] = f'style="background: {color_good}"' + else: + attrs[3] = f'style="background: {color_bad}"' + else: + attrs[3] = '' + print(tableRow(row, attrs)) + print(tableEnd()) + + try: + print_benchmark_results() + except: + report_errors.append( + traceback.format_exception_only( + *sys.exc_info()[:2])[-1]) + pass + print_report_errors() print(""" From 0da6e1c9de6d5cdd7b24e08de815589c6d2d36cc Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 12 Jun 2020 15:12:12 +0300 Subject: [PATCH 02/13] typo --- docker/test/performance-comparison/compare.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 3d49e9e841a..241fdaec70d 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -167,7 +167,7 @@ function run_tests function run_benchmark { rm -rf benchmark ||: - mkdir bencmhark ||: + mkdir benchmark ||: # TODO disable this when there is an explicit list of tests to run "$script_dir/perf.py" --print right/performance/website.xml > benchmark/website-queries.tsv From 5101708831d6550ac061b9d8b070c3439aad4968 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 12 Jun 2020 18:11:33 +0300 Subject: [PATCH 03/13] fixup --- docker/test/performance-comparison/compare.sh | 7 +++++-- .../config/users.d/perf-comparison-tweaks-users.xml | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 241fdaec70d..5435d37e2e0 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -171,8 +171,11 @@ function run_benchmark # TODO disable this when there is an explicit list of tests to run "$script_dir/perf.py" --print right/performance/website.xml > benchmark/website-queries.tsv - clickhouse-benchmark --port 9001 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-left.json < benchmark/website-queries.tsv - clickhouse-benchmark --port 9002 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-right.json < benchmark/website-queries.tsv + # TODO things to fix in clickhouse-benchmark: + # - --max_memory_usage setting does nothing + # - no way to continue on error + clickhouse-benchmark --port 9001 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-left.json -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv + clickhouse-benchmark --port 9002 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-right.json -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv } function get_profiles_watchdog diff --git a/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml b/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml index 6e3e3df5d39..1bde2a1388b 100644 --- a/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml +++ b/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml @@ -6,6 +6,8 @@ 1 1 1 + + 30000000000 From 56869228a2db535d27bd2ab7767cf2ed64247ac9 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 12 Jun 2020 21:28:07 +0300 Subject: [PATCH 04/13] add flag to continue on errors --- docker/test/performance-comparison/compare.sh | 5 +- programs/benchmark/Benchmark.cpp | 60 ++++++++++++------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 5435d37e2e0..1dbf712ff50 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -173,9 +173,8 @@ function run_benchmark "$script_dir/perf.py" --print right/performance/website.xml > benchmark/website-queries.tsv # TODO things to fix in clickhouse-benchmark: # - --max_memory_usage setting does nothing - # - no way to continue on error - clickhouse-benchmark --port 9001 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-left.json -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv - clickhouse-benchmark --port 9002 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-right.json -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv + clickhouse-benchmark --port 9001 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-left.json --continue_on_errors -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv + clickhouse-benchmark --port 9002 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-right.json --continue_on_errors -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv } function get_profiles_watchdog diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index e17320b39ea..6884f6faed3 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -59,11 +59,14 @@ public: bool cumulative_, bool secure_, const String & default_database_, const String & user_, const String & password_, const String & stage, bool randomize_, size_t max_iterations_, double max_time_, - const String & json_path_, size_t confidence_, const String & query_id_, const Settings & settings_) + const String & json_path_, size_t confidence_, + const String & query_id_, bool continue_on_errors_, + const Settings & settings_) : concurrency(concurrency_), delay(delay_), queue(concurrency), randomize(randomize_), cumulative(cumulative_), max_iterations(max_iterations_), max_time(max_time_), - json_path(json_path_), confidence(confidence_), query_id(query_id_), settings(settings_), + json_path(json_path_), confidence(confidence_), query_id(query_id_), + continue_on_errors(continue_on_errors_), settings(settings_), shared_context(Context::createShared()), global_context(Context::createGlobal(shared_context.get())), pool(concurrency) { @@ -149,6 +152,7 @@ private: String json_path; size_t confidence; std::string query_id; + bool continue_on_errors; Settings settings; SharedContextHolder shared_context; Context global_context; @@ -332,35 +336,45 @@ private: pcg64 generator(randomSeed()); std::uniform_int_distribution distribution(0, connection_entries.size() - 1); - try + /// In these threads we do not accept INT signal. + sigset_t sig_set; + if (sigemptyset(&sig_set) + || sigaddset(&sig_set, SIGINT) + || pthread_sigmask(SIG_BLOCK, &sig_set, nullptr)) { - /// In these threads we do not accept INT signal. - sigset_t sig_set; - if (sigemptyset(&sig_set) - || sigaddset(&sig_set, SIGINT) - || pthread_sigmask(SIG_BLOCK, &sig_set, nullptr)) - throwFromErrno("Cannot block signal.", ErrorCodes::CANNOT_BLOCK_SIGNAL); + throwFromErrno("Cannot block signal.", ErrorCodes::CANNOT_BLOCK_SIGNAL); + } - while (true) + while (true) + { + bool extracted = false; + + while (!extracted) { - bool extracted = false; + extracted = queue.tryPop(query, 100); - while (!extracted) + if (shutdown + || (max_iterations && queries_executed == max_iterations)) { - extracted = queue.tryPop(query, 100); - - if (shutdown || (max_iterations && queries_executed == max_iterations)) - return; + return; } + } + + try + { execute(connection_entries, query, distribution(generator)); ++queries_executed; } - } - catch (...) - { - shutdown = true; - std::cerr << "An error occurred while processing query:\n" << query << "\n"; - throw; + catch (...) + { + std::cerr << "An error occurred while processing query:\n" + << query << "\n"; + if (!continue_on_errors) + { + shutdown = true; + throw; + } + } } } @@ -541,6 +555,7 @@ int mainEntryClickHouseBenchmark(int argc, char ** argv) ("stacktrace", "print stack traces of exceptions") ("confidence", value()->default_value(5), "set the level of confidence for T-test [0=80%, 1=90%, 2=95%, 3=98%, 4=99%, 5=99.5%(default)") ("query_id", value()->default_value(""), "") + ("continue_on_errors", "continue testing even if a query fails") ; Settings settings; @@ -580,6 +595,7 @@ int mainEntryClickHouseBenchmark(int argc, char ** argv) options["json"].as(), options["confidence"].as(), options["query_id"].as(), + options.count("continue_on_errors") > 0, settings); return benchmark.run(); } From c33b472f9a02c19b65cea68f57a6fcfa5e59be66 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Mon, 15 Jun 2020 17:25:42 +0300 Subject: [PATCH 05/13] fixup --- programs/benchmark/Benchmark.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index 6884f6faed3..590e1496fd6 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -363,7 +363,6 @@ private: try { execute(connection_entries, query, distribution(generator)); - ++queries_executed; } catch (...) { @@ -374,7 +373,14 @@ private: shutdown = true; throw; } + else + { + std::cerr << getCurrentExceptionMessage(print_stacktrace, true) ; + } } + // Count failed queries toward executed, so that we'd reach + // max_iterations even if every run fails. + ++queries_executed; } } From 857582245e894ef71e59decef44555760f8f9908 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Mon, 15 Jun 2020 19:39:00 +0300 Subject: [PATCH 06/13] fixup --- programs/benchmark/Benchmark.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index af56aaa6db5..b8e4a0c346a 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -62,12 +62,13 @@ public: bool randomize_, size_t max_iterations_, double max_time_, const String & json_path_, size_t confidence_, const String & query_id_, bool continue_on_errors_, - const Settings & settings_) + bool print_stacktrace_, const Settings & settings_) : concurrency(concurrency_), delay(delay_), queue(concurrency), randomize(randomize_), cumulative(cumulative_), max_iterations(max_iterations_), max_time(max_time_), json_path(json_path_), confidence(confidence_), query_id(query_id_), - continue_on_errors(continue_on_errors_), settings(settings_), + continue_on_errors(continue_on_errors_), + print_stacktrace(print_stacktrace_), settings(settings_), shared_context(Context::createShared()), global_context(Context::createGlobal(shared_context.get())), pool(concurrency) { @@ -154,6 +155,7 @@ private: size_t confidence; std::string query_id; bool continue_on_errors; + bool print_stacktrace; Settings settings; SharedContextHolder shared_context; Context global_context; @@ -376,7 +378,8 @@ private: } else { - std::cerr << getCurrentExceptionMessage(print_stacktrace, true) ; + std::cerr << getCurrentExceptionMessage(print_stacktrace, + true /*check embedded stack trace*/) ; } } // Count failed queries toward executed, so that we'd reach @@ -605,6 +608,7 @@ int mainEntryClickHouseBenchmark(int argc, char ** argv) options["confidence"].as(), options["query_id"].as(), options.count("continue_on_errors") > 0, + print_stacktrace, settings); return benchmark.run(); } From cd769e5ebf2e2cdd1e4340e292412fafd6a6add9 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Tue, 16 Jun 2020 11:21:15 +0300 Subject: [PATCH 07/13] fixup --- docker/test/performance-comparison/compare.sh | 2 +- docker/test/performance-comparison/report.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 1dbf712ff50..c9beff6d7db 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -235,7 +235,7 @@ function build_log_column_definitions { # FIXME This loop builds column definitons from TSVWithNamesAndTypes in an # absolutely atrocious way. This should be done by the file() function itself. -for x in {right,left}-{addresses,{query,query-thread,trace,metric}-log}.tsv +for x in {right,left}-{addresses,{query,query-thread,trace,{async-,}metric}-log}.tsv do paste -d' ' \ <(sed -n '1{s/\t/\n/g;p;q}' "$x" | sed 's/\(^.*$\)/"\1"/') \ diff --git a/docker/test/performance-comparison/report.py b/docker/test/performance-comparison/report.py index d7e30190aef..d830b6e65fc 100755 --- a/docker/test/performance-comparison/report.py +++ b/docker/test/performance-comparison/report.py @@ -325,8 +325,8 @@ if args.report == 'main': def print_benchmark_results(): left_json = json.load(open('benchmark/website-left.json')); right_json = json.load(open('benchmark/website-right.json')); - left_qps = left_json["statistics"]["QPS"] - right_qps = right_json["statistics"]["QPS"] + left_qps = next(iter(left_json.values()))["statistics"]["QPS"] + right_qps = next(iter(right_json.values()))["statistics"]["QPS"] relative_diff = (right_qps - left_qps) / left_qps; times_diff = max(right_qps, left_qps) / max(0.01, min(right_qps, left_qps)) print(tableStart('Concurrent benchmarks')) From fbecf42dfcb838d0ef08090edf001a2406df2c4c Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Sat, 20 Jun 2020 01:41:15 +0300 Subject: [PATCH 08/13] report the number of errors --- docker/test/performance-comparison/report.py | 31 ++- programs/benchmark/Benchmark.cpp | 179 ++++++++++++------ src/AggregateFunctions/ReservoirSampler.h | 9 +- .../ReservoirSamplerDeterministic.h | 9 +- 4 files changed, 161 insertions(+), 67 deletions(-) diff --git a/docker/test/performance-comparison/report.py b/docker/test/performance-comparison/report.py index d830b6e65fc..a55c5259444 100755 --- a/docker/test/performance-comparison/report.py +++ b/docker/test/performance-comparison/report.py @@ -323,15 +323,16 @@ if args.report == 'main': print_test_times() def print_benchmark_results(): - left_json = json.load(open('benchmark/website-left.json')); - right_json = json.load(open('benchmark/website-right.json')); - left_qps = next(iter(left_json.values()))["statistics"]["QPS"] - right_qps = next(iter(right_json.values()))["statistics"]["QPS"] - relative_diff = (right_qps - left_qps) / left_qps; - times_diff = max(right_qps, left_qps) / max(0.01, min(right_qps, left_qps)) + json_reports = [json.load(open(f'benchmark/website-{x}.json')) for x in ['left', 'right']] + stats = [next(iter(x.values()))["statistics"] for x in json_reports] + qps = [x["QPS"] for x in stats] + errors = [x["num_errors"] for x in stats] + relative_diff = (qps[1] - qps[0]) / max(0.01, qps[0]); + times_diff = max(qps) / max(0.01, min(qps)) print(tableStart('Concurrent benchmarks')) print(tableHeader(['Benchmark', 'Old, queries/s', 'New, queries/s', 'Relative difference', 'Times difference'])) - row = ['website', f'{left_qps:.3f}', f'{right_qps:.3f}', f'{relative_diff:.3f}', f'x{times_diff:.3f}'] + all_rows = [] + row = ['website', f'{qps[0]:.3f}', f'{qps[1]:.3f}', f'{relative_diff:.3f}', f'x{times_diff:.3f}'] attrs = ['' for r in row] if abs(relative_diff) > 0.1: # More queries per second is better. @@ -341,7 +342,23 @@ if args.report == 'main': attrs[3] = f'style="background: {color_bad}"' else: attrs[3] = '' + all_rows.append((rows, attrs)); print(tableRow(row, attrs)) + + if max(errors): + attrs = ['' for r in row] + row[1] = f'{errors[0]:.3f}' + row[2] = f'{errors[1]:.3f}' + if errors[0]: + attrs[1] += f' style="background: {color_bad}" ' + if errors[1]: + attrs[2] += f' style="background: {color_bad}" ' + + all_rows[0][1] += " colspan=2 " + + for row, attrs in all_rows: + print(tableRow(row, attrs)) + print(tableEnd()) try: diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index b8e4a0c346a..8288ab01f36 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -53,6 +52,71 @@ namespace ErrorCodes extern const int EMPTY_DATA_PASSED; } +template +class EventQueue +{ + std::mutex mutex; + std::condition_variable condvar; + std::deque queue; + bool is_closed = false; + + const size_t max_queue_size; + +public: + EventQueue(size_t max_queue_size_) : max_queue_size(max_queue_size_) {} + + template + bool push(Args && ... args) + { + std::unique_lock lock(mutex); + condvar.wait(lock, + [this]() { return queue.size() < max_queue_size || is_closed; }); + + if (is_closed) + { + return false; + } + + queue.push_back(std::forward(args)...); + condvar.notify_all(); + + return true; + } + + bool pop(Event & event) + { + std::unique_lock lock(mutex); + condvar.wait(lock, [this]() { return queue.size() > 0 || is_closed; }); + + if (queue.size() > 0) + { + event = queue.front(); + queue.pop_front(); + + condvar.notify_all(); + return true; + } + + assert(is_closed); + return false; + } + + void close() + { + std::unique_lock lock(mutex); + is_closed = true; + condvar.notify_all(); + } + + void clearAndClose() + { + std::unique_lock lock(mutex); + is_closed = true; + queue.clear(); + condvar.notify_all(); + } +}; + class Benchmark : public Poco::Util::Application { public: @@ -64,7 +128,7 @@ public: const String & query_id_, bool continue_on_errors_, bool print_stacktrace_, const Settings & settings_) : - concurrency(concurrency_), delay(delay_), queue(concurrency), randomize(randomize_), + concurrency(concurrency_), delay(delay_), queue(concurrency_ * 2), randomize(randomize_), cumulative(cumulative_), max_iterations(max_iterations_), max_time(max_time_), json_path(json_path_), confidence(confidence_), query_id(query_id_), continue_on_errors(continue_on_errors_), @@ -140,8 +204,7 @@ private: using Queries = std::vector; Queries queries; - using Queue = ConcurrentBoundedQueue; - Queue queue; + EventQueue queue; using ConnectionPoolUniq = std::unique_ptr; using ConnectionPoolUniqs = std::vector; @@ -161,14 +224,12 @@ private: Context global_context; QueryProcessingStage::Enum query_processing_stage; - /// Don't execute new queries after timelimit or SIGINT or exception - std::atomic shutdown{false}; - std::atomic queries_executed{0}; struct Stats { std::atomic queries{0}; + size_t errors = 0; size_t read_rows = 0; size_t read_bytes = 0; size_t result_rows = 0; @@ -245,36 +306,29 @@ private: /// Try push new query and check cancellation conditions bool tryPushQueryInteractively(const String & query, InterruptListener & interrupt_listener) { - bool inserted = false; - - while (!inserted) + if (!queue.push(query)) { - inserted = queue.tryPush(query, 100); + /// An exception occurred in a worker + return false; + } - if (shutdown) - { - /// An exception occurred in a worker - return false; - } + if (max_time > 0 && total_watch.elapsedSeconds() >= max_time) + { + std::cout << "Stopping launch of queries. Requested time limit is exhausted.\n"; + return false; + } - if (max_time > 0 && total_watch.elapsedSeconds() >= max_time) - { - std::cout << "Stopping launch of queries. Requested time limit is exhausted.\n"; - return false; - } + if (interrupt_listener.check()) + { + std::cout << "Stopping launch of queries. SIGINT received." << std::endl; + return false; + } - if (interrupt_listener.check()) - { - std::cout << "Stopping launch of queries. SIGINT received.\n"; - return false; - } - - if (delay > 0 && delay_watch.elapsedSeconds() > delay) - { - printNumberOfQueriesExecuted(queries_executed); - cumulative ? report(comparison_info_total) : report(comparison_info_per_interval); - delay_watch.restart(); - } + if (delay > 0 && delay_watch.elapsedSeconds() > delay) + { + printNumberOfQueriesExecuted(queries_executed); + cumulative ? report(comparison_info_total) : report(comparison_info_per_interval); + delay_watch.restart(); } return true; @@ -315,10 +369,13 @@ private: if (!tryPushQueryInteractively(queries[query_index], interrupt_listener)) { - shutdown = true; + // A stop condition ocurred, so clear all the queries that are + // in queue. + queue.clearAndClose(); break; } } + queue.close(); pool.wait(); total_watch.stop(); @@ -350,36 +407,32 @@ private: while (true) { - bool extracted = false; - - while (!extracted) + if (!queue.pop(query)) { - extracted = queue.tryPop(query, 100); - - if (shutdown - || (max_iterations && queries_executed == max_iterations)) - { - return; - } + return; } + const auto connection_index = distribution(generator); try { - execute(connection_entries, query, distribution(generator)); + execute(connection_entries, query, connection_index); } catch (...) { - std::cerr << "An error occurred while processing query:\n" - << query << "\n"; + std::cerr << "An error occurred while processing the query '\n" + << query << "'.\n"; if (!continue_on_errors) { - shutdown = true; + queue.clearAndClose(); throw; } else { std::cerr << getCurrentExceptionMessage(print_stacktrace, - true /*check embedded stack trace*/) ; + true /*check embedded stack trace*/) << std::endl; + + comparison_info_per_interval[connection_index]->errors++; + comparison_info_total[connection_index]->errors++; } } // Count failed queries toward executed, so that we'd reach @@ -433,7 +486,12 @@ private: std::cerr << connections[i]->getDescription() << ", " - << "queries " << info->queries << ", " + << "queries " << info->queries << ", "; + if (info->errors) + { + std::cerr << "errors " << info->errors << ", "; + } + std::cerr << "QPS: " << (info->queries / seconds) << ", " << "RPS: " << (info->read_rows / seconds) << ", " << "MiB/s: " << (info->read_bytes / seconds / 1048576) << ", " @@ -500,18 +558,22 @@ private: print_key_value("MiBPS", info->read_bytes / info->work_time); print_key_value("RPS_result", info->result_rows / info->work_time); print_key_value("MiBPS_result", info->result_bytes / info->work_time); - print_key_value("num_queries", info->queries.load(), false); + print_key_value("num_queries", info->queries.load()); + print_key_value("num_errors", info->errors, false); json_out << "},\n"; json_out << double_quote << "query_time_percentiles" << ": {\n"; - for (int percent = 0; percent <= 90; percent += 10) - print_percentile(*info, percent); + if (info->queries != 0) + { + for (int percent = 0; percent <= 90; percent += 10) + print_percentile(*info, percent); - print_percentile(*info, 95); - print_percentile(*info, 99); - print_percentile(*info, 99.9); - print_percentile(*info, 99.99, false); + print_percentile(*info, 95); + print_percentile(*info, 99); + print_percentile(*info, 99.9); + print_percentile(*info, 99.99, false); + } json_out << "}\n"; json_out << (i == infos.size() - 1 ? "}\n" : "},\n"); @@ -524,7 +586,8 @@ public: ~Benchmark() override { - shutdown = true; + queue.clearAndClose(); + pool.wait(); } }; diff --git a/src/AggregateFunctions/ReservoirSampler.h b/src/AggregateFunctions/ReservoirSampler.h index b61027ce692..0cb402420b6 100644 --- a/src/AggregateFunctions/ReservoirSampler.h +++ b/src/AggregateFunctions/ReservoirSampler.h @@ -13,6 +13,13 @@ #include #include +namespace DB +{ +namespace ErrorCodes +{ + extern const int LOGICAL_ERROR; +} +} /// Implementing the Reservoir Sampling algorithm. Incrementally selects from the added objects a random subset of the sample_count size. /// Can approximately get quantiles. @@ -236,7 +243,7 @@ private: ResultType onEmpty() const { if (OnEmpty == ReservoirSamplerOnEmpty::THROW) - throw Poco::Exception("Quantile of empty ReservoirSampler"); + throw DB::Exception(DB::ErrorCodes::LOGICAL_ERROR, "Quantile of empty ReservoirSampler"); else return NanLikeValueConstructor>::getValue(); } diff --git a/src/AggregateFunctions/ReservoirSamplerDeterministic.h b/src/AggregateFunctions/ReservoirSamplerDeterministic.h index a520b8236b7..f23833ece01 100644 --- a/src/AggregateFunctions/ReservoirSamplerDeterministic.h +++ b/src/AggregateFunctions/ReservoirSamplerDeterministic.h @@ -14,6 +14,13 @@ #include #include +namespace DB +{ +namespace ErrorCodes +{ + extern const int LOGICAL_ERROR; +} +} /// Implementation of Reservoir Sampling algorithm. Incrementally selects from the added objects a random subset of the `sample_count` size. /// Can approximately get quantiles. @@ -223,7 +230,7 @@ private: ResultType onEmpty() const { if (OnEmpty == ReservoirSamplerDeterministicOnEmpty::THROW) - throw Poco::Exception("Quantile of empty ReservoirSamplerDeterministic"); + throw DB::Exception(DB::ErrorCodes::LOGICAL_ERROR, "Quantile of empty ReservoirSamplerDeterministic"); else return NanLikeValueConstructor>::getValue(); } From 96368b7d0c45abfaa0c65d02d0905e3524b730e8 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Sat, 20 Jun 2020 02:03:13 +0300 Subject: [PATCH 09/13] fixup --- programs/benchmark/Benchmark.cpp | 141 ++++++++++--------------------- 1 file changed, 46 insertions(+), 95 deletions(-) diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index 8288ab01f36..150e31feca3 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -52,71 +53,6 @@ namespace ErrorCodes extern const int EMPTY_DATA_PASSED; } -template -class EventQueue -{ - std::mutex mutex; - std::condition_variable condvar; - std::deque queue; - bool is_closed = false; - - const size_t max_queue_size; - -public: - EventQueue(size_t max_queue_size_) : max_queue_size(max_queue_size_) {} - - template - bool push(Args && ... args) - { - std::unique_lock lock(mutex); - condvar.wait(lock, - [this]() { return queue.size() < max_queue_size || is_closed; }); - - if (is_closed) - { - return false; - } - - queue.push_back(std::forward(args)...); - condvar.notify_all(); - - return true; - } - - bool pop(Event & event) - { - std::unique_lock lock(mutex); - condvar.wait(lock, [this]() { return queue.size() > 0 || is_closed; }); - - if (queue.size() > 0) - { - event = queue.front(); - queue.pop_front(); - - condvar.notify_all(); - return true; - } - - assert(is_closed); - return false; - } - - void close() - { - std::unique_lock lock(mutex); - is_closed = true; - condvar.notify_all(); - } - - void clearAndClose() - { - std::unique_lock lock(mutex); - is_closed = true; - queue.clear(); - condvar.notify_all(); - } -}; - class Benchmark : public Poco::Util::Application { public: @@ -128,7 +64,7 @@ public: const String & query_id_, bool continue_on_errors_, bool print_stacktrace_, const Settings & settings_) : - concurrency(concurrency_), delay(delay_), queue(concurrency_ * 2), randomize(randomize_), + concurrency(concurrency_), delay(delay_), queue(concurrency), randomize(randomize_), cumulative(cumulative_), max_iterations(max_iterations_), max_time(max_time_), json_path(json_path_), confidence(confidence_), query_id(query_id_), continue_on_errors(continue_on_errors_), @@ -204,7 +140,8 @@ private: using Queries = std::vector; Queries queries; - EventQueue queue; + using Queue = ConcurrentBoundedQueue; + Queue queue; using ConnectionPoolUniq = std::unique_ptr; using ConnectionPoolUniqs = std::vector; @@ -224,6 +161,9 @@ private: Context global_context; QueryProcessingStage::Enum query_processing_stage; + /// Don't execute new queries after timelimit or SIGINT or exception + std::atomic shutdown{false}; + std::atomic queries_executed{0}; struct Stats @@ -306,29 +246,36 @@ private: /// Try push new query and check cancellation conditions bool tryPushQueryInteractively(const String & query, InterruptListener & interrupt_listener) { - if (!queue.push(query)) - { - /// An exception occurred in a worker - return false; - } + bool inserted = false; - if (max_time > 0 && total_watch.elapsedSeconds() >= max_time) + while (!inserted) { - std::cout << "Stopping launch of queries. Requested time limit is exhausted.\n"; - return false; - } + inserted = queue.tryPush(query, 100); - if (interrupt_listener.check()) - { - std::cout << "Stopping launch of queries. SIGINT received." << std::endl; - return false; - } + if (shutdown) + { + /// An exception occurred in a worker + return false; + } - if (delay > 0 && delay_watch.elapsedSeconds() > delay) - { - printNumberOfQueriesExecuted(queries_executed); - cumulative ? report(comparison_info_total) : report(comparison_info_per_interval); - delay_watch.restart(); + if (max_time > 0 && total_watch.elapsedSeconds() >= max_time) + { + std::cout << "Stopping launch of queries. Requested time limit is exhausted.\n"; + return false; + } + + if (interrupt_listener.check()) + { + std::cout << "Stopping launch of queries. SIGINT received." << std::endl; + return false; + } + + if (delay > 0 && delay_watch.elapsedSeconds() > delay) + { + printNumberOfQueriesExecuted(queries_executed); + cumulative ? report(comparison_info_total) : report(comparison_info_per_interval); + delay_watch.restart(); + } } return true; @@ -369,13 +316,10 @@ private: if (!tryPushQueryInteractively(queries[query_index], interrupt_listener)) { - // A stop condition ocurred, so clear all the queries that are - // in queue. - queue.clearAndClose(); + shutdown = true; break; } } - queue.close(); pool.wait(); total_watch.stop(); @@ -407,9 +351,17 @@ private: while (true) { - if (!queue.pop(query)) + bool extracted = false; + + while (!extracted) { - return; + extracted = queue.tryPop(query, 100); + + if (shutdown + || (max_iterations && queries_executed == max_iterations)) + { + return; + } } const auto connection_index = distribution(generator); @@ -423,7 +375,7 @@ private: << query << "'.\n"; if (!continue_on_errors) { - queue.clearAndClose(); + shutdown = true; throw; } else @@ -586,8 +538,7 @@ public: ~Benchmark() override { - queue.clearAndClose(); - pool.wait(); + shutdown = true; } }; From ed935cd66bee2176f544dae859cf25cc1d45357e Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Sat, 20 Jun 2020 02:04:42 +0300 Subject: [PATCH 10/13] fixup --- programs/benchmark/Benchmark.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/benchmark/Benchmark.cpp b/programs/benchmark/Benchmark.cpp index 150e31feca3..89b9a877d5e 100644 --- a/programs/benchmark/Benchmark.cpp +++ b/programs/benchmark/Benchmark.cpp @@ -371,7 +371,7 @@ private: } catch (...) { - std::cerr << "An error occurred while processing the query '\n" + std::cerr << "An error occurred while processing the query '" << query << "'.\n"; if (!continue_on_errors) { From eac6eb8c5a31757e6ecdb86294439729386ad102 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Mon, 22 Jun 2020 15:22:09 +0300 Subject: [PATCH 11/13] report --- docker/test/performance-comparison/report.py | 49 +++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/docker/test/performance-comparison/report.py b/docker/test/performance-comparison/report.py index a55c5259444..3af710606c2 100755 --- a/docker/test/performance-comparison/report.py +++ b/docker/test/performance-comparison/report.py @@ -7,6 +7,7 @@ import csv import itertools import json import os +import pprint import sys import traceback @@ -102,7 +103,7 @@ def tableRow(cell_values, cell_attributes = []): for v, a in itertools.zip_longest( cell_values, cell_attributes, fillvalue = '') - if a is not None])) + if a is not None and v is not None])) def tableHeader(r): return tr(''.join([th(f) for f in r])) @@ -326,39 +327,53 @@ if args.report == 'main': json_reports = [json.load(open(f'benchmark/website-{x}.json')) for x in ['left', 'right']] stats = [next(iter(x.values()))["statistics"] for x in json_reports] qps = [x["QPS"] for x in stats] + queries = [x["num_queries"] for x in stats] errors = [x["num_errors"] for x in stats] relative_diff = (qps[1] - qps[0]) / max(0.01, qps[0]); times_diff = max(qps) / max(0.01, min(qps)) - print(tableStart('Concurrent benchmarks')) - print(tableHeader(['Benchmark', 'Old, queries/s', 'New, queries/s', 'Relative difference', 'Times difference'])) + all_rows = [] - row = ['website', f'{qps[0]:.3f}', f'{qps[1]:.3f}', f'{relative_diff:.3f}', f'x{times_diff:.3f}'] - attrs = ['' for r in row] + header = ['Benchmark', 'Metric', 'Old', 'New', 'Relative difference', 'Times difference']; + + attrs = ['' for x in header] + row = ['website', 'queries', f'{queries[0]:d}', f'{queries[1]:d}', '--', '--'] + attrs[0] = 'rowspan=2' + all_rows.append([row, attrs]) + + attrs = ['' for x in header] + row = [None, 'queries/s', f'{qps[0]:.3f}', f'{qps[1]:.3f}', f'{relative_diff:.3f}', f'x{times_diff:.3f}'] if abs(relative_diff) > 0.1: # More queries per second is better. if relative_diff > 0.: - attrs[3] = f'style="background: {color_good}"' + attrs[4] = f'style="background: {color_good}"' else: - attrs[3] = f'style="background: {color_bad}"' + attrs[4] = f'style="background: {color_bad}"' else: - attrs[3] = '' - all_rows.append((rows, attrs)); - print(tableRow(row, attrs)) + attrs[4] = '' + all_rows.append([row, attrs]); if max(errors): - attrs = ['' for r in row] - row[1] = f'{errors[0]:.3f}' - row[2] = f'{errors[1]:.3f}' + all_rows[0][1][0] = "rowspan=3" + row = [''] * (len(header)) + attrs = ['' for x in header] + + attrs[0] = None + row[1] = 'errors' + row[2] = f'{errors[0]:d}' + row[3] = f'{errors[1]:d}' + row[4] = '--' + row[5] = '--' if errors[0]: - attrs[1] += f' style="background: {color_bad}" ' - if errors[1]: attrs[2] += f' style="background: {color_bad}" ' + if errors[1]: + attrs[3] += f' style="background: {color_bad}" ' - all_rows[0][1] += " colspan=2 " + all_rows.append([row, attrs]) + print(tableStart('Concurrent benchmarks')) + print(tableHeader(header)) for row, attrs in all_rows: print(tableRow(row, attrs)) - print(tableEnd()) try: From ab809f59b9796c3170215f699b33874223c36969 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Tue, 23 Jun 2020 15:30:45 +0300 Subject: [PATCH 12/13] memory usage settings --- docker/test/performance-comparison/compare.sh | 25 ++++++++++++++----- .../users.d/perf-comparison-tweaks-users.xml | 2 -- docker/test/performance-comparison/perf.py | 14 +++++++++-- programs/server/Server.cpp | 14 +++++++++-- src/Core/SettingsCollection.h | 3 +++ src/Core/SettingsCollectionImpl.h | 13 ++++++++++ 6 files changed, 59 insertions(+), 12 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 8f4defb2c6d..0a205796c9d 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -131,6 +131,11 @@ function run_tests test_files=$(ls "$test_prefix"/*.xml) fi + # Determine which concurrent benchmarks to run. For now, the only test + # we run as a concurrent benchmark is 'website'. Run it as benchmark if we + # are also going to run it as a normal test. + for test in $test_files; do echo $test; done | sed -n '/website/p' > benchmarks-to-run.txt + # Delete old report files. for x in {test-times,wall-clock-times}.tsv do @@ -169,12 +174,20 @@ function run_benchmark rm -rf benchmark ||: mkdir benchmark ||: - # TODO disable this when there is an explicit list of tests to run - "$script_dir/perf.py" --print right/performance/website.xml > benchmark/website-queries.tsv - # TODO things to fix in clickhouse-benchmark: - # - --max_memory_usage setting does nothing - clickhouse-benchmark --port 9001 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-left.json --continue_on_errors -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv - clickhouse-benchmark --port 9002 --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --json benchmark/website-right.json --continue_on_errors -- --max_memory_usage 30000000000 < benchmark/website-queries.tsv + # The list is built by run_tests. + for file in $(cat benchmarks-to-run.txt) + do + name=$(basename "$file" ".xml") + + "$script_dir/perf.py" --print-queries "$file" > "benchmark/$name-queries.txt" + "$script_dir/perf.py" --print-settings "$file" > "benchmark/$name-settings.txt" + + readarray -t settings < "benchmark/$name-settings.txt" + command=(clickhouse-benchmark --concurrency 6 --cumulative --iterations 1000 --randomize 1 --delay 0 --continue_on_errors "${settings[@]}") + + "${command[@]}" --port 9001 --json "benchmark/$name-left.json" < "benchmark/$name-queries.txt" + "${command[@]}" --port 9002 --json "benchmark/$name-right.json" < "benchmark/$name-queries.txt" + done } function get_profiles_watchdog diff --git a/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml b/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml index 1bde2a1388b..6e3e3df5d39 100644 --- a/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml +++ b/docker/test/performance-comparison/config/users.d/perf-comparison-tweaks-users.xml @@ -6,8 +6,6 @@ 1 1 1 - - 30000000000 diff --git a/docker/test/performance-comparison/perf.py b/docker/test/performance-comparison/perf.py index 74d0300b074..f81673bd8a1 100755 --- a/docker/test/performance-comparison/perf.py +++ b/docker/test/performance-comparison/perf.py @@ -21,7 +21,8 @@ parser.add_argument('--host', nargs='*', default=['localhost'], help="Server hos parser.add_argument('--port', nargs='*', default=[9000], help="Server port(s). Corresponds to '--host' options.") parser.add_argument('--runs', type=int, default=int(os.environ.get('CHPC_RUNS', 17)), help='Number of query runs per server. Defaults to CHPC_RUNS environment variable.') parser.add_argument('--long', action='store_true', help='Do not skip the tests tagged as long.') -parser.add_argument('--print', action='store_true', help='Print test queries and exit.') +parser.add_argument('--print-queries', action='store_true', help='Print test queries and exit.') +parser.add_argument('--print-settings', action='store_true', help='Print test settings and exit.') args = parser.parse_args() test_name = os.path.splitext(os.path.basename(args.file[0].name))[0] @@ -52,11 +53,20 @@ test_query_templates = [q.text for q in root.findall('query')] test_queries = substitute_parameters(test_query_templates) # If we're only asked to print the queries, do that and exit -if args.print: +if args.print_queries: for q in test_queries: print(q) exit(0) +# If we're only asked to print the settings, do that and exit. These are settings +# for clickhouse-benchmark, so we print them as command line arguments, e.g. +# '--max_memory_usage=10000000'. +if args.print_settings: + for s in root.findall('settings/*'): + print(f'--{s.tag}={s.text}') + + exit(0) + # Skip long tests if not args.long: for tag in root.findall('.//tag'): diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index 25d8e5595b7..424c0f3ccf7 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -645,12 +645,22 @@ int Server::main(const std::vector & /*args*/) if (max_server_memory_usage == 0) { max_server_memory_usage = default_max_server_memory_usage; - LOG_INFO(log, "Setting max_server_memory_usage was set to {}", formatReadableSizeWithBinarySuffix(max_server_memory_usage)); + LOG_INFO(log, "Setting max_server_memory_usage was set to {}" + " ({} available * {:.2f} max_server_memory_usage_to_ram_ratio)", + formatReadableSizeWithBinarySuffix(max_server_memory_usage), + formatReadableSizeWithBinarySuffix(memory_amount), + max_server_memory_usage_to_ram_ratio); } else if (max_server_memory_usage > default_max_server_memory_usage) { max_server_memory_usage = default_max_server_memory_usage; - LOG_INFO(log, "Setting max_server_memory_usage was lowered to {} because the system has low amount of memory", formatReadableSizeWithBinarySuffix(max_server_memory_usage)); + LOG_INFO(log, "Setting max_server_memory_usage was lowered to {}" + " because the system has low amount of memory. The amount was" + " calculated as {} available" + " * {:.2f} max_server_memory_usage_to_ram_ratio", + formatReadableSizeWithBinarySuffix(max_server_memory_usage), + formatReadableSizeWithBinarySuffix(memory_amount), + max_server_memory_usage_to_ram_ratio); } total_memory_tracker.setOrRaiseHardLimit(max_server_memory_usage); diff --git a/src/Core/SettingsCollection.h b/src/Core/SettingsCollection.h index 71a308fb37e..10c64c615ff 100644 --- a/src/Core/SettingsCollection.h +++ b/src/Core/SettingsCollection.h @@ -549,6 +549,9 @@ public: /// Gathers all changed values (e.g. for applying them later to another collection of settings). SettingsChanges changes() const; + // A debugging aid. + std::string dumpChangesToString() const; + /// Applies change to concrete setting. void applyChange(const SettingChange & change); diff --git a/src/Core/SettingsCollectionImpl.h b/src/Core/SettingsCollectionImpl.h index 877567a7caf..f0854f11b8a 100644 --- a/src/Core/SettingsCollectionImpl.h +++ b/src/Core/SettingsCollectionImpl.h @@ -219,6 +219,19 @@ SettingsChanges SettingsCollection::changes() const } +template +std::string SettingsCollection::dumpChangesToString() const +{ + std::stringstream ss; + for (const auto & c : changes()) + { + ss << c.name << " = " + << applyVisitor(FieldVisitorToString(), c.value) << "\n"; + } + return ss.str(); +} + + template void SettingsCollection::applyChange(const SettingChange & change) { From 58747df00c1ffceb50c460be134929db2637c20c Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 25 Jun 2020 09:35:21 +0300 Subject: [PATCH 13/13] Update docker/test/performance-comparison/perf.py --- docker/test/performance-comparison/perf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/test/performance-comparison/perf.py b/docker/test/performance-comparison/perf.py index f81673bd8a1..9a6081b751c 100755 --- a/docker/test/performance-comparison/perf.py +++ b/docker/test/performance-comparison/perf.py @@ -19,7 +19,7 @@ parser = argparse.ArgumentParser(description='Run performance test.') parser.add_argument('file', metavar='FILE', type=argparse.FileType('r', encoding='utf-8'), nargs=1, help='test description file') parser.add_argument('--host', nargs='*', default=['localhost'], help="Server hostname(s). Corresponds to '--port' options.") parser.add_argument('--port', nargs='*', default=[9000], help="Server port(s). Corresponds to '--host' options.") -parser.add_argument('--runs', type=int, default=int(os.environ.get('CHPC_RUNS', 17)), help='Number of query runs per server. Defaults to CHPC_RUNS environment variable.') +parser.add_argument('--runs', type=int, default=int(os.environ.get('CHPC_RUNS', 13)), help='Number of query runs per server. Defaults to CHPC_RUNS environment variable.') parser.add_argument('--long', action='store_true', help='Do not skip the tests tagged as long.') parser.add_argument('--print-queries', action='store_true', help='Print test queries and exit.') parser.add_argument('--print-settings', action='store_true', help='Print test settings and exit.')