From 06f0780a3f4683ed591573086ac4d08eb78fd13d Mon Sep 17 00:00:00 2001 From: woodlzm Date: Fri, 10 May 2024 23:27:42 -0700 Subject: [PATCH 1/7] Add a build_id ALIAS column to trace_log to facilitate auto renaming upon detecting binary changes. --- src/Interpreters/TraceLog.cpp | 13 +++++++++++++ src/Interpreters/TraceLog.h | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/TraceLog.cpp b/src/Interpreters/TraceLog.cpp index 01bedf34f15..2c56eb79089 100644 --- a/src/Interpreters/TraceLog.cpp +++ b/src/Interpreters/TraceLog.cpp @@ -8,6 +8,7 @@ #include #include #include +#include namespace DB @@ -53,6 +54,18 @@ ColumnsDescription TraceLogElement::getColumnsDescription() }; } +NamesAndAliases TraceLogElement::getNamesAndAliases() +{ + String build_id_hex; +#if defined(__ELF__) && !defined(OS_FREEBSD) + build_id_hex = SymbolIndex::instance().getBuildIDHex(); +#endif + return + { + {"build_id", std::make_shared(), "\'" + build_id_hex + "\'"}, + }; +} + void TraceLogElement::appendToBlock(MutableColumns & columns) const { size_t i = 0; diff --git a/src/Interpreters/TraceLog.h b/src/Interpreters/TraceLog.h index 418b8d546a0..c4314cfd7b0 100644 --- a/src/Interpreters/TraceLog.h +++ b/src/Interpreters/TraceLog.h @@ -39,7 +39,7 @@ struct TraceLogElement static std::string name() { return "TraceLog"; } static ColumnsDescription getColumnsDescription(); - static NamesAndAliases getNamesAndAliases() { return {}; } + static NamesAndAliases getNamesAndAliases(); void appendToBlock(MutableColumns & columns) const; }; From 77a8a0ce98d5ea66fb8b76af7f89a245866ad315 Mon Sep 17 00:00:00 2001 From: woodlzm Date: Fri, 10 May 2024 23:29:07 -0700 Subject: [PATCH 2/7] Add stateless test for build_id column addition in trace_log. --- .../0_stateless/03150_trace_log_add_build_id.reference | 2 ++ tests/queries/0_stateless/03150_trace_log_add_build_id.sql | 6 ++++++ 2 files changed, 8 insertions(+) create mode 100644 tests/queries/0_stateless/03150_trace_log_add_build_id.reference create mode 100644 tests/queries/0_stateless/03150_trace_log_add_build_id.sql diff --git a/tests/queries/0_stateless/03150_trace_log_add_build_id.reference b/tests/queries/0_stateless/03150_trace_log_add_build_id.reference new file mode 100644 index 00000000000..0d66ea1aee9 --- /dev/null +++ b/tests/queries/0_stateless/03150_trace_log_add_build_id.reference @@ -0,0 +1,2 @@ +0 +1 diff --git a/tests/queries/0_stateless/03150_trace_log_add_build_id.sql b/tests/queries/0_stateless/03150_trace_log_add_build_id.sql new file mode 100644 index 00000000000..1f7bf1c02de --- /dev/null +++ b/tests/queries/0_stateless/03150_trace_log_add_build_id.sql @@ -0,0 +1,6 @@ +SELECT sleep(1); + +SYSTEM FLUSH LOGS; + +SELECT COUNT(*) > 1 FROM system.trace_log WHERE build_id IS NOT NULL; + From 7d809cbe9bb25cf292faf5a314614d45f0c4b6de Mon Sep 17 00:00:00 2001 From: woodlzm Date: Fri, 10 May 2024 23:30:48 -0700 Subject: [PATCH 3/7] Add integration test for checking new build_id column in trace_log plus the table renaming behavior upon binary changes. --- .../test_trace_log_build_id/__init__.py | 0 .../test_trace_log_build_id/test.py | 69 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 tests/integration/test_trace_log_build_id/__init__.py create mode 100644 tests/integration/test_trace_log_build_id/test.py diff --git a/tests/integration/test_trace_log_build_id/__init__.py b/tests/integration/test_trace_log_build_id/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_trace_log_build_id/test.py b/tests/integration/test_trace_log_build_id/test.py new file mode 100644 index 00000000000..b4a49b2e4d3 --- /dev/null +++ b/tests/integration/test_trace_log_build_id/test.py @@ -0,0 +1,69 @@ +import pytest +from helpers.cluster import ClickHouseCluster, CLICKHOUSE_CI_MIN_TESTED_VERSION + +TEST_QUERY_ID = "test_trace_log_build_id_query_{}" +OLD_TEST_QUERY_ID = TEST_QUERY_ID.format('0') +NEW_TEST_QUERY_ID = TEST_QUERY_ID.format('1') +ACTIVE_TRACE_LOG_TABLE = "trace_log" +RENAMED_TRACE_LOG_TABLE = "trace_log_0" + +cluster = ClickHouseCluster(__file__) +node = cluster.add_instance( + "node", + with_zookeeper=True, + image="clickhouse/clickhouse-server", + tag=CLICKHOUSE_CI_MIN_TESTED_VERSION, + stay_alive=True, + with_installed_binary=True, +) + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + + yield cluster + + except Exception as ex: + print(ex) + + finally: + cluster.shutdown() + + +def test_trace_log_build_id(started_cluster): + # This test checks that build_id column of system_log.trace_log is non-empty, and gets renamed when binary version changes. + # We make queries to create entries in trace_log, then restart with new version and verify if the old + # trace_log table is renamed and a new trace_log table is created. + + + query_for_table_name = "EXISTS TABLE system.{table}" + + node.query( + "SELECT sleep(1)", + query_id=OLD_TEST_QUERY_ID, + ) + node.query("SYSTEM FLUSH LOGS") + assert node.query(query_for_table_name.format(table=ACTIVE_TRACE_LOG_TABLE)) == "1\n" + assert node.query(query_for_table_name.format(table=RENAMED_TRACE_LOG_TABLE)) == "0\n" + + node.restart_with_latest_version() + + query_for_test_query_id = """ + SELECT EXISTS + ( + SELECT * + FROM system.{table} + WHERE query_id = \'{query_id}\' + ) + """ + node.query( + "SELECT sleep(1)", + query_id=NEW_TEST_QUERY_ID, + ) + node.query("SYSTEM FLUSH LOGS") + assert node.query(query_for_test_query_id.format(table=ACTIVE_TRACE_LOG_TABLE, query_id=OLD_TEST_QUERY_ID)) == "0\n" + assert node.query(query_for_test_query_id.format(table=ACTIVE_TRACE_LOG_TABLE, query_id=NEW_TEST_QUERY_ID)) == "1\n" + assert node.query(query_for_test_query_id.format(table=RENAMED_TRACE_LOG_TABLE, query_id=OLD_TEST_QUERY_ID)) == "1\n" + From 7dca8c0f75c104715e2c626fd16f1f57b71e1321 Mon Sep 17 00:00:00 2001 From: woodlzm Date: Sat, 11 May 2024 12:28:12 -0700 Subject: [PATCH 4/7] Fix styles for test.py. --- .../test_trace_log_build_id/test.py | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_trace_log_build_id/test.py b/tests/integration/test_trace_log_build_id/test.py index b4a49b2e4d3..26ef3684f86 100644 --- a/tests/integration/test_trace_log_build_id/test.py +++ b/tests/integration/test_trace_log_build_id/test.py @@ -2,8 +2,8 @@ import pytest from helpers.cluster import ClickHouseCluster, CLICKHOUSE_CI_MIN_TESTED_VERSION TEST_QUERY_ID = "test_trace_log_build_id_query_{}" -OLD_TEST_QUERY_ID = TEST_QUERY_ID.format('0') -NEW_TEST_QUERY_ID = TEST_QUERY_ID.format('1') +OLD_TEST_QUERY_ID = TEST_QUERY_ID.format("0") +NEW_TEST_QUERY_ID = TEST_QUERY_ID.format("1") ACTIVE_TRACE_LOG_TABLE = "trace_log" RENAMED_TRACE_LOG_TABLE = "trace_log_0" @@ -37,7 +37,6 @@ def test_trace_log_build_id(started_cluster): # We make queries to create entries in trace_log, then restart with new version and verify if the old # trace_log table is renamed and a new trace_log table is created. - query_for_table_name = "EXISTS TABLE system.{table}" node.query( @@ -45,8 +44,12 @@ def test_trace_log_build_id(started_cluster): query_id=OLD_TEST_QUERY_ID, ) node.query("SYSTEM FLUSH LOGS") - assert node.query(query_for_table_name.format(table=ACTIVE_TRACE_LOG_TABLE)) == "1\n" - assert node.query(query_for_table_name.format(table=RENAMED_TRACE_LOG_TABLE)) == "0\n" + assert ( + node.query(query_for_table_name.format(table=ACTIVE_TRACE_LOG_TABLE)) == "1\n" + ) + assert ( + node.query(query_for_table_name.format(table=RENAMED_TRACE_LOG_TABLE)) == "0\n" + ) node.restart_with_latest_version() @@ -63,7 +66,28 @@ def test_trace_log_build_id(started_cluster): query_id=NEW_TEST_QUERY_ID, ) node.query("SYSTEM FLUSH LOGS") - assert node.query(query_for_test_query_id.format(table=ACTIVE_TRACE_LOG_TABLE, query_id=OLD_TEST_QUERY_ID)) == "0\n" - assert node.query(query_for_test_query_id.format(table=ACTIVE_TRACE_LOG_TABLE, query_id=NEW_TEST_QUERY_ID)) == "1\n" - assert node.query(query_for_test_query_id.format(table=RENAMED_TRACE_LOG_TABLE, query_id=OLD_TEST_QUERY_ID)) == "1\n" + assert ( + node.query( + query_for_test_query_id.format( + table=ACTIVE_TRACE_LOG_TABLE, query_id=OLD_TEST_QUERY_ID + ) + ) + == "0\n" + ) + assert ( + node.query( + query_for_test_query_id.format( + table=ACTIVE_TRACE_LOG_TABLE, query_id=NEW_TEST_QUERY_ID + ) + ) + == "1\n" + ) + assert ( + node.query( + query_for_test_query_id.format( + table=RENAMED_TRACE_LOG_TABLE, query_id=OLD_TEST_QUERY_ID + ) + ) + == "1\n" + ) From 06409bf53ddc9fade924532cc6bb58f150ca8934 Mon Sep 17 00:00:00 2001 From: woodlzm Date: Sat, 11 May 2024 12:28:12 -0700 Subject: [PATCH 5/7] Fix styles for test.py. --- tests/integration/test_trace_log_build_id/test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_trace_log_build_id/test.py b/tests/integration/test_trace_log_build_id/test.py index 26ef3684f86..84392ab12b1 100644 --- a/tests/integration/test_trace_log_build_id/test.py +++ b/tests/integration/test_trace_log_build_id/test.py @@ -90,4 +90,3 @@ def test_trace_log_build_id(started_cluster): ) == "1\n" ) - From 0202bc1c7550f627bdc8013527804b0ec65e395b Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 14 May 2024 18:05:00 +0000 Subject: [PATCH 6/7] Skip the added tests in sanitizers where trace_log is disabled. --- tests/integration/test_trace_log_build_id/test.py | 7 ++++++- tests/queries/0_stateless/03150_trace_log_add_build_id.sql | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_trace_log_build_id/test.py b/tests/integration/test_trace_log_build_id/test.py index 84392ab12b1..fb4316e91bf 100644 --- a/tests/integration/test_trace_log_build_id/test.py +++ b/tests/integration/test_trace_log_build_id/test.py @@ -37,10 +37,15 @@ def test_trace_log_build_id(started_cluster): # We make queries to create entries in trace_log, then restart with new version and verify if the old # trace_log table is renamed and a new trace_log table is created. + if node.is_built_with_sanitizer(): + pytest.skip( + "Sanitizers are skipped, because trace_log is disabled with sanitizers." + ) + query_for_table_name = "EXISTS TABLE system.{table}" node.query( - "SELECT sleep(1)", + "SELECT sleep(2)", query_id=OLD_TEST_QUERY_ID, ) node.query("SYSTEM FLUSH LOGS") diff --git a/tests/queries/0_stateless/03150_trace_log_add_build_id.sql b/tests/queries/0_stateless/03150_trace_log_add_build_id.sql index 1f7bf1c02de..75122de47b5 100644 --- a/tests/queries/0_stateless/03150_trace_log_add_build_id.sql +++ b/tests/queries/0_stateless/03150_trace_log_add_build_id.sql @@ -1,5 +1,9 @@ -SELECT sleep(1); +-- Tags: no-asan, no-tsan, no-msan, no-ubsan, no-sanitize-coverage +SET log_queries = 1; +SET log_query_threads = 1; +SET query_profiler_real_time_period_ns = 100000000; +SELECT sleep(1); SYSTEM FLUSH LOGS; SELECT COUNT(*) > 1 FROM system.trace_log WHERE build_id IS NOT NULL; From bdca4c73fc8a3501bb09c28d35c80376751445c5 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 14 May 2024 18:05:00 +0000 Subject: [PATCH 7/7] Skip the added tests in sanitizers where trace_log is disabled, and add a new sanitizer_check_node to facilitate checking sanitizer config existence. --- tests/integration/test_trace_log_build_id/test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_trace_log_build_id/test.py b/tests/integration/test_trace_log_build_id/test.py index fb4316e91bf..8d654aef342 100644 --- a/tests/integration/test_trace_log_build_id/test.py +++ b/tests/integration/test_trace_log_build_id/test.py @@ -16,6 +16,7 @@ node = cluster.add_instance( stay_alive=True, with_installed_binary=True, ) +sanitizer_check_node = cluster.add_instance("sanitizer_check_node") @pytest.fixture(scope="module") @@ -37,7 +38,7 @@ def test_trace_log_build_id(started_cluster): # We make queries to create entries in trace_log, then restart with new version and verify if the old # trace_log table is renamed and a new trace_log table is created. - if node.is_built_with_sanitizer(): + if sanitizer_check_node.is_built_with_sanitizer(): pytest.skip( "Sanitizers are skipped, because trace_log is disabled with sanitizers." ) @@ -67,7 +68,7 @@ def test_trace_log_build_id(started_cluster): ) """ node.query( - "SELECT sleep(1)", + "SELECT sleep(2)", query_id=NEW_TEST_QUERY_ID, ) node.query("SYSTEM FLUSH LOGS")