Merge branch 'master' of github.com:ClickHouse/ClickHouse into sentry

base/common/phdr_cache.cpp

@@ -1,20 +1,6 @@
 /// This code was based on the code by Fedor Korotkiy (prime@yandex-team.ru) for YT product in Yandex.
 
-#if defined(__has_feature)
-    #if __has_feature(address_sanitizer)
-        #define ADDRESS_SANITIZER 1
-    #endif
-    #if __has_feature(thread_sanitizer)
-        #define THREAD_SANITIZER 1
-    #endif
-#else
-    #if defined(__SANITIZE_ADDRESS__)
-        #define ADDRESS_SANITIZER 1
-    #endif
-    #if defined(__SANITIZE_THREAD__)
-        #define THREAD_SANITIZER 1
-    #endif
-#endif
+#include <common/defines.h>
 
 #if defined(__linux__) && !defined(THREAD_SANITIZER)
     #define USE_PHDR_CACHE 1

docs/ru/development/style.md

@@ -688,10 +688,77 @@ auto s = std::string{"Hello"};
 
 ## Неиспользуемые возможности языка C++ {#neispolzuemye-vozmozhnosti-iazyka-c}
 
-**1.** Виртуальное наследование не используется.
-
 **2.** Спецификаторы исключений из C++03 не используются.
 
+## Сообщения об ошибках {#error-messages}
+
+Сообщения об ошибках -- это часть пользовательского интерфейса программы, предназначенная для того, чтобы позволить пользователю:
+* замечать ошибочные ситуации,
+* понимать их смысл и причины,
+* устранять эти ситуации.
+
+Форма и содержание сообщений об ошибках должны способствовать достижению этих целей.
+
+Есть два основных вида ошибок:
+* пользовательская или системная ошибка,
+* внутренняя программная ошибка.
+
+### Пользовательская ошибка {#error-messages-user-error}
+
+Такая ошибка вызвана действиями пользователя (неверный синтаксис запроса) или конфигурацией внешних систем (кончилось место на диске). Предполагается, что пользователь может устранить её самостоятельно. Для этого в сообщении об ошибке должна содержаться следующая информация:
+* что произошло. Это должно объясняться в пользовательских терминах (`Function pow() is not supported for data type UInt128`), а не загадочными конструкциями из кода (`runtime overload resolution failed in DB::BinaryOperationBuilder<FunctionAdaptor<pow>::Impl, UInt128, Int8>::kaboongleFastPath()`).
+* почему/где/когда -- любой контекст, который помогает отладить проблему. Представьте, как бы её отлаживали вы (программировать и пользоваться отладчиком нельзя).
+* что можно предпринять для устранения ошибки. Здесь можно перечислить типичные причины проблемы, настройки, влияющие на это поведение, и так далее.
+
+Пример нормального сообщения:
+```
+No alias for subquery or table function in JOIN (set joined_subquery_requires_alias=0 to disable restriction).
+While processing '(SELECT 2 AS a)'.
+```
+Сказано что не хватает алиаса, показано, для какой части запроса, и предложена настройка, позволяющая ослабить это требование.
+
+Пример катастрофически плохого сообщения:
+```
+The dictionary is configured incorrectly.
+```
+Из него не понятно:
+- какой словарь?
+- в чём ошибка конфигурации?
+
+Что может сделать пользователь в такой ситуации: применять внешние отладочные инструменты, спрашивать совета на форумах, гадать на кофейной гуще, и, конечно же, ненавидеть софт, который над ним так издевается. Не нужно издеваться над пользователями, это плохой UX.
+
+
+### Внутренняя программная ошибка {#error-messages-internal-error}
+
+Такая ошибка вызвана нарушением внутренних инвариантов программы: например, внутренняя функция вызвана с неверными параметрами, не совпадают размеры колонок в блоке, произошло разыменование нулевого указателя, и так далее. Сигналы типа `SIGSEGV` относятся к этой же категории.
+
+Появление такой ошибки всегда свидетельствует о наличии бага в программе. Пользователь не может исправить такую ошибку самостоятельно, и должен сообщить о ней разработчикам.
+
+Есть два основных варианта проверки на такие ошибки:
+* Исключение с кодом `LOGICAL_ERROR`. Его можно использовать для важных проверок, которые делаются в том числе в релизной сборке.
+* `assert`. Такие условия не проверяются в релизной сборке, можно использовать для тяжёлых и опциональных проверок.	
+
+Пример сообщения, у которого должен быть код `LOGICAL_ERROR`:
+`Block header is inconsistent with Chunk in ICompicatedProcessor::munge(). It is a bug!`
+По каким признакам можно заметить, что здесь говорится о внутренней программной ошибке? 
+* в сообщении упоминаются внутренние сущности из кода,
+* в сообщении написано it's a bug,
+* непосредственные действия пользователя не могут исправить эту ошибку. Мы ожидаем, что пользователь зарепортит её как баг, и будем исправлять в коде.
+
+### Как выбрать код ошибки? {#error-messages-choose}
+
+Код ошибки предназначен для автоматической обработки некоторых видов ошибок, подобно кодам HTTP. SQL стандартизирует некоторые коды, но на деле ClickHouse не всегда соответствует этим стандартам. Лучше всего выбрать существующий код из `ErrorCodes.cpp`, который больше всего подходит по смыслу. Можно использовать общие коды типа `BAD_ARGUMENTS` или `TYPE_MISMATCH`. Заводить новый код нужно, только если вы чётко понимаете, что вам нужна специальная автоматическая обработка конкретно этой ошибки на клиенте. Для внутренних программных ошибок используется код `LOGICAL_ERROR`.
+
+### Как добавить новое сообщение об ошибке? {#error-messages-add}
+
+Когда добавляете сообщение об ошибке:
+1. Опишите, что произошло, в пользовательских терминах, а не кусками кода.
+2. Добавьте максимум контекста (с чем произошло, когда, почему, и т.д.).
+3. Добавьте типичные причины.
+4. Добавьте варианты исправления (настройки, ссылки на документацию).
+5. Вообразите дальнейшие действия пользователя. Ваше сообщение должно помочь ему решить проблему без использования отладочных инструментов и без чужой помощи.
+6. Если сообщение об ошибке не формулируется в пользовательских терминах, и действия пользователя не могут исправить проблему -- это внутренняя программная ошибка, используйте код LOGICAL_ERROR или assert.
+
 ## Платформа {#platforma}
 
 **1.** Мы пишем код под конкретные платформы.

