From 223fc4d1e7b0ffd6ad7f2b2226d2210eec5af64d Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Thu, 10 Sep 2020 12:59:10 +0300 Subject: [PATCH 1/3] Avoid error while building the report on broken perf tests --- docker/test/performance-comparison/compare.sh | 12 ++++++++++++ src/Interpreters/ExpressionActions.cpp | 12 ++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 364e9994ab7..2ae7910dcaa 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -394,12 +394,24 @@ create table query_run_metrics_denorm engine File(TSV, 'analyze/query-run-metric order by test, query_index, metric_names, version, query_id ; +-- Filter out tests that don't have an even number of runs, to avoid breaking +-- the further calculations. This may happen if there was an error during the +-- test runs, e.g. the server died. It will be reported in test errors, so we +-- don't have to report it again. +create view broken_tests as + select test_name + from query_runs + group by test_name + having count(*) % 2 == 0 + ; + -- This is for statistical processing with eqmed.sql create table query_run_metrics_for_stats engine File( TSV, -- do not add header -- will parse with grep 'analyze/query-run-metrics-for-stats.tsv') as select test, query_index, 0 run, version, metric_values from query_run_metric_arrays + where test not in broken_tests order by test, query_index, run, version ; diff --git a/src/Interpreters/ExpressionActions.cpp b/src/Interpreters/ExpressionActions.cpp index 33fa6215160..0c287e4026d 100644 --- a/src/Interpreters/ExpressionActions.cpp +++ b/src/Interpreters/ExpressionActions.cpp @@ -607,8 +607,16 @@ void ExpressionActions::execute(Block & block, bool dry_run) const { for (const auto & action : actions) { - action.execute(block, dry_run); - checkLimits(block); + try + { + action.execute(block, dry_run); + checkLimits(block); + } + catch (Exception & e) + { + e.addMessage(fmt::format("while executing '{}'", action.toString())); + throw; + } } } From a2a647eb1caac92b13f73b04651c7d64b66c0fc1 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Thu, 10 Sep 2020 13:02:45 +0300 Subject: [PATCH 2/3] fixup --- docker/test/performance-comparison/compare.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docker/test/performance-comparison/compare.sh b/docker/test/performance-comparison/compare.sh index 2ae7910dcaa..08f4cb599ab 100755 --- a/docker/test/performance-comparison/compare.sh +++ b/docker/test/performance-comparison/compare.sh @@ -398,11 +398,11 @@ create table query_run_metrics_denorm engine File(TSV, 'analyze/query-run-metric -- the further calculations. This may happen if there was an error during the -- test runs, e.g. the server died. It will be reported in test errors, so we -- don't have to report it again. -create view broken_tests as - select test_name +create view broken_queries as + select test, query_index from query_runs - group by test_name - having count(*) % 2 == 0 + group by test, query_index + having count(*) % 2 != 0 ; -- This is for statistical processing with eqmed.sql @@ -411,7 +411,7 @@ create table query_run_metrics_for_stats engine File( 'analyze/query-run-metrics-for-stats.tsv') as select test, query_index, 0 run, version, metric_values from query_run_metric_arrays - where test not in broken_tests + where (test, query_index) not in broken_queries order by test, query_index, run, version ; From da2a3fffe88665bb5c5e8e5e9546f1955cac4fd5 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Wed, 16 Sep 2020 13:00:15 +0300 Subject: [PATCH 3/3] fixup --- src/Dictionaries/CacheDictionary.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Dictionaries/CacheDictionary.cpp b/src/Dictionaries/CacheDictionary.cpp index 29aee9bfc21..cb39dffeb6c 100644 --- a/src/Dictionaries/CacheDictionary.cpp +++ b/src/Dictionaries/CacheDictionary.cpp @@ -822,7 +822,24 @@ void CacheDictionary::waitForCurrentUpdateFinish(UpdateUnitPtr & update_unit_ptr if (update_unit_ptr->current_exception) - std::rethrow_exception(update_unit_ptr->current_exception); + { + // There might have been a single update unit for multiple callers in + // independent threads, and current_exception will be the same for them. + // Don't just rethrow it, because sharing the same exception object + // between multiple threads can lead to weird effects if they decide to + // modify it, for example, by adding some error context. + try + { + std::rethrow_exception(update_unit_ptr->current_exception); + } + catch (...) + { + throw DB::Exception(ErrorCodes::CACHE_DICTIONARY_UPDATE_FAIL, + "Dictionary update failed: {}", + getCurrentExceptionMessage(true /*with stack trace*/, + true /*check embedded stack trace*/)); + } + } } void CacheDictionary::tryPushToUpdateQueueOrThrow(UpdateUnitPtr & update_unit_ptr) const