From 6b7a3eac40b96dec9b3aa9904f70f2d33297b30b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 31 Mar 2023 12:21:01 +0200 Subject: [PATCH 01/40] Update autogenerated version to 23.3.2.1 and contributors --- cmake/autogenerated_versions.txt | 8 ++--- .../StorageSystemContributors.generated.cpp | 35 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/cmake/autogenerated_versions.txt b/cmake/autogenerated_versions.txt index b52b2eda992d..055430fa0308 100644 --- a/cmake/autogenerated_versions.txt +++ b/cmake/autogenerated_versions.txt @@ -5,8 +5,8 @@ SET(VERSION_REVISION 54472) SET(VERSION_MAJOR 23) SET(VERSION_MINOR 3) -SET(VERSION_PATCH 1) -SET(VERSION_GITHASH 52bf836e03a6ba7cf2d654eaaf73231701abc3a2) -SET(VERSION_DESCRIBE v23.3.1.2537-testing) -SET(VERSION_STRING 23.3.1.2537) +SET(VERSION_PATCH 2) +SET(VERSION_GITHASH 46e85357ce2da2a99f56ee83a079e892d7ec3726) +SET(VERSION_DESCRIBE v23.3.2.1-lts) +SET(VERSION_STRING 23.3.2.1) # end of autochange diff --git a/src/Storages/System/StorageSystemContributors.generated.cpp b/src/Storages/System/StorageSystemContributors.generated.cpp index ca19687918cc..a6704144fdef 100644 --- a/src/Storages/System/StorageSystemContributors.generated.cpp +++ b/src/Storages/System/StorageSystemContributors.generated.cpp @@ -35,6 +35,7 @@ const char * auto_contributors[] { "Aleksei Filatov", "Aleksei Levushkin", "Aleksei Semiglazov", + "Aleksei Tikhomirov", "Aleksey", "Aleksey Akulovich", "Alex", @@ -84,6 +85,7 @@ const char * auto_contributors[] { "Alexey Gusev", "Alexey Ilyukhov", "Alexey Ivanov", + "Alexey Korepanov", "Alexey Milovidov", "Alexey Perevyshin", "Alexey Tronov", @@ -135,6 +137,7 @@ const char * auto_contributors[] { "Andrii R", "Andy Liang", "Andy Yang", + "AndyB", "Anish Bhanwala", "Anmol Arora", "Anna", @@ -155,6 +158,7 @@ const char * auto_contributors[] { "Anton Yuzhaninov", "Anton Zhabolenko", "Antonio Andelic", + "Antonio Bonuccelli", "Ariel Robaldo", "Arsen Hakobyan", "Arslan G", @@ -227,6 +231,7 @@ const char * auto_contributors[] { "Christoph Wurm", "Chun-Sheng, Li", "Ciprian Hacman", + "Clayton McClure", "Clement Rodriguez", "ClickHouse Admin", "Clément Rodriguez", @@ -256,6 +261,7 @@ const char * auto_contributors[] { "Dario", "DarkWanderer", "Darío", + "Dave Lahn", "Denis Burlaka", "Denis Glazachev", "Denis Krivak", @@ -391,6 +397,7 @@ const char * auto_contributors[] { "HeenaBansal2009", "Hiroaki Nakamura", "Hongbin", + "Hosun Lee", "HuFuwang", "Hui Wang", "ILya Limarenko", @@ -457,12 +464,15 @@ const char * auto_contributors[] { "Jiebin Sun", "Joanna Hulboj", "Jochen Schalanda", + "Joey", + "Johannes Visintini", "John", "John Hummel", "John Skopis", "Jonatas Freitas", "Jonathan-Ackerman", "Jordi Villar", + "Joris Giovannangeli", "Jose", "Josh Taylor", "João Figueiredo", @@ -481,6 +491,7 @@ const char * auto_contributors[] { "Kevin Chiang", "Kevin Michel", "Kevin Zhang", + "KevinyhZou", "KinderRiven", "Kiran", "Kirill Danshin", @@ -525,6 +536,7 @@ const char * auto_contributors[] { "Li Yin", "Liu Cong", "LiuCong", + "LiuNeng", "LiuYangkuan", "Lloyd-Pottiger", "Lopatin Konstantin", @@ -544,6 +556,7 @@ const char * auto_contributors[] { "Maksim Buren", "Maksim Fedotov", "Maksim Kita", + "Maksym Sobolyev", "Mallik Hassan", "Malte", "Manuel de la Peña", @@ -625,6 +638,7 @@ const char * auto_contributors[] { "Mikhail Salosin", "Mikhail Surin", "Mikhail f. Shiryaev", + "MikhailBurdukov", "MikuSugar", "Milad Arabi", "Mingliang Pan", @@ -694,6 +708,7 @@ const char * auto_contributors[] { "OmarBazaraa", "OnePiece", "Onehr7", + "Ongkong", "Orivej Desh", "Orkhan Zeynalli", "Oskar Wojciski", @@ -701,6 +716,7 @@ const char * auto_contributors[] { "PHO", "Pablo Alegre", "Pablo Marcos", + "Palash Goel", "Paramtamtam", "Patrick Zippenfenig", "Paul Loyd", @@ -795,6 +811,7 @@ const char * auto_contributors[] { "Sergei Bocharov", "Sergei Semin", "Sergei Shtykov", + "Sergei Solomatov", "Sergei Trifonov", "Sergei Tsetlin (rekub)", "Sergey Demurin", @@ -844,6 +861,7 @@ const char * auto_contributors[] { "Stupnikov Andrey", "SuperBot", "SuperDJY", + "SupunKavinda", "Suzy Wang", "SuzyWangIBMer", "Sébastien", @@ -865,6 +883,7 @@ const char * auto_contributors[] { "The-Alchemist", "Thom O'Connor", "Thomas Berdy", + "Thomas Casteleyn", "Tian Xinhui", "Tiaonmmn", "Tigran Khudaverdyan", @@ -996,6 +1015,7 @@ const char * auto_contributors[] { "Zhipeng", "Zijie Lu", "Zoran Pandovski", + "[데이터플랫폼팀] 이호선", "a.palagashvili", "aaapetrenko", "abdrakhmanov", @@ -1011,6 +1031,7 @@ const char * auto_contributors[] { "akuzm", "alekseik1", "alesapin", + "alex filatov", "alex-zaitsev", "alex.lvxin", "alexX512", @@ -1035,12 +1056,14 @@ const char * auto_contributors[] { "anton", "ap11", "aprudaev", + "artem-yadr", "artpaul", "asiana21", "atereh", "attack204", "avasiliev", "avogar", + "avoiderboi", "avsharapov", "awakeljw", "awesomeleo", @@ -1079,6 +1102,7 @@ const char * auto_contributors[] { "chou.fan", "christophe.kalenzaga", "clarkcaoliu", + "clickhouse-adrianfraguela", "clickhouse-robot-curie", "cms", "cmsxbc", @@ -1111,6 +1135,7 @@ const char * auto_contributors[] { "dmitriy", "dmitry kuzmin", "dongyifeng", + "ducle.canh", "eaxdev", "eejoin", "egatov", @@ -1123,6 +1148,8 @@ const char * auto_contributors[] { "erikbaan", "ermaotech", "evtan", + "exX512", + "exmy", "exprmntr", "ezhaka", "f1yegor", @@ -1152,6 +1179,7 @@ const char * auto_contributors[] { "fuwhu", "fuzhe1989", "fuzzERot", + "fyu", "g-arslan", "ggerogery", "giordyb", @@ -1178,9 +1206,11 @@ const char * auto_contributors[] { "hhell", "homeward", "hotid", + "houbaron", "huangzhaowei", "hustnn", "huzhichengdd", + "iammagicc", "ianton-ru", "ice1x", "idfer", @@ -1212,6 +1242,7 @@ const char * auto_contributors[] { "jinjunzh", "jkuklis", "jthmath", + "jun won", "jus1096", "jyz0309", "karnevil13", @@ -1223,6 +1254,7 @@ const char * auto_contributors[] { "kigerzhang", "kirillikoff", "kmeaw", + "kolechenkov", "koloshmet", "kolsys", "konnectr", @@ -1252,6 +1284,7 @@ const char * auto_contributors[] { "liangqian", "libenwang", "lichengxiang", + "liding1992", "linceyou", "lincion", "lingo-xp", @@ -1330,6 +1363,7 @@ const char * auto_contributors[] { "nauta", "nautaa", "ndchikin", + "nellicus", "neng.liu", "never lee", "ni1l", @@ -1545,6 +1579,7 @@ const char * auto_contributors[] { "Дмитрий Канатников", "Иванов Евгений", "Илья Исаев", + "Коренберг Марк", "Коренберг ☢️ Марк", "Павел Литвиненко", "Смитюх Вячеслав", From 3d908aada33aa7da8c946ae8e69a7d5703d479d8 Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Sun, 2 Apr 2023 12:03:05 +0000 Subject: [PATCH 02/40] Backport #48314 to 23.3: Fix ThreadPool for DistributedSink and use StrongTypedef for CurrentMetrics/ProfileEvents/StatusInfo to avoid further errors --- base/base/strong_typedef.h | 2 +- programs/server/MetricsTransmitter.cpp | 6 +++--- src/Common/CurrentMetrics.cpp | 6 ++++-- src/Common/CurrentMetrics.h | 3 ++- src/Common/ProfileEvents.cpp | 10 +++++----- src/Common/ProfileEvents.h | 5 +++-- src/Common/StatusInfo.cpp | 4 ++-- src/Common/StatusInfo.h | 3 ++- src/Common/tests/gtest_thread_pool_limit.cpp | 9 ++++++++- src/Disks/TemporaryFileOnDisk.cpp | 2 +- src/Disks/TemporaryFileOnDisk.h | 2 +- src/Interpreters/DDLWorker.cpp | 8 ++++---- src/Interpreters/MetricLog.cpp | 4 ++-- src/Interpreters/ProfileEventsExt.cpp | 4 ++-- src/Interpreters/TemporaryDataOnDisk.cpp | 2 +- src/Interpreters/TemporaryDataOnDisk.h | 4 ++-- src/Server/PrometheusMetricsWriter.cpp | 2 +- src/Storages/Distributed/DistributedSink.cpp | 9 ++++++--- src/Storages/System/StorageSystemEvents.cpp | 2 +- utils/check-style/check-style | 1 + 20 files changed, 52 insertions(+), 36 deletions(-) diff --git a/base/base/strong_typedef.h b/base/base/strong_typedef.h index 2ddea6412f56..b3b8bced688c 100644 --- a/base/base/strong_typedef.h +++ b/base/base/strong_typedef.h @@ -35,7 +35,7 @@ struct StrongTypedef Self & operator=(T && rhs) { t = std::move(rhs); return *this;} // NOLINTBEGIN(google-explicit-constructor) - operator const T & () const { return t; } + constexpr operator const T & () const { return t; } operator T & () { return t; } // NOLINTEND(google-explicit-constructor) diff --git a/programs/server/MetricsTransmitter.cpp b/programs/server/MetricsTransmitter.cpp index 2f28f0a1d162..ae9fa5ecc2c9 100644 --- a/programs/server/MetricsTransmitter.cpp +++ b/programs/server/MetricsTransmitter.cpp @@ -87,7 +87,7 @@ void MetricsTransmitter::transmit(std::vector & prev_count if (send_events) { - for (size_t i = 0, end = ProfileEvents::end(); i < end; ++i) + for (ProfileEvents::Event i = ProfileEvents::Event(0), end = ProfileEvents::end(); i < end; ++i) { const auto counter = ProfileEvents::global_counters[i].load(std::memory_order_relaxed); const auto counter_increment = counter - prev_counters[i]; @@ -100,7 +100,7 @@ void MetricsTransmitter::transmit(std::vector & prev_count if (send_events_cumulative) { - for (size_t i = 0, end = ProfileEvents::end(); i < end; ++i) + for (ProfileEvents::Event i = ProfileEvents::Event(0), end = ProfileEvents::end(); i < end; ++i) { const auto counter = ProfileEvents::global_counters[i].load(std::memory_order_relaxed); std::string key{ProfileEvents::getName(static_cast(i))}; @@ -110,7 +110,7 @@ void MetricsTransmitter::transmit(std::vector & prev_count if (send_metrics) { - for (size_t i = 0, end = CurrentMetrics::end(); i < end; ++i) + for (CurrentMetrics::Metric i = CurrentMetrics::Metric(0), end = CurrentMetrics::end(); i < end; ++i) { const auto value = CurrentMetrics::values[i].load(std::memory_order_relaxed); diff --git a/src/Common/CurrentMetrics.cpp b/src/Common/CurrentMetrics.cpp index 542c48148c8b..cfe9f41befeb 100644 --- a/src/Common/CurrentMetrics.cpp +++ b/src/Common/CurrentMetrics.cpp @@ -126,6 +126,8 @@ M(DDLWorkerThreadsActive, "Number of threads in the DDLWORKER thread pool for ON CLUSTER queries running a task.") \ M(StorageDistributedThreads, "Number of threads in the StorageDistributed thread pool.") \ M(StorageDistributedThreadsActive, "Number of threads in the StorageDistributed thread pool running a task.") \ + M(DistributedInsertThreads, "Number of threads used for INSERT into Distributed.") \ + M(DistributedInsertThreadsActive, "Number of threads used for INSERT into Distributed running a task.") \ M(StorageS3Threads, "Number of threads in the StorageS3 thread pool.") \ M(StorageS3ThreadsActive, "Number of threads in the StorageS3 thread pool running a task.") \ M(MergeTreePartsLoaderThreads, "Number of threads in the MergeTree parts loader thread pool.") \ @@ -184,10 +186,10 @@ namespace CurrentMetrics { - #define M(NAME, DOCUMENTATION) extern const Metric NAME = __COUNTER__; + #define M(NAME, DOCUMENTATION) extern const Metric NAME = Metric(__COUNTER__); APPLY_FOR_METRICS(M) #undef M - constexpr Metric END = __COUNTER__; + constexpr Metric END = Metric(__COUNTER__); std::atomic values[END] {}; /// Global variable, initialized by zeros. diff --git a/src/Common/CurrentMetrics.h b/src/Common/CurrentMetrics.h index 0ae16e2d08d7..a1ef254485dd 100644 --- a/src/Common/CurrentMetrics.h +++ b/src/Common/CurrentMetrics.h @@ -6,6 +6,7 @@ #include #include #include +#include /** Allows to count number of simultaneously happening processes or current value of some metric. * - for high-level profiling. @@ -22,7 +23,7 @@ namespace CurrentMetrics { /// Metric identifier (index in array). - using Metric = size_t; + using Metric = StrongTypedef; using Value = DB::Int64; /// Get name of metric by identifier. Returns statically allocated string. diff --git a/src/Common/ProfileEvents.cpp b/src/Common/ProfileEvents.cpp index 3cee4a8e7181..1d035952f13c 100644 --- a/src/Common/ProfileEvents.cpp +++ b/src/Common/ProfileEvents.cpp @@ -497,10 +497,10 @@ The server successfully detected this situation and will download merged part fr namespace ProfileEvents { -#define M(NAME, DOCUMENTATION) extern const Event NAME = __COUNTER__; +#define M(NAME, DOCUMENTATION) extern const Event NAME = Event(__COUNTER__); APPLY_FOR_EVENTS(M) #undef M -constexpr Event END = __COUNTER__; +constexpr Event END = Event(__COUNTER__); /// Global variable, initialized by zeros. Counter global_counters_array[END] {}; @@ -522,7 +522,7 @@ void Counters::resetCounters() { if (counters) { - for (Event i = 0; i < num_counters; ++i) + for (Event i = Event(0); i < num_counters; ++i) counters[i].store(0, std::memory_order_relaxed); } } @@ -540,7 +540,7 @@ Counters::Snapshot::Snapshot() Counters::Snapshot Counters::getPartiallyAtomicSnapshot() const { Snapshot res; - for (Event i = 0; i < num_counters; ++i) + for (Event i = Event(0); i < num_counters; ++i) res.counters_holder[i] = counters[i].load(std::memory_order_relaxed); return res; } @@ -616,7 +616,7 @@ CountersIncrement::CountersIncrement(Counters::Snapshot const & snapshot) CountersIncrement::CountersIncrement(Counters::Snapshot const & after, Counters::Snapshot const & before) { init(); - for (Event i = 0; i < Counters::num_counters; ++i) + for (Event i = Event(0); i < Counters::num_counters; ++i) increment_holder[i] = static_cast(after[i]) - static_cast(before[i]); } diff --git a/src/Common/ProfileEvents.h b/src/Common/ProfileEvents.h index 867b5b551c62..a36e68742cf5 100644 --- a/src/Common/ProfileEvents.h +++ b/src/Common/ProfileEvents.h @@ -1,7 +1,8 @@ #pragma once #include -#include "base/types.h" +#include +#include #include #include #include @@ -14,7 +15,7 @@ namespace ProfileEvents { /// Event identifier (index in array). - using Event = size_t; + using Event = StrongTypedef; using Count = size_t; using Increment = Int64; using Counter = std::atomic; diff --git a/src/Common/StatusInfo.cpp b/src/Common/StatusInfo.cpp index 32afc8330013..1f9ddfaf4b9e 100644 --- a/src/Common/StatusInfo.cpp +++ b/src/Common/StatusInfo.cpp @@ -8,10 +8,10 @@ namespace CurrentStatusInfo { - #define M(NAME, DOCUMENTATION, ENUM) extern const Status NAME = __COUNTER__; + #define M(NAME, DOCUMENTATION, ENUM) extern const Status NAME = Status(__COUNTER__); APPLY_FOR_STATUS(M) #undef M - constexpr Status END = __COUNTER__; + constexpr Status END = Status(__COUNTER__); std::mutex locks[END] {}; std::unordered_map values[END] {}; diff --git a/src/Common/StatusInfo.h b/src/Common/StatusInfo.h index 9aa185cd0c36..91e6d4d3b850 100644 --- a/src/Common/StatusInfo.h +++ b/src/Common/StatusInfo.h @@ -6,13 +6,14 @@ #include #include #include +#include #include #include namespace CurrentStatusInfo { - using Status = size_t; + using Status = StrongTypedef; using Key = std::string; const char * getName(Status event); diff --git a/src/Common/tests/gtest_thread_pool_limit.cpp b/src/Common/tests/gtest_thread_pool_limit.cpp index bc67ffd0bc16..17f79d17894f 100644 --- a/src/Common/tests/gtest_thread_pool_limit.cpp +++ b/src/Common/tests/gtest_thread_pool_limit.cpp @@ -1,16 +1,23 @@ #include #include #include +#include #include +namespace CurrentMetrics +{ + extern const Metric LocalThread; + extern const Metric LocalThreadActive; +} + /// Test for thread self-removal when number of free threads in pool is too large. /// Just checks that nothing weird happens. template int test() { - Pool pool(10, 2, 10); + Pool pool(CurrentMetrics::LocalThread, CurrentMetrics::LocalThreadActive, 10, 2, 10); std::atomic counter{0}; for (size_t i = 0; i < 10; ++i) diff --git a/src/Disks/TemporaryFileOnDisk.cpp b/src/Disks/TemporaryFileOnDisk.cpp index d31b09b2185f..6fe6fd5a1c9a 100644 --- a/src/Disks/TemporaryFileOnDisk.cpp +++ b/src/Disks/TemporaryFileOnDisk.cpp @@ -27,7 +27,7 @@ TemporaryFileOnDisk::TemporaryFileOnDisk(const DiskPtr & disk_) : TemporaryFileOnDisk(disk_, "") {} -TemporaryFileOnDisk::TemporaryFileOnDisk(const DiskPtr & disk_, CurrentMetrics::Value metric_scope) +TemporaryFileOnDisk::TemporaryFileOnDisk(const DiskPtr & disk_, CurrentMetrics::Metric metric_scope) : TemporaryFileOnDisk(disk_) { sub_metric_increment.emplace(metric_scope); diff --git a/src/Disks/TemporaryFileOnDisk.h b/src/Disks/TemporaryFileOnDisk.h index 9ba59c3eaf0a..4c376383087f 100644 --- a/src/Disks/TemporaryFileOnDisk.h +++ b/src/Disks/TemporaryFileOnDisk.h @@ -17,7 +17,7 @@ class TemporaryFileOnDisk { public: explicit TemporaryFileOnDisk(const DiskPtr & disk_); - explicit TemporaryFileOnDisk(const DiskPtr & disk_, CurrentMetrics::Value metric_scope); + explicit TemporaryFileOnDisk(const DiskPtr & disk_, CurrentMetrics::Metric metric_scope); explicit TemporaryFileOnDisk(const DiskPtr & disk_, const String & prefix); ~TemporaryFileOnDisk(); diff --git a/src/Interpreters/DDLWorker.cpp b/src/Interpreters/DDLWorker.cpp index 22bece0ef042..c4529af2c516 100644 --- a/src/Interpreters/DDLWorker.cpp +++ b/src/Interpreters/DDLWorker.cpp @@ -1022,10 +1022,10 @@ String DDLWorker::enqueueQuery(DDLLogEntry & entry) { String str_buf = node_path.substr(query_path_prefix.length()); DB::ReadBufferFromString in(str_buf); - CurrentMetrics::Metric id; - readText(id, in); - id = std::max(*max_pushed_entry_metric, id); - CurrentMetrics::set(*max_pushed_entry_metric, id); + CurrentMetrics::Value pushed_entry; + readText(pushed_entry, in); + pushed_entry = std::max(CurrentMetrics::get(*max_pushed_entry_metric), pushed_entry); + CurrentMetrics::set(*max_pushed_entry_metric, pushed_entry); } /// We cannot create status dirs in a single transaction with previous request, diff --git a/src/Interpreters/MetricLog.cpp b/src/Interpreters/MetricLog.cpp index 6e98f84bc82f..578cc118a6b5 100644 --- a/src/Interpreters/MetricLog.cpp +++ b/src/Interpreters/MetricLog.cpp @@ -50,7 +50,7 @@ void MetricLogElement::appendToBlock(MutableColumns & columns) const columns[column_idx++]->insert(profile_events[i]); for (size_t i = 0, end = CurrentMetrics::end(); i < end; ++i) - columns[column_idx++]->insert(current_metrics[i]); + columns[column_idx++]->insert(current_metrics[i].toUnderType()); } @@ -97,7 +97,7 @@ void MetricLog::metricThreadFunction() elem.milliseconds = timeInMilliseconds(current_time) - timeInSeconds(current_time) * 1000; elem.profile_events.resize(ProfileEvents::end()); - for (size_t i = 0, end = ProfileEvents::end(); i < end; ++i) + for (ProfileEvents::Event i = ProfileEvents::Event(0), end = ProfileEvents::end(); i < end; ++i) { const ProfileEvents::Count new_value = ProfileEvents::global_counters[i].load(std::memory_order_relaxed); auto & old_value = prev_profile_events[i]; diff --git a/src/Interpreters/ProfileEventsExt.cpp b/src/Interpreters/ProfileEventsExt.cpp index 7fbbe3c662b0..44977524c565 100644 --- a/src/Interpreters/ProfileEventsExt.cpp +++ b/src/Interpreters/ProfileEventsExt.cpp @@ -32,7 +32,7 @@ void dumpToMapColumn(const Counters::Snapshot & counters, DB::IColumn * column, auto & value_column = tuple_column.getColumn(1); size_t size = 0; - for (Event event = 0; event < Counters::num_counters; ++event) + for (Event event = Event(0); event < Counters::num_counters; ++event) { UInt64 value = counters[event]; @@ -54,7 +54,7 @@ static void dumpProfileEvents(ProfileEventsSnapshot const & snapshot, DB::Mutabl size_t rows = 0; auto & name_column = columns[NAME_COLUMN_INDEX]; auto & value_column = columns[VALUE_COLUMN_INDEX]; - for (Event event = 0; event < Counters::num_counters; ++event) + for (Event event = Event(0); event < Counters::num_counters; ++event) { Int64 value = snapshot.counters[event]; diff --git a/src/Interpreters/TemporaryDataOnDisk.cpp b/src/Interpreters/TemporaryDataOnDisk.cpp index 25252f8226bc..c57de88d964c 100644 --- a/src/Interpreters/TemporaryDataOnDisk.cpp +++ b/src/Interpreters/TemporaryDataOnDisk.cpp @@ -49,7 +49,7 @@ TemporaryDataOnDisk::TemporaryDataOnDisk(TemporaryDataOnDiskScopePtr parent_) : TemporaryDataOnDiskScope(std::move(parent_), /* limit_ = */ 0) {} -TemporaryDataOnDisk::TemporaryDataOnDisk(TemporaryDataOnDiskScopePtr parent_, CurrentMetrics::Value metric_scope) +TemporaryDataOnDisk::TemporaryDataOnDisk(TemporaryDataOnDiskScopePtr parent_, CurrentMetrics::Metric metric_scope) : TemporaryDataOnDiskScope(std::move(parent_), /* limit_ = */ 0) , current_metric_scope(metric_scope) {} diff --git a/src/Interpreters/TemporaryDataOnDisk.h b/src/Interpreters/TemporaryDataOnDisk.h index f0e02f16fb69..f7a6249a1ee1 100644 --- a/src/Interpreters/TemporaryDataOnDisk.h +++ b/src/Interpreters/TemporaryDataOnDisk.h @@ -85,7 +85,7 @@ class TemporaryDataOnDisk : private TemporaryDataOnDiskScope explicit TemporaryDataOnDisk(TemporaryDataOnDiskScopePtr parent_); - explicit TemporaryDataOnDisk(TemporaryDataOnDiskScopePtr parent_, CurrentMetrics::Value metric_scope); + explicit TemporaryDataOnDisk(TemporaryDataOnDiskScopePtr parent_, CurrentMetrics::Metric metric_scope); /// If max_file_size > 0, then check that there's enough space on the disk and throw an exception in case of lack of free space TemporaryFileStream & createStream(const Block & header, size_t max_file_size = 0); @@ -102,7 +102,7 @@ class TemporaryDataOnDisk : private TemporaryDataOnDiskScope mutable std::mutex mutex; std::vector streams TSA_GUARDED_BY(mutex); - typename CurrentMetrics::Value current_metric_scope = CurrentMetrics::TemporaryFilesUnknown; + typename CurrentMetrics::Metric current_metric_scope = CurrentMetrics::TemporaryFilesUnknown; }; /* diff --git a/src/Server/PrometheusMetricsWriter.cpp b/src/Server/PrometheusMetricsWriter.cpp index abf2a2c0b6bc..2331e455225f 100644 --- a/src/Server/PrometheusMetricsWriter.cpp +++ b/src/Server/PrometheusMetricsWriter.cpp @@ -59,7 +59,7 @@ void PrometheusMetricsWriter::write(WriteBuffer & wb) const { if (send_events) { - for (size_t i = 0, end = ProfileEvents::end(); i < end; ++i) + for (ProfileEvents::Event i = ProfileEvents::Event(0), end = ProfileEvents::end(); i < end; ++i) { const auto counter = ProfileEvents::global_counters[i].load(std::memory_order_relaxed); diff --git a/src/Storages/Distributed/DistributedSink.cpp b/src/Storages/Distributed/DistributedSink.cpp index 11b938cd722b..720a951299a1 100644 --- a/src/Storages/Distributed/DistributedSink.cpp +++ b/src/Storages/Distributed/DistributedSink.cpp @@ -41,6 +41,8 @@ namespace CurrentMetrics { extern const Metric DistributedSend; + extern const Metric DistributedInsertThreads; + extern const Metric DistributedInsertThreadsActive; } namespace ProfileEvents @@ -460,9 +462,10 @@ void DistributedSink::writeSync(const Block & block) size_t jobs_count = random_shard_insert ? 1 : (remote_jobs_count + local_jobs_count); size_t max_threads = std::min(settings.max_distributed_connections, jobs_count); - pool.emplace(/* max_threads_= */ max_threads, - /* max_free_threads_= */ max_threads, - /* queue_size_= */ jobs_count); + pool.emplace( + CurrentMetrics::DistributedInsertThreads, + CurrentMetrics::DistributedInsertThreadsActive, + max_threads, max_threads, jobs_count); if (!throttler && (settings.max_network_bandwidth || settings.max_network_bytes)) { diff --git a/src/Storages/System/StorageSystemEvents.cpp b/src/Storages/System/StorageSystemEvents.cpp index be2d3f8d49ef..b9b07cfe0ac5 100644 --- a/src/Storages/System/StorageSystemEvents.cpp +++ b/src/Storages/System/StorageSystemEvents.cpp @@ -18,7 +18,7 @@ NamesAndTypesList StorageSystemEvents::getNamesAndTypes() void StorageSystemEvents::fillData(MutableColumns & res_columns, ContextPtr context, const SelectQueryInfo &) const { - for (size_t i = 0, end = ProfileEvents::end(); i < end; ++i) + for (ProfileEvents::Event i = ProfileEvents::Event(0), end = ProfileEvents::end(); i < end; ++i) { UInt64 value = ProfileEvents::global_counters[i]; diff --git a/utils/check-style/check-style b/utils/check-style/check-style index 7a1fa6ce123f..a6cc20bb7c85 100755 --- a/utils/check-style/check-style +++ b/utils/check-style/check-style @@ -78,6 +78,7 @@ EXTERN_TYPES_EXCLUDES=( CurrentMetrics::add CurrentMetrics::sub + CurrentMetrics::get CurrentMetrics::set CurrentMetrics::end CurrentMetrics::Increment From 7d350fbaedc696b54a3d7c4f1a8d74a4faeadcdb Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Thu, 6 Apr 2023 08:05:00 +0000 Subject: [PATCH 03/40] Backport #47246 to 23.3: Change the behavior of formatter %M in function formatDateTime() from minutes to month name --- .../functions/date-time-functions.md | 8 +- src/Common/Concepts.h | 4 + src/Common/typeid_cast.h | 3 - src/Core/Settings.h | 1 + src/Core/SettingsChangesHistory.h | 1 + src/Functions/FunctionsConversion.h | 1 + src/Functions/formatDateTime.cpp | 729 +++++++++++++----- src/Functions/parseDateTime.cpp | 82 +- src/Functions/widthBucket.cpp | 1 + .../00718_format_datetime.reference | 1 + .../0_stateless/00718_format_datetime.sql | 3 +- ...00921_datetime64_compatibility_long.python | 2 +- ...21_datetime64_compatibility_long.reference | 2 +- .../0_stateless/01411_from_unixtime.reference | 2 +- .../0_stateless/02564_date_format.reference | 2 +- .../02668_parse_datetime.reference | 19 + .../0_stateless/02668_parse_datetime.sql | 13 + 17 files changed, 637 insertions(+), 237 deletions(-) diff --git a/docs/en/sql-reference/functions/date-time-functions.md b/docs/en/sql-reference/functions/date-time-functions.md index d06ab253cf74..425d67ed5a05 100644 --- a/docs/en/sql-reference/functions/date-time-functions.md +++ b/docs/en/sql-reference/functions/date-time-functions.md @@ -1276,16 +1276,16 @@ Using replacement fields, you can define a pattern for the resulting string. “ | %k | hour in 24h format (00-23) | 22 | | %l | hour in 12h format (01-12) | 09 | | %m | month as an integer number (01-12) | 01 | -| %M | minute (00-59) | 33 | +| %M | full month name (January-December) | January | | %n | new-line character (‘’) | | | %p | AM or PM designation | PM | | %Q | Quarter (1-4) | 1 | -| %r | 12-hour HH:MM AM/PM time, equivalent to %H:%M %p | 10:30 PM | -| %R | 24-hour HH:MM time, equivalent to %H:%M | 22:33 | +| %r | 12-hour HH:MM AM/PM time, equivalent to %H:%i %p | 10:30 PM | +| %R | 24-hour HH:MM time, equivalent to %H:%i | 22:33 | | %s | second (00-59) | 44 | | %S | second (00-59) | 44 | | %t | horizontal-tab character (’) | | -| %T | ISO 8601 time format (HH:MM:SS), equivalent to %H:%M:%S | 22:33:44 | +| %T | ISO 8601 time format (HH:MM:SS), equivalent to %H:%i:%S | 22:33:44 | | %u | ISO 8601 weekday as number with Monday as 1 (1-7) | 2 | | %V | ISO 8601 week number (01-53) | 01 | | %w | weekday as a integer number with Sunday as 0 (0-6) | 2 | diff --git a/src/Common/Concepts.h b/src/Common/Concepts.h index b1bf591024d1..927f42aa4bea 100644 --- a/src/Common/Concepts.h +++ b/src/Common/Concepts.h @@ -5,6 +5,10 @@ namespace DB { +template +concept is_any_of = (std::same_as || ...); + + template concept OptionalArgument = requires(T &&...) { diff --git a/src/Common/typeid_cast.h b/src/Common/typeid_cast.h index 1568d3809389..baee3aaf6327 100644 --- a/src/Common/typeid_cast.h +++ b/src/Common/typeid_cast.h @@ -18,9 +18,6 @@ namespace DB } } -template -concept is_any_of = (std::same_as || ...); - /** Checks type by comparing typeid. * The exact match of the type is checked. That is, cast to the ancestor will be unsuccessful. diff --git a/src/Core/Settings.h b/src/Core/Settings.h index e9db155fb123..119c6602b1da 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -462,6 +462,7 @@ class IColumn; M(Bool, allow_introspection_functions, false, "Allow functions for introspection of ELF and DWARF for query profiling. These functions are slow and may impose security considerations.", 0) \ \ M(Bool, allow_execute_multiif_columnar, true, "Allow execute multiIf function columnar", 0) \ + M(Bool, formatdatetime_parsedatetime_m_is_month_name, true, "Formatter '%M' in function 'formatDateTime' produces the month name instead of minutes.", 0) \ \ M(UInt64, max_partitions_per_insert_block, 100, "Limit maximum number of partitions in single INSERTed block. Zero means unlimited. Throw exception if the block contains too many partitions. This setting is a safety threshold, because using large number of partitions is a common misconception.", 0) \ M(Int64, max_partitions_to_read, -1, "Limit the max number of partitions that can be accessed in one query. <= 0 means unlimited.", 0) \ diff --git a/src/Core/SettingsChangesHistory.h b/src/Core/SettingsChangesHistory.h index caf18cf8fb8f..4f89397ed9d7 100644 --- a/src/Core/SettingsChangesHistory.h +++ b/src/Core/SettingsChangesHistory.h @@ -101,6 +101,7 @@ static std::map sett {"query_plan_aggregation_in_order", 0, 1, "Enable some refactoring around query plan"}, {"format_binary_max_string_size", 0, 1_GiB, "Prevent allocating large amount of memory"}}}, {"22.11", {{"use_structure_from_insertion_table_in_table_functions", 0, 2, "Improve using structure from insertion table in table functions"}}}, + {"23.4", {{"formatdatetime_parsedatetime_m_is_month_name", false, true, "Improved compatibility with MySQL DATE_FORMAT/STR_TO_DATE"}}}, {"22.9", {{"force_grouping_standard_compatibility", false, true, "Make GROUPING function output the same as in SQL standard and other DBMS"}}}, {"22.7", {{"cross_to_inner_join_rewrite", 1, 2, "Force rewrite comma join to inner"}, {"enable_positional_arguments", false, true, "Enable positional arguments feature by default"}, diff --git a/src/Functions/FunctionsConversion.h b/src/Functions/FunctionsConversion.h index f832bf404a8b..28002d34acc4 100644 --- a/src/Functions/FunctionsConversion.h +++ b/src/Functions/FunctionsConversion.h @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include diff --git a/src/Functions/formatDateTime.cpp b/src/Functions/formatDateTime.cpp index bbb4c3ba5b01..1ed5948a3677 100644 --- a/src/Functions/formatDateTime.cpp +++ b/src/Functions/formatDateTime.cpp @@ -17,6 +17,7 @@ #include +#include #include #include #include @@ -38,22 +39,19 @@ namespace ErrorCodes namespace { +using Pos = const char *; -struct FormatDateTimeTraits +enum class SupportInteger { - enum class SupportInteger - { - Yes, - No - }; - - enum class FormatSyntax - { - MySQL, - Joda - }; + Yes, + No }; +enum class FormatSyntax +{ + MySQL, + Joda +}; template struct InstructionValueTypeMap {}; template <> struct InstructionValueTypeMap { using InstructionValueType = UInt32; }; @@ -85,11 +83,9 @@ constexpr std::string_view weekdaysFull[] = {"Sunday", "Monday", "Tuesday", "Wed constexpr std::string_view weekdaysShort[] = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"}; -constexpr std::string_view monthsFull[] - = {"January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"}; +constexpr std::string_view monthsFull[] = {"January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"}; -constexpr std::string_view monthsShort[] - = {"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"}; +constexpr std::string_view monthsShort[] = {"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"}; /** formatDateTime(time, 'format') * Performs formatting of time, according to provided format. @@ -115,13 +111,13 @@ constexpr std::string_view monthsShort[] * * Performance on Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz: * - * WITH formatDateTime(now() + number, '%H:%M:%S') AS x SELECT count() FROM system.numbers WHERE NOT ignore(x); + * WITH formatDateTime(now() + number, '%H:%i:%S') AS x SELECT count() FROM system.numbers WHERE NOT ignore(x); * - 97 million rows per second per core; * * WITH formatDateTime(toDateTime('2018-01-01 00:00:00') + number, '%F %T') AS x SELECT count() FROM system.numbers WHERE NOT ignore(x) * - 71 million rows per second per core; * - * select count() from (select formatDateTime(t, '%m/%d/%Y %H:%M:%S') from (select toDateTime('2018-01-01 00:00:00')+number as t from numbers(100000000))); + * select count() from (select formatDateTime(t, '%m/%d/%Y %H:%i:%S') from (select toDateTime('2018-01-01 00:00:00')+number as t from numbers(100000000))); * - 53 million rows per second per core; * * select count() from (select formatDateTime(t, 'Hello %Y World') from (select toDateTime('2018-01-01 00:00:00')+number as t from numbers(100000000))); @@ -129,7 +125,7 @@ constexpr std::string_view monthsShort[] * * PS. We can make this function to return FixedString. Currently it returns String. */ -template +template class FunctionFormatDateTimeImpl : public IFunction { private: @@ -152,26 +148,34 @@ class FunctionFormatDateTimeImpl : public IFunction class Instruction { public: - /// Using std::function will cause performance degradation in MySQL format by 0.45x. - /// But std::function is required for Joda format to capture extra variables. - /// This is the reason why we use raw function pointer in MySQL format and std::function - /// in Joda format. - using Func = std::conditional_t< - format_syntax == FormatDateTimeTraits::FormatSyntax::MySQL, - size_t (*)(char *, Time, UInt64, UInt32, const DateLUTImpl &), - std::function>; + /// Joda format generally requires capturing extra variables (i.e. holding state) which is more convenient with + /// std::function and std::bind. Unfortunately, std::function causes a performance degradation by 0.45x compared to raw function + /// pointers. For MySQL format, we generally prefer raw function pointers. Because of the special case that not all formatters are + /// fixed-width formatters (see mysqlLiteral instruction), we still need to be able to store state. For that reason, we use member + /// function pointers instead of static function pointers. + using FuncMysql = size_t (Instruction