programs/server/config.xml

@@ -199,7 +199,7 @@
 
     <!-- Path to folder where users and roles created by SQL commands are stored. -->
     <access_control_path>/var/lib/clickhouse/access/</access_control_path>
-    
+
     <!-- Path to configuration file with users, access rights, profiles of settings, quotas. -->
     <users_config>users.xml</users_config>
 
@@ -413,9 +413,6 @@
     </prometheus>
     -->
 
-    <!-- Lazy system.*_log table creation -->
-    <!-- <system_tables_lazy_load>false</system_tables_lazy_load> -->
-
     <!-- Query log. Used only for queries with setting log_queries = 1. -->
     <query_log>
         <!-- What table to insert data. If table is not exist, it will be created.

src/Common/tests/gtest_rw_lock.cpp

@@ -5,6 +5,7 @@
 #include <Common/Stopwatch.h>
 #include <common/types.h>
 #include <Common/ThreadPool.h>
+#include <common/phdr_cache.h>
 #include <random>
 #include <pcg_random.hpp>
 #include <thread>
@@ -25,6 +26,13 @@ namespace DB
 
 TEST(Common, RWLock1)
 {
+    /// Tests with threads require this, because otherwise
+    ///  when tested under Memory Sanitizer,
+    ///  it tries to obtain stack trace on 'free' invocation at thread exit,
+    ///  but cannot do that due to infinite recursion.
+    /// Alternative solution: disable PHDR Cache under memory sanitizer.
+    updatePHDRCache();
+
     constexpr int cycles = 1000;
     const std::vector<size_t> pool_sizes{1, 2, 4, 8};
 
@@ -92,6 +100,8 @@ TEST(Common, RWLock1)
 
 TEST(Common, RWLockRecursive)
 {
+    updatePHDRCache();
+
     constexpr auto cycles = 10000;
 
     static auto fifo_lock = RWLockImpl::create();
@@ -134,6 +144,8 @@ TEST(Common, RWLockRecursive)
 
 TEST(Common, RWLockDeadlock)
 {
+    updatePHDRCache();
+
     static auto lock1 = RWLockImpl::create();
     static auto lock2 = RWLockImpl::create();
 
@@ -216,6 +228,8 @@ TEST(Common, RWLockDeadlock)
 
 TEST(Common, RWLockPerfTestReaders)
 {
+    updatePHDRCache();
+
     constexpr int cycles = 100000; // 100k
     const std::vector<size_t> pool_sizes{1, 2, 4, 8};
 

src/Interpreters/SystemLog.cpp

@@ -89,16 +89,10 @@ SystemLogs::SystemLogs(Context & global_context, const Poco::Util::AbstractConfi
     if (metric_log)
         logs.emplace_back(metric_log.get());
 
-    bool lazy_load = config.getBool("system_tables_lazy_load", false);
-
     try
     {
         for (auto & log : logs)
-        {
-            if (!lazy_load)
-                log->prepareTable();
             log->startup();
-        }
     }
     catch (...)
     {

tests/integration/test_part_log_table/test.py

@@ -24,18 +24,16 @@ def test_config_without_part_log(start_cluster):
     node1.query("SYSTEM FLUSH LOGS")
     assert "Table system.part_log doesn't exist" in node1.query_and_get_error("SELECT * FROM system.part_log")
 
+# Note: if part_log is defined, we cannot say when the table will be created - because of metric_log, trace_log, text_log, query_log...
+
 def test_config_with_standard_part_log(start_cluster):
-    assert node2.query("SELECT * FROM system.part_log") == ''
     node2.query("CREATE TABLE test_table(word String, value UInt64) ENGINE=MergeTree() Order by value")
-    assert node2.query("SELECT * FROM system.part_log") == ''
     node2.query("INSERT INTO test_table VALUES ('name', 1)")
     node2.query("SYSTEM FLUSH LOGS")
-    assert int(node2.query("SELECT count() FROM system.part_log")) == 1
+    assert node2.query("SELECT * FROM system.part_log") != ""
 
 def test_config_with_non_standard_part_log(start_cluster):
-    assert node3.query("SELECT * FROM system.own_part_log") == ''
     node3.query("CREATE TABLE test_table(word String, value UInt64) ENGINE=MergeTree() Order by value")
-    assert node3.query("SELECT * FROM system.own_part_log") == ''
     node3.query("INSERT INTO test_table VALUES ('name', 1)")
     node3.query("SYSTEM FLUSH LOGS")
-    assert int(node3.query("SELECT count() FROM system.own_part_log")) == 1
+    assert node3.query("SELECT * FROM system.own_part_log") != ""

tests/integration/test_system_tables_lazy_load/configs/config_lazy.xml

@@ -1,4 +0,0 @@
-<?xml version="1.0"?>
-<yandex>
-    <system_tables_lazy_load>true</system_tables_lazy_load>
-</yandex>

tests/integration/test_system_tables_lazy_load/test.py

@@ -1,32 +0,0 @@
-import pytest
-from helpers.cluster import ClickHouseCluster
-
-cluster = ClickHouseCluster(__file__)
-node_default = cluster.add_instance('node_default')
-# main_configs is mandatory ,since system_tables_lazy_load will be read earlier then parsing of config_lazy.xml
-node_lazy = cluster.add_instance('node_lazy', config_dir='configs', main_configs=['configs/config_lazy.xml'])
-
-system_logs = [
-    # disabled by default
-    # ('system.part_log'),
-    # ('system.text_log'),
-
-    # enabled by default
-    ('system.query_log'),
-    ('system.query_thread_log'),
-    ('system.trace_log'),
-    ('system.metric_log'),
-]
-
-@pytest.fixture(scope='module')
-def start_cluster():
-    try:
-        cluster.start()
-        yield cluster
-    finally:
-        cluster.shutdown()
-
-@pytest.mark.parametrize('table', system_logs)
-def test_system_table(start_cluster, table):
-    node_default.query('SELECT * FROM {}'.format(table))
-    assert "Table {} doesn't exist".format(table) in node_lazy.query_and_get_error('SELECT * FROM {}'.format(table))