From 632c5427626f27b4ab528653fa44f01003345aee Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Tue, 25 Dec 2018 03:19:17 +0300 Subject: [PATCH 01/57] add basic timer with logging for query threads --- dbms/src/Common/ThreadStatus.h | 7 +++ dbms/src/Interpreters/ThreadStatusExt.cpp | 56 +++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/dbms/src/Common/ThreadStatus.h b/dbms/src/Common/ThreadStatus.h index 822e1931447..05994883040 100644 --- a/dbms/src/Common/ThreadStatus.h +++ b/dbms/src/Common/ThreadStatus.h @@ -144,6 +144,10 @@ protected: void initPerformanceCounters(); + void initQueryProfiler(); + + void finalizeQueryProfiler(); + void logToQueryThreadLog(QueryThreadLog & thread_log); void assertState(const std::initializer_list & permitted_states, const char * description = nullptr); @@ -165,6 +169,9 @@ protected: time_t query_start_time = 0; size_t queries_started = 0; + bool has_query_profiler = false; + timer_t query_profiler_timer_id = 0; + Poco::Logger * log = nullptr; friend class CurrentThread; diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index eac9251cdf0..ffd12572cf1 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -5,7 +5,12 @@ #include #include #include +#include +#include +#include +#include +#include /// Implement some methods of ThreadStatus and CurrentThread here to avoid extra linking dependencies in clickhouse_common_io namespace DB @@ -92,6 +97,10 @@ void ThreadStatus::attachQuery(const ThreadGroupStatusPtr & thread_group_, bool } initPerformanceCounters(); + + LOG_INFO(&Logger::get("laplab"), "Attaching"); + initQueryProfiler(); + thread_state = ThreadState::AttachedToQuery; } @@ -119,6 +128,49 @@ void ThreadStatus::finalizePerformanceCounters() } } +namespace { + void queryProfilerTimerHandler(int /* signal */) { + LOG_INFO(&Logger::get("laplab"), "Hello from handler!"); + } +} + +void ThreadStatus::initQueryProfiler() { + sigevent sev; + sev.sigev_notify = SIGEV_THREAD_ID; + sev.sigev_signo = SIGALRM; + sev._sigev_un._tid = os_thread_id; + // TODO(laplab): get clock type from settings + if (timer_create(CLOCK_REALTIME, &sev, &query_profiler_timer_id)) { + LOG_ERROR(log, "Failed to create query profiler timer"); + return; + } + + // TODO(laplab): get period from settings + timespec period{.tv_sec = 10, .tv_nsec = 0}; + itimerspec timer_spec = {.it_interval = period, .it_value = period}; + if (timer_settime(query_profiler_timer_id, 0, &timer_spec, nullptr)) { + LOG_ERROR(log, "Failed to set query profiler timer"); + return; + } + + std::signal(SIGALRM, queryProfilerTimerHandler); + + has_query_profiler = true; +} + +void ThreadStatus::finalizeQueryProfiler() { + if (!has_query_profiler) { + return; + } + + if (timer_delete(query_profiler_timer_id)) { + LOG_ERROR(log, "Failed to delete query profiler timer"); + return; + } + + has_query_profiler = false; +} + void ThreadStatus::detachQuery(bool exit_if_already_detached, bool thread_exits) { if (exit_if_already_detached && thread_state == ThreadState::DetachedFromQuery) @@ -128,6 +180,8 @@ void ThreadStatus::detachQuery(bool exit_if_already_detached, bool thread_exits) } assertState({ThreadState::AttachedToQuery}, __PRETTY_FUNCTION__); + + finalizeQueryProfiler(); finalizePerformanceCounters(); /// Detach from thread group @@ -140,6 +194,8 @@ void ThreadStatus::detachQuery(bool exit_if_already_detached, bool thread_exits) query_context = nullptr; thread_group.reset(); + LOG_INFO(&Logger::get("laplab"), "Detaching"); + thread_state = thread_exits ? ThreadState::Died : ThreadState::DetachedFromQuery; } From 4c55802552557d53f060ef0f8cae0ad23f01883f Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Fri, 18 Jan 2019 13:10:43 +0000 Subject: [PATCH 02/57] refactor - add exceptions on timer errors - move from signal to sigaction - extract backtrace collection logic --- dbms/src/Interpreters/ThreadStatusExt.cpp | 53 ++-- libs/libdaemon/CMakeLists.txt | 3 +- libs/libdaemon/include/daemon/Backtrace.h | 78 ++++++ libs/libdaemon/src/Backtrace.cpp | 319 ++++++++++++++++++++++ libs/libdaemon/src/BaseDaemon.cpp | 215 +-------------- 5 files changed, 437 insertions(+), 231 deletions(-) create mode 100644 libs/libdaemon/include/daemon/Backtrace.h create mode 100644 libs/libdaemon/src/Backtrace.cpp diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index ffd12572cf1..3f7c77fd006 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -97,10 +97,8 @@ void ThreadStatus::attachQuery(const ThreadGroupStatusPtr & thread_group_, bool } initPerformanceCounters(); - - LOG_INFO(&Logger::get("laplab"), "Attaching"); initQueryProfiler(); - + thread_state = ThreadState::AttachedToQuery; } @@ -129,31 +127,49 @@ void ThreadStatus::finalizePerformanceCounters() } namespace { - void queryProfilerTimerHandler(int /* signal */) { + void queryProfilerTimerHandler(int /* sig */, siginfo_t * /* info */, void * /* context */) { LOG_INFO(&Logger::get("laplab"), "Hello from handler!"); } } void ThreadStatus::initQueryProfiler() { - sigevent sev; + if (!query_context) { + LOG_INFO(log, "Query profiler disabled - no context"); + return; + } + + struct sigevent sev; sev.sigev_notify = SIGEV_THREAD_ID; sev.sigev_signo = SIGALRM; sev._sigev_un._tid = os_thread_id; // TODO(laplab): get clock type from settings if (timer_create(CLOCK_REALTIME, &sev, &query_profiler_timer_id)) { - LOG_ERROR(log, "Failed to create query profiler timer"); - return; - } - - // TODO(laplab): get period from settings - timespec period{.tv_sec = 10, .tv_nsec = 0}; - itimerspec timer_spec = {.it_interval = period, .it_value = period}; - if (timer_settime(query_profiler_timer_id, 0, &timer_spec, nullptr)) { - LOG_ERROR(log, "Failed to set query profiler timer"); - return; + throw Poco::Exception("Failed to create query profiler timer"); } - std::signal(SIGALRM, queryProfilerTimerHandler); + // TODO(laplab): get period from settings + struct timespec period{.tv_sec = 0, .tv_nsec = 200000000}; + struct itimerspec timer_spec = {.it_interval = period, .it_value = period}; + if (timer_settime(query_profiler_timer_id, 0, &timer_spec, nullptr)) { + throw Poco::Exception("Failed to set query profiler timer"); + } + + struct sigaction sa; + memset(&sa, 0, sizeof(sa)); + sa.sa_sigaction = queryProfilerTimerHandler; + sa.sa_flags = SA_SIGINFO; + + if (sigemptyset(&sa.sa_mask)) { + throw Poco::Exception("Failed to clean signal mask for query profiler"); + } + + if (sigaddset(&sa.sa_mask, SIGALRM)) { + throw Poco::Exception("Failed to add signal to mask for query profiler"); + } + + if (sigaction(SIGALRM, &sa, nullptr)) { + throw Poco::Exception("Failed to setup signal handler for query profiler"); + } has_query_profiler = true; } @@ -164,8 +180,7 @@ void ThreadStatus::finalizeQueryProfiler() { } if (timer_delete(query_profiler_timer_id)) { - LOG_ERROR(log, "Failed to delete query profiler timer"); - return; + throw Poco::Exception("Failed to delete query profiler timer"); } has_query_profiler = false; @@ -194,8 +209,6 @@ void ThreadStatus::detachQuery(bool exit_if_already_detached, bool thread_exits) query_context = nullptr; thread_group.reset(); - LOG_INFO(&Logger::get("laplab"), "Detaching"); - thread_state = thread_exits ? ThreadState::Died : ThreadState::DetachedFromQuery; } diff --git a/libs/libdaemon/CMakeLists.txt b/libs/libdaemon/CMakeLists.txt index b352602f81c..2a9a651cdba 100644 --- a/libs/libdaemon/CMakeLists.txt +++ b/libs/libdaemon/CMakeLists.txt @@ -5,6 +5,7 @@ add_library (daemon ${LINK_MODE} src/OwnPatternFormatter.cpp src/OwnFormattingChannel.cpp src/OwnSplitChannel.cpp + src/Backtrace.cpp include/daemon/BaseDaemon.h include/daemon/GraphiteWriter.h @@ -12,7 +13,7 @@ add_library (daemon ${LINK_MODE} include/daemon/OwnPatternFormatter.h include/daemon/OwnFormattingChannel.h include/daemon/OwnSplitChannel.h -) + include/daemon/Backtrace.h) if (USE_UNWIND) target_compile_definitions (daemon PRIVATE USE_UNWIND=1) diff --git a/libs/libdaemon/include/daemon/Backtrace.h b/libs/libdaemon/include/daemon/Backtrace.h new file mode 100644 index 00000000000..6cadc463642 --- /dev/null +++ b/libs/libdaemon/include/daemon/Backtrace.h @@ -0,0 +1,78 @@ +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if USE_UNWIND +#define UNW_LOCAL_ONLY + #include +#endif + +#ifdef __APPLE__ +// ucontext is not available without _XOPEN_SOURCE +#define _XOPEN_SOURCE +#endif +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +std::string signalToErrorMessage(int sig, siginfo_t & info, ucontext_t & context); + +void * getCallerAddress(ucontext_t & context); + +std::vector getBacktraceFrames(ucontext_t & context); + +std::string backtraceFramesToString(const std::vector & frames); diff --git a/libs/libdaemon/src/Backtrace.cpp b/libs/libdaemon/src/Backtrace.cpp new file mode 100644 index 00000000000..d8ff20d999a --- /dev/null +++ b/libs/libdaemon/src/Backtrace.cpp @@ -0,0 +1,319 @@ +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if USE_UNWIND +#define UNW_LOCAL_ONLY + #include +#endif + +#ifdef __APPLE__ +// ucontext is not available without _XOPEN_SOURCE +#define _XOPEN_SOURCE +#endif +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +#if USE_UNWIND +std::vector backtraceLibUnwind(size_t max_frames, ucontext_t & context) +{ + std::vector out_frames; + unw_cursor_t cursor; + + if (unw_init_local2(&cursor, &context, UNW_INIT_SIGNAL_FRAME) >= 0) + { + for (size_t i = 0; i < max_frames; ++i) + { + unw_word_t ip; + unw_get_reg(&cursor, UNW_REG_IP, &ip); + out_frames.push_back(reinterpret_cast(ip)); + + /// NOTE This triggers "AddressSanitizer: stack-buffer-overflow". Looks like false positive. + /// It's Ok, because we use this method if the program is crashed nevertheless. + if (!unw_step(&cursor)) + break; + } + } + + return out_frames; +} +#endif + +std::string signalToErrorMessage(int sig, siginfo_t & info, ucontext_t & context) +{ + std::stringstream error; + switch (sig) + { + case SIGSEGV: + { + /// Print info about address and reason. + if (nullptr == info.si_addr) + error << "Address: NULL pointer."; + else + error << "Address: " << info.si_addr; + +#if defined(__x86_64__) && !defined(__FreeBSD__) && !defined(__APPLE__) + auto err_mask = context.uc_mcontext.gregs[REG_ERR]; + if ((err_mask & 0x02)) + error << " Access: write."; + else + error << " Access: read."; +#endif + + switch (info.si_code) + { + case SEGV_ACCERR: + error << " Attempted access has violated the permissions assigned to the memory area."; + break; + case SEGV_MAPERR: + error << " Address not mapped to object."; + break; + default: + error << " Unknown si_code."; + break; + } + break; + } + + case SIGBUS: + { + switch (info.si_code) + { + case BUS_ADRALN: + error << "Invalid address alignment."; + break; + case BUS_ADRERR: + error << "Non-existant physical address."; + break; + case BUS_OBJERR: + error << "Object specific hardware error."; + break; + + // Linux specific +#if defined(BUS_MCEERR_AR) + case BUS_MCEERR_AR: + error << "Hardware memory error: action required."; + break; +#endif +#if defined(BUS_MCEERR_AO) + case BUS_MCEERR_AO: + error << "Hardware memory error: action optional."; + break; +#endif + + default: + error << "Unknown si_code."; + break; + } + break; + } + + case SIGILL: + { + switch (info.si_code) + { + case ILL_ILLOPC: + error << "Illegal opcode."; + break; + case ILL_ILLOPN: + error << "Illegal operand."; + break; + case ILL_ILLADR: + error << "Illegal addressing mode."; + break; + case ILL_ILLTRP: + error << "Illegal trap."; + break; + case ILL_PRVOPC: + error << "Privileged opcode."; + break; + case ILL_PRVREG: + error << "Privileged register."; + break; + case ILL_COPROC: + error << "Coprocessor error."; + break; + case ILL_BADSTK: + error << "Internal stack error."; + break; + default: + error << "Unknown si_code."; + break; + } + break; + } + + case SIGFPE: + { + switch (info.si_code) + { + case FPE_INTDIV: + error << "Integer divide by zero."; + break; + case FPE_INTOVF: + error << "Integer overflow."; + break; + case FPE_FLTDIV: + error << "Floating point divide by zero."; + break; + case FPE_FLTOVF: + error << "Floating point overflow."; + break; + case FPE_FLTUND: + error << "Floating point underflow."; + break; + case FPE_FLTRES: + error << "Floating point inexact result."; + break; + case FPE_FLTINV: + error << "Floating point invalid operation."; + break; + case FPE_FLTSUB: + error << "Subscript out of range."; + break; + default: + error << "Unknown si_code."; + break; + } + break; + } + } + + return error.str(); +} + +void * getCallerAddress(ucontext_t & context) +{ +#if defined(__x86_64__) + /// Get the address at the time the signal was raised from the RIP (x86-64) +#if defined(__FreeBSD__) + return reinterpret_cast(context.uc_mcontext.mc_rip); +#elif defined(__APPLE__) + return reinterpret_cast(context.uc_mcontext->__ss.__rip); +#else + return reinterpret_cast(context.uc_mcontext.gregs[REG_RIP]); +#endif +#elif defined(__aarch64__) + return reinterpret_cast(context.uc_mcontext.pc); +#endif + + return nullptr; +} + +std::vector getBacktraceFrames(ucontext_t & context) +{ + std::vector frames; + +#if USE_UNWIND + static size_t max_frames = 50; + frames = backtraceLibUnwind(max_frames, context); +#else + /// No libunwind means no backtrace, because we are in a different thread from the one where the signal happened. + /// So at least print the function where the signal happened. + void * caller_address = getCallerAddress(context); + if (caller_address) + frames.push_back(caller_address); +#endif + + return frames; +} + +std::string backtraceFramesToString(const std::vector & frames) +{ + std::stringstream backtrace; + char ** symbols = backtrace_symbols(frames.data(), frames.size()); + + if (!symbols) + { + if (frames.size() > 0) + backtrace << "No symbols could be found for backtrace starting at " << frames[0]; + } + else + { + for (size_t i = 0; i < frames.size(); ++i) + { + /// Perform demangling of names. Name is in parentheses, before '+' character. + + char * name_start = nullptr; + char * name_end = nullptr; + char * demangled_name = nullptr; + int status = 0; + + if (nullptr != (name_start = strchr(symbols[i], '(')) + && nullptr != (name_end = strchr(name_start, '+'))) + { + ++name_start; + *name_end = '\0'; + demangled_name = abi::__cxa_demangle(name_start, 0, 0, &status); + *name_end = '+'; + } + + backtrace << i << ". "; + + if (nullptr != demangled_name && 0 == status) + { + backtrace.write(symbols[i], name_start - symbols[i]); + backtrace << demangled_name << name_end; + } + else + backtrace << symbols[i]; + } + } + + return backtrace.str(); +} diff --git a/libs/libdaemon/src/BaseDaemon.cpp b/libs/libdaemon/src/BaseDaemon.cpp index bad38c78529..09ecfdf89e9 100644 --- a/libs/libdaemon/src/BaseDaemon.cpp +++ b/libs/libdaemon/src/BaseDaemon.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -297,218 +298,12 @@ private: LOG_ERROR(log, "(from thread " << thread_num << ") " << "Received signal " << strsignal(sig) << " (" << sig << ")" << "."); - void * caller_address = nullptr; + LOG_ERROR(log, signalToErrorMessage(sig, info, context)); -#if defined(__x86_64__) - /// Get the address at the time the signal was raised from the RIP (x86-64) - #if defined(__FreeBSD__) - caller_address = reinterpret_cast(context.uc_mcontext.mc_rip); - #elif defined(__APPLE__) - caller_address = reinterpret_cast(context.uc_mcontext->__ss.__rip); - #else - caller_address = reinterpret_cast(context.uc_mcontext.gregs[REG_RIP]); - auto err_mask = context.uc_mcontext.gregs[REG_ERR]; - #endif -#elif defined(__aarch64__) - caller_address = reinterpret_cast(context.uc_mcontext.pc); -#endif + std::vector frames = getBacktraceFrames(context); + std::string backtrace = backtraceFramesToString(frames); - switch (sig) - { - case SIGSEGV: - { - /// Print info about address and reason. - if (nullptr == info.si_addr) - LOG_ERROR(log, "Address: NULL pointer."); - else - LOG_ERROR(log, "Address: " << info.si_addr); - -#if defined(__x86_64__) && !defined(__FreeBSD__) && !defined(__APPLE__) - if ((err_mask & 0x02)) - LOG_ERROR(log, "Access: write."); - else - LOG_ERROR(log, "Access: read."); -#endif - - switch (info.si_code) - { - case SEGV_ACCERR: - LOG_ERROR(log, "Attempted access has violated the permissions assigned to the memory area."); - break; - case SEGV_MAPERR: - LOG_ERROR(log, "Address not mapped to object."); - break; - default: - LOG_ERROR(log, "Unknown si_code."); - break; - } - break; - } - - case SIGBUS: - { - switch (info.si_code) - { - case BUS_ADRALN: - LOG_ERROR(log, "Invalid address alignment."); - break; - case BUS_ADRERR: - LOG_ERROR(log, "Non-existant physical address."); - break; - case BUS_OBJERR: - LOG_ERROR(log, "Object specific hardware error."); - break; - - // Linux specific -#if defined(BUS_MCEERR_AR) - case BUS_MCEERR_AR: - LOG_ERROR(log, "Hardware memory error: action required."); - break; -#endif -#if defined(BUS_MCEERR_AO) - case BUS_MCEERR_AO: - LOG_ERROR(log, "Hardware memory error: action optional."); - break; -#endif - - default: - LOG_ERROR(log, "Unknown si_code."); - break; - } - break; - } - - case SIGILL: - { - switch (info.si_code) - { - case ILL_ILLOPC: - LOG_ERROR(log, "Illegal opcode."); - break; - case ILL_ILLOPN: - LOG_ERROR(log, "Illegal operand."); - break; - case ILL_ILLADR: - LOG_ERROR(log, "Illegal addressing mode."); - break; - case ILL_ILLTRP: - LOG_ERROR(log, "Illegal trap."); - break; - case ILL_PRVOPC: - LOG_ERROR(log, "Privileged opcode."); - break; - case ILL_PRVREG: - LOG_ERROR(log, "Privileged register."); - break; - case ILL_COPROC: - LOG_ERROR(log, "Coprocessor error."); - break; - case ILL_BADSTK: - LOG_ERROR(log, "Internal stack error."); - break; - default: - LOG_ERROR(log, "Unknown si_code."); - break; - } - break; - } - - case SIGFPE: - { - switch (info.si_code) - { - case FPE_INTDIV: - LOG_ERROR(log, "Integer divide by zero."); - break; - case FPE_INTOVF: - LOG_ERROR(log, "Integer overflow."); - break; - case FPE_FLTDIV: - LOG_ERROR(log, "Floating point divide by zero."); - break; - case FPE_FLTOVF: - LOG_ERROR(log, "Floating point overflow."); - break; - case FPE_FLTUND: - LOG_ERROR(log, "Floating point underflow."); - break; - case FPE_FLTRES: - LOG_ERROR(log, "Floating point inexact result."); - break; - case FPE_FLTINV: - LOG_ERROR(log, "Floating point invalid operation."); - break; - case FPE_FLTSUB: - LOG_ERROR(log, "Subscript out of range."); - break; - default: - LOG_ERROR(log, "Unknown si_code."); - break; - } - break; - } - } - - static const int max_frames = 50; - void * frames[max_frames]; - -#if USE_UNWIND - int frames_size = backtraceLibUnwind(frames, max_frames, context); - - if (frames_size) - { -#else - /// No libunwind means no backtrace, because we are in a different thread from the one where the signal happened. - /// So at least print the function where the signal happened. - if (caller_address) - { - frames[0] = caller_address; - int frames_size = 1; -#endif - - char ** symbols = backtrace_symbols(frames, frames_size); - - if (!symbols) - { - if (caller_address) - LOG_ERROR(log, "Caller address: " << caller_address); - } - else - { - for (int i = 0; i < frames_size; ++i) - { - /// Perform demangling of names. Name is in parentheses, before '+' character. - - char * name_start = nullptr; - char * name_end = nullptr; - char * demangled_name = nullptr; - int status = 0; - - if (nullptr != (name_start = strchr(symbols[i], '(')) - && nullptr != (name_end = strchr(name_start, '+'))) - { - ++name_start; - *name_end = '\0'; - demangled_name = abi::__cxa_demangle(name_start, 0, 0, &status); - *name_end = '+'; - } - - std::stringstream res; - - res << i << ". "; - - if (nullptr != demangled_name && 0 == status) - { - res.write(symbols[i], name_start - symbols[i]); - res << demangled_name << name_end; - } - else - res << symbols[i]; - - LOG_ERROR(log, res.rdbuf()); - } - } - } + LOG_ERROR(log, backtrace); } }; From df3db1bff219351a84cb17385afdd8605ccc6acb Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Sun, 3 Feb 2019 09:57:12 +0000 Subject: [PATCH 03/57] basic pipe --- dbms/CMakeLists.txt | 2 +- dbms/src/Common/Pipe.h | 65 +++++++++++++++++++ dbms/src/Common/ShellCommand.cpp | 76 +++++------------------ dbms/src/Interpreters/Context.cpp | 11 +++- dbms/src/Interpreters/Context.h | 2 + dbms/src/Interpreters/ThreadStatusExt.cpp | 14 ++++- dbms/src/Interpreters/TraceCollector.cpp | 34 ++++++++++ dbms/src/Interpreters/TraceCollector.h | 25 ++++++++ libs/libdaemon/CMakeLists.txt | 3 +- libs/libdaemon/include/daemon/Backtrace.h | 2 + libs/libdaemon/include/daemon/Pipe.h | 51 +++++++++++++++ libs/libdaemon/src/BaseDaemon.cpp | 50 +-------------- 12 files changed, 221 insertions(+), 114 deletions(-) create mode 100644 dbms/src/Common/Pipe.h create mode 100644 dbms/src/Interpreters/TraceCollector.cpp create mode 100644 dbms/src/Interpreters/TraceCollector.h create mode 100644 libs/libdaemon/include/daemon/Pipe.h diff --git a/dbms/CMakeLists.txt b/dbms/CMakeLists.txt index b7f11731662..90e49aefc02 100644 --- a/dbms/CMakeLists.txt +++ b/dbms/CMakeLists.txt @@ -97,7 +97,7 @@ list (APPEND dbms_headers list (APPEND dbms_sources src/TableFunctions/ITableFunction.cpp src/TableFunctions/TableFunctionFactory.cpp) list (APPEND dbms_headers src/TableFunctions/ITableFunction.h src/TableFunctions/TableFunctionFactory.h) -add_library(clickhouse_common_io ${LINK_MODE} ${clickhouse_common_io_headers} ${clickhouse_common_io_sources}) +add_library(clickhouse_common_io ${LINK_MODE} ${clickhouse_common_io_headers} ${clickhouse_common_io_sources} src/Common/Pipe.h) if (OS_FREEBSD) target_compile_definitions (clickhouse_common_io PUBLIC CLOCK_MONOTONIC_COARSE=CLOCK_MONOTONIC_FAST) diff --git a/dbms/src/Common/Pipe.h b/dbms/src/Common/Pipe.h new file mode 100644 index 00000000000..2aa5f2ef4a6 --- /dev/null +++ b/dbms/src/Common/Pipe.h @@ -0,0 +1,65 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace DB { + + struct Pipe + { + union + { + int fds[2]; + struct + { + int read_fd; + int write_fd; + }; + }; + + Pipe() + { + read_fd = -1; + write_fd = -1; + + if (0 != pipe(fds)) + DB::throwFromErrno("Cannot create pipe", 0); + } + + void close() + { + if (-1 != read_fd) + { + ::close(read_fd); + read_fd = -1; + } + + if (-1 != write_fd) + { + ::close(write_fd); + write_fd = -1; + } + + LOG_INFO(&Logger::get("TraceCollector"), "Pipe is closed"); + } + + ~Pipe() + { + close(); + } + }; + + class PipeSingleton : public ext::singleton + { + }; + +} \ No newline at end of file diff --git a/dbms/src/Common/ShellCommand.cpp b/dbms/src/Common/ShellCommand.cpp index 2e19d442f9a..5812cfa9ab8 100644 --- a/dbms/src/Common/ShellCommand.cpp +++ b/dbms/src/Common/ShellCommand.cpp @@ -8,73 +8,29 @@ #include #include #include +#include namespace DB { - namespace ErrorCodes - { - extern const int CANNOT_PIPE; - extern const int CANNOT_DLSYM; - extern const int CANNOT_FORK; - extern const int CANNOT_WAITPID; - extern const int CHILD_WAS_NOT_EXITED_NORMALLY; - extern const int CANNOT_CREATE_CHILD_PROCESS; - } + +namespace ErrorCodes +{ + extern const int CANNOT_DLSYM; + extern const int CANNOT_FORK; + extern const int CANNOT_WAITPID; + extern const int CHILD_WAS_NOT_EXITED_NORMALLY; + extern const int CANNOT_CREATE_CHILD_PROCESS; } - -namespace -{ - struct Pipe - { - union - { - int fds[2]; - struct - { - int read_fd; - int write_fd; - }; - }; - - Pipe() - { - #ifndef __APPLE__ - if (0 != pipe2(fds, O_CLOEXEC)) - DB::throwFromErrno("Cannot create pipe", DB::ErrorCodes::CANNOT_PIPE); - #else - if (0 != pipe(fds)) - DB::throwFromErrno("Cannot create pipe", DB::ErrorCodes::CANNOT_PIPE); - if (0 != fcntl(fds[0], F_SETFD, FD_CLOEXEC)) - DB::throwFromErrno("Cannot create pipe", DB::ErrorCodes::CANNOT_PIPE); - if (0 != fcntl(fds[1], F_SETFD, FD_CLOEXEC)) - DB::throwFromErrno("Cannot create pipe", DB::ErrorCodes::CANNOT_PIPE); - #endif - } - - ~Pipe() - { - if (read_fd >= 0) - close(read_fd); - if (write_fd >= 0) - close(write_fd); - } - }; - - /// By these return codes from the child process, we learn (for sure) about errors when creating it. - enum class ReturnCodes : int - { - CANNOT_DUP_STDIN = 0x55555555, /// The value is not important, but it is chosen so that it's rare to conflict with the program return code. - CANNOT_DUP_STDOUT = 0x55555556, - CANNOT_DUP_STDERR = 0x55555557, - CANNOT_EXEC = 0x55555558, - }; -} - - -namespace DB +/// By these return codes from the child process, we learn (for sure) about errors when creating it. +enum class ReturnCodes : int { + CANNOT_DUP_STDIN = 0x55555555, /// The value is not important, but it is chosen so that it's rare to conflict with the program return code. + CANNOT_DUP_STDOUT = 0x55555556, + CANNOT_DUP_STDERR = 0x55555557, + CANNOT_EXEC = 0x55555558, +}; ShellCommand::ShellCommand(pid_t pid, int in_fd, int out_fd, int err_fd, bool terminate_in_destructor_) : pid(pid) diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 1e33c90be2c..1f948d78cfb 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -52,6 +52,7 @@ #include #include #include +#include "TraceCollector.h" namespace ProfileEvents @@ -150,6 +151,9 @@ struct ContextShared String format_schema_path; /// Path to a directory that contains schema files used by input formats. ActionLocksManagerPtr action_locks_manager; /// Set of storages' action lockers + Poco::Thread trace_collector_thread; /// Thread collecting traces from threads executing queries + std::unique_ptr trace_collector; + /// Named sessions. The user could specify session identifier to reuse settings and temporary tables in subsequent requests. class SessionKeyHash @@ -267,7 +271,11 @@ struct ContextShared private: void initialize() { - security_manager = runtime_components_factory->createSecurityManager(); + security_manager = runtime_components_factory->createSecurityManager(); + + /// Set up trace collector for query profiler + trace_collector.reset(new TraceCollector()); + trace_collector_thread.start(*trace_collector); } }; @@ -484,7 +492,6 @@ DatabasePtr Context::tryGetDatabase(const String & database_name) return it->second; } - String Context::getPath() const { auto lock = getLock(); diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index 6e38e056a0f..d76374dd641 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -18,6 +18,8 @@ #include #include #include +#include +#include namespace Poco diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index 3f7c77fd006..a6a83b96775 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -5,12 +5,16 @@ #include #include #include +#include +#include +#include #include #include #include #include #include +#include /// Implement some methods of ThreadStatus and CurrentThread here to avoid extra linking dependencies in clickhouse_common_io namespace DB @@ -127,8 +131,16 @@ void ThreadStatus::finalizePerformanceCounters() } namespace { - void queryProfilerTimerHandler(int /* sig */, siginfo_t * /* info */, void * /* context */) { + void queryProfilerTimerHandler(int sig, siginfo_t * /* info */, void * /* context */) { LOG_INFO(&Logger::get("laplab"), "Hello from handler!"); + + char buffer[TraceCollector::buf_size]; + DB::WriteBufferFromFileDescriptor out(PipeSingleton::instance().write_fd, TraceCollector::buf_size, buffer); + + DB::writeBinary(sig, out); + out.next(); + + ::sleep(10); } } diff --git a/dbms/src/Interpreters/TraceCollector.cpp b/dbms/src/Interpreters/TraceCollector.cpp new file mode 100644 index 00000000000..1fde0870744 --- /dev/null +++ b/dbms/src/Interpreters/TraceCollector.cpp @@ -0,0 +1,34 @@ +#include "TraceCollector.h" +#include +#include + +namespace DB { + + const size_t TraceCollector::buf_size = sizeof(int); + + TraceCollector::TraceCollector() + : log(&Logger::get("TraceCollector")) + { + } + + void TraceCollector::run() + { + LOG_INFO(log, "TraceCollector started"); + + char buf[buf_size]; + DB::ReadBufferFromFileDescriptor in(PipeSingleton::instance().read_fd, buf_size, buf); + + LOG_INFO(log, "Preparing to read from: " << PipeSingleton::instance().read_fd); + + while (true) + { + int sig = 0; + DB::readBinary(sig, in); + + LOG_INFO(log, "Received signal: " << sig); + } + + LOG_INFO(log, "TraceCollector exited"); + } + +} diff --git a/dbms/src/Interpreters/TraceCollector.h b/dbms/src/Interpreters/TraceCollector.h new file mode 100644 index 00000000000..745616e122e --- /dev/null +++ b/dbms/src/Interpreters/TraceCollector.h @@ -0,0 +1,25 @@ +#pragma once + +#include +#include +#include +#include + +namespace DB +{ + + using Poco::Logger; + + class TraceCollector : public Poco::Runnable + { + private: + Logger * log; + + public: + explicit TraceCollector(); + + void run() override; + + static const size_t buf_size; + }; +} diff --git a/libs/libdaemon/CMakeLists.txt b/libs/libdaemon/CMakeLists.txt index 2a9a651cdba..2fa8b3c488b 100644 --- a/libs/libdaemon/CMakeLists.txt +++ b/libs/libdaemon/CMakeLists.txt @@ -13,7 +13,8 @@ add_library (daemon ${LINK_MODE} include/daemon/OwnPatternFormatter.h include/daemon/OwnFormattingChannel.h include/daemon/OwnSplitChannel.h - include/daemon/Backtrace.h) + include/daemon/Backtrace.h + include/daemon/Pipe.h) if (USE_UNWIND) target_compile_definitions (daemon PRIVATE USE_UNWIND=1) diff --git a/libs/libdaemon/include/daemon/Backtrace.h b/libs/libdaemon/include/daemon/Backtrace.h index 6cadc463642..0cb5973c12e 100644 --- a/libs/libdaemon/include/daemon/Backtrace.h +++ b/libs/libdaemon/include/daemon/Backtrace.h @@ -1,3 +1,5 @@ +#pragma once + #include #include #include diff --git a/libs/libdaemon/include/daemon/Pipe.h b/libs/libdaemon/include/daemon/Pipe.h new file mode 100644 index 00000000000..9a5b1d3afa5 --- /dev/null +++ b/libs/libdaemon/include/daemon/Pipe.h @@ -0,0 +1,51 @@ +#pragma once + +#include + +/** For transferring information from signal handler to a separate thread. + * If you need to do something serious in case of a signal (example: write a message to the log), + * then sending information to a separate thread through pipe and doing all the stuff asynchronously + * - is probably the only safe method for doing it. + * (Because it's only safe to use reentrant functions in signal handlers.) + */ +struct Pipe +{ + union + { + int fds[2]; + struct + { + int read_fd; + int write_fd; + }; + }; + + Pipe() + { + read_fd = -1; + write_fd = -1; + + if (0 != pipe(fds)) + DB::throwFromErrno("Cannot create pipe", 0); + } + + void close() + { + if (-1 != read_fd) + { + ::close(read_fd); + read_fd = -1; + } + + if (-1 != write_fd) + { + ::close(write_fd); + write_fd = -1; + } + } + + ~Pipe() + { + close(); + } +}; \ No newline at end of file diff --git a/libs/libdaemon/src/BaseDaemon.cpp b/libs/libdaemon/src/BaseDaemon.cpp index 09ecfdf89e9..20f6074968d 100644 --- a/libs/libdaemon/src/BaseDaemon.cpp +++ b/libs/libdaemon/src/BaseDaemon.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -70,55 +71,6 @@ #include -/** For transferring information from signal handler to a separate thread. - * If you need to do something serious in case of a signal (example: write a message to the log), - * then sending information to a separate thread through pipe and doing all the stuff asynchronously - * - is probably the only safe method for doing it. - * (Because it's only safe to use reentrant functions in signal handlers.) - */ -struct Pipe -{ - union - { - int fds[2]; - struct - { - int read_fd; - int write_fd; - }; - }; - - Pipe() - { - read_fd = -1; - write_fd = -1; - - if (0 != pipe(fds)) - DB::throwFromErrno("Cannot create pipe", 0); - } - - void close() - { - if (-1 != read_fd) - { - ::close(read_fd); - read_fd = -1; - } - - if (-1 != write_fd) - { - ::close(write_fd); - write_fd = -1; - } - } - - ~Pipe() - { - close(); - } -}; - - Pipe signal_pipe; From 5c54bbb7506803899f45d0e73f8f7a2a9e5b0c4c Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Sun, 3 Feb 2019 21:30:45 +0000 Subject: [PATCH 04/57] write traces to trace_log --- CMakeLists.txt | 2 +- dbms/CMakeLists.txt | 2 +- dbms/programs/server/Server.cpp | 3 + dbms/programs/server/config.xml | 8 ++ dbms/src/Common/Pipe.h | 24 +++--- dbms/src/Common/ShellCommand.cpp | 61 +++----------- dbms/src/Interpreters/Context.cpp | 25 +++++- dbms/src/Interpreters/Context.h | 9 ++- .../Interpreters/InterpreterSystemQuery.cpp | 4 +- dbms/src/Interpreters/SystemLog.cpp | 2 + dbms/src/Interpreters/SystemLog.h | 2 + dbms/src/Interpreters/ThreadStatusExt.cpp | 20 ++--- dbms/src/Interpreters/TraceCollector.cpp | 39 ++++++--- dbms/src/Interpreters/TraceCollector.h | 9 ++- dbms/src/Interpreters/TraceLog.cpp | 42 ++++++++++ dbms/src/Interpreters/TraceLog.h | 25 ++++++ libs/libcommon/CMakeLists.txt | 8 ++ .../cmake/find_unwind.cmake | 0 libs/libcommon/include/common/Backtrace.h | 25 ++++++ .../src/Backtrace.cpp | 74 ++--------------- libs/libdaemon/CMakeLists.txt | 11 +-- libs/libdaemon/include/daemon/Backtrace.h | 80 ------------------- libs/libdaemon/src/BaseDaemon.cpp | 34 +------- 23 files changed, 228 insertions(+), 281 deletions(-) create mode 100644 dbms/src/Interpreters/TraceLog.cpp create mode 100644 dbms/src/Interpreters/TraceLog.h rename libs/{libdaemon => libcommon}/cmake/find_unwind.cmake (100%) create mode 100644 libs/libcommon/include/common/Backtrace.h rename libs/{libdaemon => libcommon}/src/Backtrace.cpp (80%) delete mode 100644 libs/libdaemon/include/daemon/Backtrace.h diff --git a/CMakeLists.txt b/CMakeLists.txt index b0aed779dcf..f2d9e30a954 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -273,7 +273,7 @@ include (libs/libcommon/cmake/find_gperftools.cmake) include (libs/libcommon/cmake/find_jemalloc.cmake) include (libs/libcommon/cmake/find_cctz.cmake) include (libs/libmysqlxx/cmake/find_mysqlclient.cmake) -include (libs/libdaemon/cmake/find_unwind.cmake) +include (libs/libcommon/cmake/find_unwind.cmake) include (cmake/print_flags.cmake) diff --git a/dbms/CMakeLists.txt b/dbms/CMakeLists.txt index 8730d1cb909..90e3679eb2c 100644 --- a/dbms/CMakeLists.txt +++ b/dbms/CMakeLists.txt @@ -131,7 +131,7 @@ list (APPEND dbms_headers list (APPEND dbms_sources src/TableFunctions/ITableFunction.cpp src/TableFunctions/TableFunctionFactory.cpp) list (APPEND dbms_headers src/TableFunctions/ITableFunction.h src/TableFunctions/TableFunctionFactory.h) -add_library(clickhouse_common_io ${LINK_MODE} ${clickhouse_common_io_headers} ${clickhouse_common_io_sources} src/Common/Pipe.h) +add_library(clickhouse_common_io ${LINK_MODE} ${clickhouse_common_io_headers} ${clickhouse_common_io_sources}) if (OS_FREEBSD) target_compile_definitions (clickhouse_common_io PUBLIC CLOCK_MONOTONIC_COARSE=CLOCK_MONOTONIC_FAST) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index c9147ead965..da139ac8a61 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -467,6 +467,9 @@ int Server::main(const std::vector & /*args*/) } LOG_DEBUG(log, "Loaded metadata."); + /// Init trace collector only after trace_log system table was created + global_context->initializeTraceCollector(); + global_context->setCurrentDatabase(default_database); SCOPE_EXIT({ diff --git a/dbms/programs/server/config.xml b/dbms/programs/server/config.xml index 108e64e3387..a30f0a3c11e 100644 --- a/dbms/programs/server/config.xml +++ b/dbms/programs/server/config.xml @@ -291,6 +291,14 @@ 7500 + + system + trace_log
+ + toYYYYMM(event_date) + 7500 + + system trace_log
diff --git a/dbms/src/Common/QueryProfiler.cpp b/dbms/src/Common/QueryProfiler.cpp index 514a4f359f9..ce0ddef094f 100644 --- a/dbms/src/Common/QueryProfiler.cpp +++ b/dbms/src/Common/QueryProfiler.cpp @@ -5,11 +5,4 @@ namespace DB LazyPipe trace_pipe; -void CloseQueryTraceStream() -{ - DB::WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1]); - DB::writeIntBinary(true, out); - out.next(); -} - } diff --git a/dbms/src/Common/QueryProfiler.h b/dbms/src/Common/QueryProfiler.h index a4a55cc36a6..236a4907a15 100644 --- a/dbms/src/Common/QueryProfiler.h +++ b/dbms/src/Common/QueryProfiler.h @@ -27,28 +27,31 @@ enum class TimerType : UInt8 Cpu, }; -void CloseQueryTraceStream(); - namespace { + /// Normally query_id is a UUID (string with a fixed length) but user can provide custom query_id. + /// Thus upper bound on query_id length should be introduced to avoid buffer overflow in signal handler. + constexpr UInt32 QUERY_ID_MAX_LEN = 1024; + void writeTraceInfo(TimerType timer_type, int /* sig */, siginfo_t * /* info */, void * context) { - char buffer[DBMS_DEFAULT_BUFFER_SIZE]; - DB::WriteBufferFromFileDescriptor out( - /* fd */ trace_pipe.fds_rw[1], - /* buf_size */ DBMS_DEFAULT_BUFFER_SIZE, - /* existing_memory */ buffer - ); + constexpr UInt32 buf_size = sizeof(char) + // TraceCollector stop flag + 8 * sizeof(char) + // maximum VarUInt length for string size + QUERY_ID_MAX_LEN * sizeof(char) + // maximum query_id length + sizeof(StackTrace) + // collected stack trace + sizeof(TimerType); // timer type + char buffer[buf_size]; + DB::WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1], buf_size, buffer); const std::string & query_id = CurrentThread::getQueryId(); const auto signal_context = *reinterpret_cast(context); const StackTrace stack_trace(signal_context); - DB::writeIntBinary(false, out); + DB::writeChar(false, out); DB::writeStringBinary(query_id, out); DB::writePODBinary(stack_trace, out); - DB::writeIntBinary(timer_type, out); + DB::writePODBinary(timer_type, out); out.next(); } @@ -56,6 +59,17 @@ namespace } +/** + * Query profiler implementation for selected thread. + * + * This class installs timer and signal handler on creation to: + * 1. periodically pause given thread + * 2. collect thread's current stack trace + * 3. write collected stack trace to trace_pipe for TraceCollector + * + * Desctructor tries to unset timer and restore previous signal handler. + * Note that signal handler implementation is defined by template parameter. See QueryProfilerReal and QueryProfilerCpu. + */ template class QueryProfilerBase { @@ -112,6 +126,7 @@ private: struct sigaction * previous_handler = nullptr; }; +/// Query profiler with timer based on real clock class QueryProfilerReal : public QueryProfilerBase { public: @@ -125,6 +140,7 @@ public: } }; +/// Query profiler with timer based on CPU clock class QueryProfilerCpu : public QueryProfilerBase { public: diff --git a/dbms/src/Common/ShellCommand.cpp b/dbms/src/Common/ShellCommand.cpp index cb7f8c21bf6..66dbab35a20 100644 --- a/dbms/src/Common/ShellCommand.cpp +++ b/dbms/src/Common/ShellCommand.cpp @@ -10,6 +10,17 @@ #include #include +namespace +{ + /// By these return codes from the child process, we learn (for sure) about errors when creating it. + enum class ReturnCodes : int + { + CANNOT_DUP_STDIN = 0x55555555, /// The value is not important, but it is chosen so that it's rare to conflict with the program return code. + CANNOT_DUP_STDOUT = 0x55555556, + CANNOT_DUP_STDERR = 0x55555557, + CANNOT_EXEC = 0x55555558, + }; +} namespace DB { @@ -23,15 +34,6 @@ namespace ErrorCodes extern const int CANNOT_CREATE_CHILD_PROCESS; } -/// By these return codes from the child process, we learn (for sure) about errors when creating it. -enum class ReturnCodes : int -{ - CANNOT_DUP_STDIN = 0x55555555, /// The value is not important, but it is chosen so that it's rare to conflict with the program return code. - CANNOT_DUP_STDOUT = 0x55555556, - CANNOT_DUP_STDERR = 0x55555557, - CANNOT_EXEC = 0x55555558, -}; - ShellCommand::ShellCommand(pid_t pid, int in_fd, int out_fd, int err_fd, bool terminate_in_destructor_) : pid(pid) , terminate_in_destructor(terminate_in_destructor_) diff --git a/dbms/src/Common/ThreadStatus.h b/dbms/src/Common/ThreadStatus.h index 39564dcacfe..19935684796 100644 --- a/dbms/src/Common/ThreadStatus.h +++ b/dbms/src/Common/ThreadStatus.h @@ -11,7 +11,6 @@ #include #include #include -#include namespace Poco @@ -176,7 +175,6 @@ protected: size_t queries_started = 0; // CPU and Real time query profilers - bool has_query_profiler = false; std::unique_ptr query_profiler_real; std::unique_ptr query_profiler_cpu; diff --git a/dbms/src/Core/Settings.h b/dbms/src/Core/Settings.h index 01de5a7428f..dd01bfb1124 100644 --- a/dbms/src/Core/Settings.h +++ b/dbms/src/Core/Settings.h @@ -218,8 +218,8 @@ struct Settings : public SettingsCollection M(SettingBool, empty_result_for_aggregation_by_empty_set, false, "Return empty result when aggregating without keys on empty set.") \ M(SettingBool, allow_distributed_ddl, true, "If it is set to true, then a user is allowed to executed distributed DDL queries.") \ M(SettingUInt64, odbc_max_field_size, 1024, "Max size of filed can be read from ODBC dictionary. Long strings are truncated.") \ - M(SettingUInt64, query_profiler_real_time_period, 500000000, "Period for real clock timer of query profiler (in nanoseconds).") \ - M(SettingUInt64, query_profiler_cpu_time_period, 500000000, "Period for CPU clock timer of query profiler (in nanoseconds).") \ + M(SettingUInt64, query_profiler_real_time_period, 500000000, "Period for real clock timer of query profiler (in nanoseconds). Set 0 value to turn off real clock query profiler") \ + M(SettingUInt64, query_profiler_cpu_time_period, 500000000, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off CPU clock query profiler") \ \ \ /** Limits during query execution are part of the settings. \ diff --git a/dbms/src/Functions/FunctionsIntrospection.h b/dbms/src/Functions/FunctionsIntrospection.h index 0252f2b4c8f..d7ca1d37efa 100644 --- a/dbms/src/Functions/FunctionsIntrospection.h +++ b/dbms/src/Functions/FunctionsIntrospection.h @@ -84,17 +84,20 @@ public: auto result_column = ColumnString::create(); - size_t pos = 0; + StackTrace::Frames frames; + size_t current_offset = 0; for (size_t i = 0; i < offsets.size(); ++i) { - std::vector frames; - for (; pos < offsets[i]; ++pos) + size_t current_size = 0; + for (; current_size < frames.size() && current_offset + current_size < offsets[i]; ++current_size) { - frames.push_back(reinterpret_cast(data[pos])); + frames[current_size] = reinterpret_cast(data[current_offset + current_size]); } - std::string backtrace = StackTrace(frames).toString(); + std::string backtrace = StackTrace(frames.begin(), frames.begin() + current_size).toString(); result_column->insertDataWithTerminatingZero(backtrace.c_str(), backtrace.length() + 1); + + current_offset = offsets[i]; } block.getByPosition(result).column = std::move(result_column); diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 65e74dfd6de..cb7dfd9b053 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -295,7 +295,7 @@ struct ContextShared if (trace_collector != nullptr) { /// Stop trace collector - CloseQueryTraceStream(); + NotifyTraceCollectorToStop(); trace_collector_thread.join(); /// Close trace pipe - definitely nobody needs to write there after diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index 5e88458caff..f2d053d776d 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -11,12 +11,6 @@ #include #include -#include -#include -#include -#include -#include - /// Implement some methods of ThreadStatus and CurrentThread here to avoid extra linking dependencies in clickhouse_common_io /// TODO It doesn't make sense. @@ -156,19 +150,12 @@ void ThreadStatus::initQueryProfiler() /* thread_id */ os_thread_id, /* period */ static_cast(settings.query_profiler_cpu_time_period) ); - - has_query_profiler = true; } void ThreadStatus::finalizeQueryProfiler() { - if (!has_query_profiler) - return; - query_profiler_real.reset(nullptr); query_profiler_cpu.reset(nullptr); - - has_query_profiler = false; } void ThreadStatus::detachQuery(bool exit_if_already_detached, bool thread_exits) diff --git a/dbms/src/Interpreters/TraceCollector.cpp b/dbms/src/Interpreters/TraceCollector.cpp index c2054d6c373..671b77eee83 100644 --- a/dbms/src/Interpreters/TraceCollector.cpp +++ b/dbms/src/Interpreters/TraceCollector.cpp @@ -1,6 +1,7 @@ #include "TraceCollector.h" -#include +#include +#include #include #include #include @@ -9,50 +10,62 @@ #include #include -namespace DB +using namespace DB; + +/** + * Sends TraceCollector stop message + * + * Each sequence of data for TraceCollector thread starts with a boolean flag. + * If this flag is true, TraceCollector must stop reading trace_pipe and exit. + * This function sends flag with a true value to stop TraceCollector gracefully. + * + * NOTE: TraceCollector will NOT stop immediately as there may be some data left in the pipe + * before stop message. + */ +void DB::NotifyTraceCollectorToStop() { + WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1]); + writeIntBinary(true, out); + out.next(); +} - TraceCollector::TraceCollector(std::shared_ptr trace_log) - : log(&Logger::get("TraceCollector")) - , trace_log(trace_log) +TraceCollector::TraceCollector(std::shared_ptr trace_log) + : log(&Poco::Logger::get("TraceCollector")) + , trace_log(trace_log) +{ + if (trace_log == nullptr) + throw Poco::Exception("Invalid trace log pointer passed"); +} + +void TraceCollector::run() +{ + ReadBufferFromFileDescriptor in(trace_pipe.fds_rw[0]); + + while (true) { - } + char is_last; + readChar(is_last, in); + if (is_last) + break; - void TraceCollector::run() - { - DB::ReadBufferFromFileDescriptor in(trace_pipe.fds_rw[0]); + std::string query_id; + StackTrace stack_trace(NoCapture{}); + TimerType timer_type; - while (true) - { - SleepForMicroseconds(1); + readStringBinary(query_id, in); + readPODBinary(stack_trace, in); + readPODBinary(timer_type, in); - bool is_last; - DB::readIntBinary(is_last, in); - if (is_last) - break; + const auto size = stack_trace.getSize(); + const auto& frames = stack_trace.getFrames(); - std::string query_id; - StackTrace stack_trace(NoCapture{}); - TimerType timer_type; + Array trace; + trace.reserve(size); + for (size_t i = 0; i < size; i++) + trace.emplace_back(UInt64(reinterpret_cast(frames[i]))); - DB::readStringBinary(query_id, in); - DB::readPODBinary(stack_trace, in); - DB::readIntBinary(timer_type, in); + TraceLogElement element{std::time(nullptr), timer_type, query_id, trace}; - if (trace_log != nullptr) - { - const auto size = stack_trace.getSize(); - const auto& frames = stack_trace.getFrames(); - - std::vector trace; - trace.reserve(size); - for (size_t i = 0; i < size; i++) - trace.push_back(reinterpret_cast(frames[i])); - - TraceLogElement element{std::time(nullptr), timer_type, query_id, trace}; - - trace_log->add(element); - } - } + trace_log->add(element); } } diff --git a/dbms/src/Interpreters/TraceCollector.h b/dbms/src/Interpreters/TraceCollector.h index 0e70873d91d..43a712a9816 100644 --- a/dbms/src/Interpreters/TraceCollector.h +++ b/dbms/src/Interpreters/TraceCollector.h @@ -1,24 +1,28 @@ #pragma once -#include #include -#include -#include #include +namespace Poco +{ + class Logger; +} + namespace DB { - using Poco::Logger; - class TraceCollector : public Poco::Runnable - { - private: - Logger * log; - std::shared_ptr trace_log; +void NotifyTraceCollectorToStop(); - public: - TraceCollector(std::shared_ptr trace_log); +class TraceCollector : public Poco::Runnable +{ +private: + Poco::Logger * log; + std::shared_ptr trace_log; + +public: + TraceCollector(std::shared_ptr trace_log); + + void run() override; +}; - void run() override; - }; } diff --git a/dbms/src/Interpreters/TraceLog.cpp b/dbms/src/Interpreters/TraceLog.cpp index 0661fbb8da0..c5583476e99 100644 --- a/dbms/src/Interpreters/TraceLog.cpp +++ b/dbms/src/Interpreters/TraceLog.cpp @@ -36,13 +36,7 @@ void TraceLogElement::appendToBlock(Block &block) const columns[i++]->insert(event_time); columns[i++]->insert(static_cast(timer_type)); columns[i++]->insertData(query_id.data(), query_id.size()); - { - Array trace_array; - trace_array.reserve(trace.size()); - for (const UInt32 trace_address : trace) - trace_array.emplace_back(UInt64(trace_address)); - columns[i++]->insert(trace_array); - } + columns[i++]->insert(trace); block.setColumns(std::move(columns)); } diff --git a/dbms/src/Interpreters/TraceLog.h b/dbms/src/Interpreters/TraceLog.h index bfff8c425d3..d5b38b69440 100644 --- a/dbms/src/Interpreters/TraceLog.h +++ b/dbms/src/Interpreters/TraceLog.h @@ -17,7 +17,7 @@ struct TraceLogElement time_t event_time{}; TimerType timer_type; String query_id{}; - std::vector trace{}; + Array trace{}; static std::string name() { return "TraceLog"; } static Block createBlock(); diff --git a/libs/libcommon/CMakeLists.txt b/libs/libcommon/CMakeLists.txt index a2d641921b4..7469aabceec 100644 --- a/libs/libcommon/CMakeLists.txt +++ b/libs/libcommon/CMakeLists.txt @@ -63,8 +63,7 @@ add_library (common include/ext/unlock_guard.h include/ext/singleton.h - ${CONFIG_COMMON} -) + ${CONFIG_COMMON}) if (USE_UNWIND) target_compile_definitions (common PRIVATE USE_UNWIND=1) @@ -133,8 +132,7 @@ target_link_libraries (common PRIVATE ${MALLOC_LIBRARIES} Threads::Threads - ${MEMCPY_LIBRARIES} -) + ${MEMCPY_LIBRARIES}) if (RT_LIBRARY) target_link_libraries (common PRIVATE ${RT_LIBRARY}) diff --git a/libs/libcommon/cmake/find_unwind.cmake b/libs/libcommon/cmake/find_unwind.cmake deleted file mode 100644 index 876bc7298e8..00000000000 --- a/libs/libcommon/cmake/find_unwind.cmake +++ /dev/null @@ -1,52 +0,0 @@ -include (CMakePushCheckState) -cmake_push_check_state () - -option (ENABLE_UNWIND "Enable libunwind (better stacktraces)" ON) - -if (ENABLE_UNWIND) - -if (CMAKE_SYSTEM MATCHES "Linux" AND NOT ARCH_ARM AND NOT ARCH_32) - option (USE_INTERNAL_UNWIND_LIBRARY "Set to FALSE to use system unwind library instead of bundled" ${NOT_UNBUNDLED}) -else () - option (USE_INTERNAL_UNWIND_LIBRARY "Set to FALSE to use system unwind library instead of bundled" OFF) -endif () - -if (NOT USE_INTERNAL_UNWIND_LIBRARY) - find_library (UNWIND_LIBRARY unwind) - find_path (UNWIND_INCLUDE_DIR NAMES unwind.h PATHS ${UNWIND_INCLUDE_PATHS}) - - include (CheckCXXSourceCompiles) - set(CMAKE_REQUIRED_INCLUDES ${UNWIND_INCLUDE_DIR}) - set(CMAKE_REQUIRED_LIBRARIES ${UNWIND_LIBRARY}) - check_cxx_source_compiles(" - #include - #define UNW_LOCAL_ONLY - #include - int main () { - ucontext_t context; - unw_cursor_t cursor; - unw_init_local2(&cursor, &context, UNW_INIT_SIGNAL_FRAME); - return 0; - } - " HAVE_UNW_INIT_LOCAL2) - if (NOT HAVE_UNW_INIT_LOCAL2) - set(UNWIND_LIBRARY "") - set(UNWIND_INCLUDE_DIR "") - endif () - -endif () - -if (UNWIND_LIBRARY AND UNWIND_INCLUDE_DIR) - set (USE_UNWIND 1) -elseif (CMAKE_SYSTEM MATCHES "Linux" AND NOT ARCH_ARM AND NOT ARCH_32) - set (USE_INTERNAL_UNWIND_LIBRARY 1) - set (UNWIND_INCLUDE_DIR "${ClickHouse_SOURCE_DIR}/contrib/libunwind/include") - set (UNWIND_LIBRARY unwind) - set (USE_UNWIND 1) -endif () - -endif () - -message (STATUS "Using unwind=${USE_UNWIND}: ${UNWIND_INCLUDE_DIR} : ${UNWIND_LIBRARY}") - -cmake_pop_check_state () diff --git a/libs/libcommon/include/common/Pipe.h b/libs/libcommon/include/common/Pipe.h index 394db6d199c..3904099c9c2 100644 --- a/libs/libcommon/include/common/Pipe.h +++ b/libs/libcommon/include/common/Pipe.h @@ -1,11 +1,7 @@ #pragma once -#include -#include +#include #include -#include -#include -#include #include struct LazyPipe diff --git a/libs/libcommon/include/common/Sleep.h b/libs/libcommon/include/common/Sleep.h index f6c5fb8090b..7c5c955f723 100644 --- a/libs/libcommon/include/common/Sleep.h +++ b/libs/libcommon/include/common/Sleep.h @@ -1,8 +1,6 @@ #pragma once #include -#include -#include void SleepForNanoseconds(uint64_t nanoseconds); diff --git a/libs/libcommon/include/common/StackTrace.h b/libs/libcommon/include/common/StackTrace.h index 9c27ffdcc65..be3d6438386 100644 --- a/libs/libcommon/include/common/StackTrace.h +++ b/libs/libcommon/include/common/StackTrace.h @@ -35,7 +35,14 @@ public: StackTrace(NoCapture); /// Fills stack trace frames with provided sequence - StackTrace(const std::vector & source_frames); + template + StackTrace(Iterator it, Iterator end) + { + while (size < capacity && it != end) + { + frames[size++] = *(it++); + } + } size_t getSize() const; const Frames & getFrames() const; diff --git a/libs/libcommon/src/Sleep.cpp b/libs/libcommon/src/Sleep.cpp index 3a27b0080cd..13b03f2b95e 100644 --- a/libs/libcommon/src/Sleep.cpp +++ b/libs/libcommon/src/Sleep.cpp @@ -1,6 +1,17 @@ #include "common/Sleep.h" +#include +#include +/** + * Sleep with nanoseconds precision + * + * In case query profiler is turned on, all threads spawned for + * query execution are repeatedly interrupted by signals from timer. + * Functions for relative sleep (sleep(3), nanosleep(2), etc.) have + * problems in this setup and man page for nanosleep(2) suggests + * using absolute deadlines, for instance clock_nanosleep(2). + */ void SleepForNanoseconds(uint64_t nanoseconds) { const auto clock_type = CLOCK_REALTIME; diff --git a/libs/libcommon/src/StackTrace.cpp b/libs/libcommon/src/StackTrace.cpp index ea60626edf0..8aea884d004 100644 --- a/libs/libcommon/src/StackTrace.cpp +++ b/libs/libcommon/src/StackTrace.cpp @@ -195,12 +195,6 @@ StackTrace::StackTrace(NoCapture) { } -StackTrace::StackTrace(const std::vector & source_frames) -{ - for (size = 0; size < std::min(source_frames.size(), capacity); ++size) - frames[size] = source_frames[size]; -} - void StackTrace::tryCapture() { size = 0; diff --git a/libs/libdaemon/src/BaseDaemon.cpp b/libs/libdaemon/src/BaseDaemon.cpp index f5de0f0231a..1ba03168496 100644 --- a/libs/libdaemon/src/BaseDaemon.cpp +++ b/libs/libdaemon/src/BaseDaemon.cpp @@ -1,7 +1,4 @@ #include -#include -#include -#include #include #include #include From 6b7f5871569c482c81c932af2cecfd4c6cb567df Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Fri, 5 Jul 2019 14:16:20 +0000 Subject: [PATCH 33/57] cut large query_ids --- dbms/src/Common/QueryProfiler.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dbms/src/Common/QueryProfiler.h b/dbms/src/Common/QueryProfiler.h index 236a4907a15..81a3d564c3f 100644 --- a/dbms/src/Common/QueryProfiler.h +++ b/dbms/src/Common/QueryProfiler.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -10,7 +11,6 @@ #include #include - namespace Poco { class Logger; @@ -31,11 +31,11 @@ namespace { /// Normally query_id is a UUID (string with a fixed length) but user can provide custom query_id. /// Thus upper bound on query_id length should be introduced to avoid buffer overflow in signal handler. - constexpr UInt32 QUERY_ID_MAX_LEN = 1024; + constexpr size_t QUERY_ID_MAX_LEN = 1024; void writeTraceInfo(TimerType timer_type, int /* sig */, siginfo_t * /* info */, void * context) { - constexpr UInt32 buf_size = sizeof(char) + // TraceCollector stop flag + constexpr size_t buf_size = sizeof(char) + // TraceCollector stop flag 8 * sizeof(char) + // maximum VarUInt length for string size QUERY_ID_MAX_LEN * sizeof(char) + // maximum query_id length sizeof(StackTrace) + // collected stack trace @@ -44,12 +44,13 @@ namespace DB::WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1], buf_size, buffer); const std::string & query_id = CurrentThread::getQueryId(); + const StringRef cut_query_id(query_id.c_str(), std::min(query_id.size(), QUERY_ID_MAX_LEN)); const auto signal_context = *reinterpret_cast(context); const StackTrace stack_trace(signal_context); DB::writeChar(false, out); - DB::writeStringBinary(query_id, out); + DB::writeStringBinary(cut_query_id, out); DB::writePODBinary(stack_trace, out); DB::writePODBinary(timer_type, out); out.next(); From c7eaa30870500b1fd00be1289d344fb0fdaf62a0 Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Fri, 5 Jul 2019 21:02:02 +0000 Subject: [PATCH 34/57] update libunwind to RIP experiment --- contrib/libunwind | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/libunwind b/contrib/libunwind index ec86b1c6a2c..17a48fbfa79 160000 --- a/contrib/libunwind +++ b/contrib/libunwind @@ -1 +1 @@ -Subproject commit ec86b1c6a2c6b8ba316f429db9a6d4122dd12710 +Subproject commit 17a48fbfa7913ee889960a698516bd3ba51d63ee From 17e3542a5a70294d1c0014a94f91a186d8da70a8 Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Fri, 5 Jul 2019 22:35:09 +0000 Subject: [PATCH 35/57] refactor --- dbms/src/Common/QueryProfiler.h | 6 +++--- dbms/src/Interpreters/ThreadStatusExt.cpp | 15 ++++----------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/dbms/src/Common/QueryProfiler.h b/dbms/src/Common/QueryProfiler.h index 81a3d564c3f..eea1bb0374d 100644 --- a/dbms/src/Common/QueryProfiler.h +++ b/dbms/src/Common/QueryProfiler.h @@ -43,14 +43,14 @@ namespace char buffer[buf_size]; DB::WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1], buf_size, buffer); - const std::string & query_id = CurrentThread::getQueryId(); - const StringRef cut_query_id(query_id.c_str(), std::min(query_id.size(), QUERY_ID_MAX_LEN)); + StringRef query_id = CurrentThread::getQueryId(); + query_id.size = std::min(query_id.size, QUERY_ID_MAX_LEN); const auto signal_context = *reinterpret_cast(context); const StackTrace stack_trace(signal_context); DB::writeChar(false, out); - DB::writeStringBinary(cut_query_id, out); + DB::writeStringBinary(query_id, out); DB::writePODBinary(stack_trace, out); DB::writePODBinary(timer_type, out); out.next(); diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index a9596577f78..ddb7d9c27a7 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -6,10 +6,6 @@ #include #include #include -#include -#include -#include -#include /// Implement some methods of ThreadStatus and CurrentThread here to avoid extra linking dependencies in clickhouse_common_io /// TODO It doesn't make sense. @@ -32,15 +28,13 @@ void ThreadStatus::attachQueryContext(Context & query_context_) if (!thread_group->global_context) thread_group->global_context = global_context; } + + initQueryProfiler(); } StringRef ThreadStatus::getQueryId() const { - static const std::string empty = ""; - if (query_context) - return query_context->getClientInfo().current_query_id; - - return empty; + return query_id; } void CurrentThread::defaultThreadDeleter() @@ -102,7 +96,6 @@ void ThreadStatus::attachQuery(const ThreadGroupStatusPtr & thread_group_, bool } initPerformanceCounters(); - initQueryProfiler(); thread_state = ThreadState::AttachedToQuery; } @@ -134,7 +127,7 @@ void ThreadStatus::finalizePerformanceCounters() void ThreadStatus::initQueryProfiler() { if (!query_context) - return; + throw Exception("Can't init query profiler without query context", ErrorCodes::LOGICAL_ERROR); auto & settings = query_context->getSettingsRef(); From ef1d84b35a1eb07a6d0b0452f386f3edd6ead4e0 Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Sat, 6 Jul 2019 20:29:00 +0000 Subject: [PATCH 36/57] do not run trace collector without trace_log. do not run query profilers without trace collector. --- dbms/src/Interpreters/Context.cpp | 14 +++++++++++++- dbms/src/Interpreters/Context.h | 1 + dbms/src/Interpreters/ThreadStatusExt.cpp | 9 +++++---- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 466d122ad82..451d05e7085 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -292,7 +292,7 @@ struct ContextShared ddl_worker.reset(); /// Trace collector is only initialized in server program - if (trace_collector != nullptr) + if (hasTraceCollector()) { /// Stop trace collector NotifyTraceCollectorToStop(); @@ -304,8 +304,15 @@ struct ContextShared } } + bool hasTraceCollector() { + return trace_collector != nullptr; + } + void initializeTraceCollector(std::shared_ptr trace_log) { + if (trace_log == nullptr) + return; + trace_pipe.open(); trace_collector.reset(new TraceCollector(trace_log)); trace_collector_thread.start(*trace_collector); @@ -1646,6 +1653,11 @@ void Context::initializeSystemLogs() shared->system_logs.emplace(*global_context, getConfigRef()); } +bool Context::hasTraceCollector() +{ + return shared->hasTraceCollector(); +} + void Context::initializeTraceCollector() { shared->initializeTraceCollector(getTraceLog()); diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index a17565a60bb..4f622a0eae1 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -416,6 +416,7 @@ public: void initializeSystemLogs(); void initializeTraceCollector(); + bool hasTraceCollector(); /// Nullptr if the query log is not ready for this moment. std::shared_ptr getQueryLog(); diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index ddb7d9c27a7..cc108f31820 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -126,8 +126,9 @@ void ThreadStatus::finalizePerformanceCounters() void ThreadStatus::initQueryProfiler() { - if (!query_context) - throw Exception("Can't init query profiler without query context", ErrorCodes::LOGICAL_ERROR); + /// query profilers are useless without trace collector + if (!global_context->hasTraceCollector()) + return; auto & settings = query_context->getSettingsRef(); @@ -146,8 +147,8 @@ void ThreadStatus::initQueryProfiler() void ThreadStatus::finalizeQueryProfiler() { - query_profiler_real.reset(nullptr); - query_profiler_cpu.reset(nullptr); + query_profiler_real.reset(); + query_profiler_cpu.reset(); } void ThreadStatus::detachQuery(bool exit_if_already_detached, bool thread_exits) From 3828684f7a9cf1a06f13eda1ab4dd6fba98161a9 Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Sat, 6 Jul 2019 20:42:03 +0000 Subject: [PATCH 37/57] style --- dbms/src/Interpreters/Context.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 451d05e7085..407cb9366d1 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -304,7 +304,8 @@ struct ContextShared } } - bool hasTraceCollector() { + bool hasTraceCollector() + { return trace_collector != nullptr; } From 9d540abc84ca2b8ba648b7dd78e5b6641b5b73d3 Mon Sep 17 00:00:00 2001 From: Nikita Lapkov Date: Wed, 10 Jul 2019 20:47:39 +0000 Subject: [PATCH 38/57] refactor --- dbms/programs/server/config.xml | 2 +- dbms/src/Common/CurrentThread.h | 8 +- dbms/src/Common/ErrorCodes.cpp | 3 + dbms/src/Common/QueryProfiler.cpp | 128 +++++++++++++++++- dbms/src/Common/QueryProfiler.h | 102 ++------------ dbms/src/Common/ThreadStatus.h | 5 +- dbms/src/Common/Throttler.h | 4 +- .../TraceCollector.cpp | 55 ++++++-- dbms/src/Common/TraceCollector.h | 32 +++++ dbms/src/Core/Settings.h | 4 +- dbms/src/DataStreams/IBlockInputStream.cpp | 4 +- dbms/src/Functions/sleep.h | 4 +- dbms/src/Interpreters/Context.cpp | 22 +-- dbms/src/Interpreters/Context.h | 1 - dbms/src/Interpreters/DDLWorker.cpp | 4 +- dbms/src/Interpreters/ThreadStatusExt.cpp | 22 +-- dbms/src/Interpreters/TraceCollector.h | 28 ---- libs/libcommon/CMakeLists.txt | 5 +- libs/libcommon/include/common/Pipe.h | 55 ++------ libs/libcommon/include/common/Sleep.h | 11 -- libs/libcommon/include/common/sleep.h | 16 +++ libs/libcommon/src/Pipe.cpp | 45 ++++++ libs/libcommon/src/{Sleep.cpp => sleep.cpp} | 22 +-- libs/libmysqlxx/src/Pool.cpp | 6 +- 24 files changed, 338 insertions(+), 250 deletions(-) rename dbms/src/{Interpreters => Common}/TraceCollector.cpp (65%) create mode 100644 dbms/src/Common/TraceCollector.h delete mode 100644 dbms/src/Interpreters/TraceCollector.h delete mode 100644 libs/libcommon/include/common/Sleep.h create mode 100644 libs/libcommon/include/common/sleep.h create mode 100644 libs/libcommon/src/Pipe.cpp rename libs/libcommon/src/{Sleep.cpp => sleep.cpp} (62%) diff --git a/dbms/programs/server/config.xml b/dbms/programs/server/config.xml index e78e3c255f7..a8f37a2cd78 100644 --- a/dbms/programs/server/config.xml +++ b/dbms/programs/server/config.xml @@ -295,7 +295,7 @@ + See query_profiler_real_time_period_ns and query_profiler_cpu_time_period_ns settings. --> system trace_log
diff --git a/dbms/src/Common/CurrentThread.h b/dbms/src/Common/CurrentThread.h index 3c248ad903f..8b15bc7c3e3 100644 --- a/dbms/src/Common/CurrentThread.h +++ b/dbms/src/Common/CurrentThread.h @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -70,7 +71,12 @@ public: static void finalizePerformanceCounters(); /// Returns a non-empty string if the thread is attached to a query - static StringRef getQueryId(); + static StringRef getQueryId() + { + if (unlikely(!current_thread)) + return {}; + return current_thread->getQueryId(); + } /// Non-master threads call this method in destructor automatically static void detachQuery(); diff --git a/dbms/src/Common/ErrorCodes.cpp b/dbms/src/Common/ErrorCodes.cpp index a3b788c230f..c472a336d73 100644 --- a/dbms/src/Common/ErrorCodes.cpp +++ b/dbms/src/Common/ErrorCodes.cpp @@ -434,6 +434,9 @@ namespace ErrorCodes extern const int BAD_QUERY_PARAMETER = 457; extern const int CANNOT_UNLINK = 458; extern const int CANNOT_SET_THREAD_PRIORITY = 459; + extern const int CANNOT_CREATE_TIMER = 460; + extern const int CANNOT_SET_TIMER_PERIOD = 461; + extern const int CANNOT_DELETE_TIMER = 462; extern const int KEEPER_EXCEPTION = 999; extern const int POCO_EXCEPTION = 1000; diff --git a/dbms/src/Common/QueryProfiler.cpp b/dbms/src/Common/QueryProfiler.cpp index ce0ddef094f..9832b64ee8b 100644 --- a/dbms/src/Common/QueryProfiler.cpp +++ b/dbms/src/Common/QueryProfiler.cpp @@ -1,8 +1,134 @@ #include "QueryProfiler.h" +#include +#include +#include +#include +#include +#include +#include +#include + namespace DB { -LazyPipe trace_pipe; +extern LazyPipe trace_pipe; + +namespace +{ + /// Normally query_id is a UUID (string with a fixed length) but user can provide custom query_id. + /// Thus upper bound on query_id length should be introduced to avoid buffer overflow in signal handler. + constexpr size_t QUERY_ID_MAX_LEN = 1024; + + void writeTraceInfo(TimerType timer_type, int /* sig */, siginfo_t * /* info */, void * context) + { + constexpr size_t buf_size = sizeof(char) + // TraceCollector stop flag + 8 * sizeof(char) + // maximum VarUInt length for string size + QUERY_ID_MAX_LEN * sizeof(char) + // maximum query_id length + sizeof(StackTrace) + // collected stack trace + sizeof(TimerType); // timer type + char buffer[buf_size]; + WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1], buf_size, buffer); + + StringRef query_id = CurrentThread::getQueryId(); + query_id.size = std::min(query_id.size, QUERY_ID_MAX_LEN); + + const auto signal_context = *reinterpret_cast(context); + const StackTrace stack_trace(signal_context); + + writeChar(false, out); + writeStringBinary(query_id, out); + writePODBinary(stack_trace, out); + writePODBinary(timer_type, out); + out.next(); + } + + const UInt32 TIMER_PRECISION = 1e9; +} + +namespace ErrorCodes +{ + extern const int CANNOT_MANIPULATE_SIGSET; + extern const int CANNOT_SET_SIGNAL_HANDLER; + extern const int CANNOT_CREATE_TIMER; + extern const int CANNOT_SET_TIMER_PERIOD; + extern const int CANNOT_DELETE_TIMER; +} + +template +QueryProfilerBase::QueryProfilerBase(const Int32 thread_id, const int clock_type, const UInt32 period, const int pause_signal) + : log(&Logger::get("QueryProfiler")) + , pause_signal(pause_signal) +{ + struct sigaction sa{}; + sa.sa_sigaction = ProfilerImpl::signalHandler; + sa.sa_flags = SA_SIGINFO | SA_RESTART; + + if (sigemptyset(&sa.sa_mask)) + throwFromErrno("Failed to clean signal mask for query profiler", ErrorCodes::CANNOT_MANIPULATE_SIGSET); + + if (sigaddset(&sa.sa_mask, pause_signal)) + throwFromErrno("Failed to add signal to mask for query profiler", ErrorCodes::CANNOT_MANIPULATE_SIGSET); + + if (sigaction(pause_signal, &sa, previous_handler)) + throwFromErrno("Failed to setup signal handler for query profiler", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER); + + try + { + struct sigevent sev; + sev.sigev_notify = SIGEV_THREAD_ID; + sev.sigev_signo = pause_signal; + sev._sigev_un._tid = thread_id; + if (timer_create(clock_type, &sev, &timer_id)) + throwFromErrno("Failed to create thread timer", ErrorCodes::CANNOT_CREATE_TIMER); + + struct timespec interval{.tv_sec = period / TIMER_PRECISION, .tv_nsec = period % TIMER_PRECISION}; + struct itimerspec timer_spec = {.it_interval = interval, .it_value = interval}; + if (timer_settime(timer_id, 0, &timer_spec, nullptr)) + throwFromErrno("Failed to set thread timer period", ErrorCodes::CANNOT_SET_TIMER_PERIOD); + } + catch (...) + { + tryCleanup(); + throw; + } +} + +template +QueryProfilerBase::~QueryProfilerBase() +{ + tryCleanup(); +} + +template +void QueryProfilerBase::tryCleanup() +{ + if (timer_id != nullptr && timer_delete(timer_id)) + LOG_ERROR(log, "Failed to delete query profiler timer " + errnoToString(ErrorCodes::CANNOT_DELETE_TIMER)); + + if (previous_handler != nullptr && sigaction(pause_signal, previous_handler, nullptr)) + LOG_ERROR(log, "Failed to restore signal handler after query profiler " + errnoToString(ErrorCodes::CANNOT_SET_SIGNAL_HANDLER)); +} + +template class QueryProfilerBase; +template class QueryProfilerBase; + +QueryProfilerReal::QueryProfilerReal(const Int32 thread_id, const UInt32 period) + : QueryProfilerBase(thread_id, CLOCK_REALTIME, period, SIGUSR1) +{} + +void QueryProfilerReal::signalHandler(int sig, siginfo_t * info, void * context) +{ + writeTraceInfo(TimerType::Real, sig, info, context); +} + +QueryProfilerCpu::QueryProfilerCpu(const Int32 thread_id, const UInt32 period) + : QueryProfilerBase(thread_id, CLOCK_THREAD_CPUTIME_ID, period, SIGUSR2) +{} + +void QueryProfilerCpu::signalHandler(int sig, siginfo_t * info, void * context) +{ + writeTraceInfo(TimerType::Cpu, sig, info, context); +} } diff --git a/dbms/src/Common/QueryProfiler.h b/dbms/src/Common/QueryProfiler.h index eea1bb0374d..d4e92f25a17 100644 --- a/dbms/src/Common/QueryProfiler.h +++ b/dbms/src/Common/QueryProfiler.h @@ -1,12 +1,6 @@ #pragma once -#include -#include -#include -#include -#include -#include -#include +#include #include #include @@ -19,47 +13,12 @@ namespace Poco namespace DB { -extern LazyPipe trace_pipe; - enum class TimerType : UInt8 { Real, Cpu, }; -namespace -{ - /// Normally query_id is a UUID (string with a fixed length) but user can provide custom query_id. - /// Thus upper bound on query_id length should be introduced to avoid buffer overflow in signal handler. - constexpr size_t QUERY_ID_MAX_LEN = 1024; - - void writeTraceInfo(TimerType timer_type, int /* sig */, siginfo_t * /* info */, void * context) - { - constexpr size_t buf_size = sizeof(char) + // TraceCollector stop flag - 8 * sizeof(char) + // maximum VarUInt length for string size - QUERY_ID_MAX_LEN * sizeof(char) + // maximum query_id length - sizeof(StackTrace) + // collected stack trace - sizeof(TimerType); // timer type - char buffer[buf_size]; - DB::WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1], buf_size, buffer); - - StringRef query_id = CurrentThread::getQueryId(); - query_id.size = std::min(query_id.size, QUERY_ID_MAX_LEN); - - const auto signal_context = *reinterpret_cast(context); - const StackTrace stack_trace(signal_context); - - DB::writeChar(false, out); - DB::writeStringBinary(query_id, out); - DB::writePODBinary(stack_trace, out); - DB::writePODBinary(timer_type, out); - out.next(); - } - - const UInt32 TIMER_PRECISION = 1e9; -} - - /** * Query profiler implementation for selected thread. * @@ -75,46 +34,13 @@ template class QueryProfilerBase { public: - QueryProfilerBase(const Int32 thread_id, const int clock_type, const UInt32 period, const int pause_signal = SIGALRM) - : log(&Logger::get("QueryProfiler")) - , pause_signal(pause_signal) - { - struct sigaction sa{}; - sa.sa_sigaction = ProfilerImpl::signalHandler; - sa.sa_flags = SA_SIGINFO | SA_RESTART; + QueryProfilerBase(const Int32 thread_id, const int clock_type, const UInt32 period, const int pause_signal = SIGALRM); - if (sigemptyset(&sa.sa_mask)) - throw Poco::Exception("Failed to clean signal mask for query profiler"); - - if (sigaddset(&sa.sa_mask, pause_signal)) - throw Poco::Exception("Failed to add signal to mask for query profiler"); - - if (sigaction(pause_signal, &sa, previous_handler)) - throw Poco::Exception("Failed to setup signal handler for query profiler"); - - struct sigevent sev; - sev.sigev_notify = SIGEV_THREAD_ID; - sev.sigev_signo = pause_signal; - sev._sigev_un._tid = thread_id; - if (timer_create(clock_type, &sev, &timer_id)) - throw Poco::Exception("Failed to create thread timer"); - - struct timespec interval{.tv_sec = period / TIMER_PRECISION, .tv_nsec = period % TIMER_PRECISION}; - struct itimerspec timer_spec = {.it_interval = interval, .it_value = interval}; - if (timer_settime(timer_id, 0, &timer_spec, nullptr)) - throw Poco::Exception("Failed to set thread timer"); - } - - ~QueryProfilerBase() - { - if (timer_delete(timer_id)) - LOG_ERROR(log, "Failed to delete query profiler timer"); - - if (sigaction(pause_signal, previous_handler, nullptr)) - LOG_ERROR(log, "Failed to restore signal handler after query profiler"); - } + ~QueryProfilerBase(); private: + void tryCleanup(); + Poco::Logger * log; /// Timer id from timer_create(2) @@ -131,28 +57,18 @@ private: class QueryProfilerReal : public QueryProfilerBase { public: - QueryProfilerReal(const Int32 thread_id, const UInt32 period) - : QueryProfilerBase(thread_id, CLOCK_REALTIME, period, SIGUSR1) - {} + QueryProfilerReal(const Int32 thread_id, const UInt32 period); - static void signalHandler(int sig, siginfo_t * info, void * context) - { - writeTraceInfo(TimerType::Real, sig, info, context); - } + static void signalHandler(int sig, siginfo_t * info, void * context); }; /// Query profiler with timer based on CPU clock class QueryProfilerCpu : public QueryProfilerBase { public: - QueryProfilerCpu(const Int32 thread_id, const UInt32 period) - : QueryProfilerBase(thread_id, CLOCK_THREAD_CPUTIME_ID, period, SIGUSR2) - {} + QueryProfilerCpu(const Int32 thread_id, const UInt32 period); - static void signalHandler(int sig, siginfo_t * info, void * context) - { - writeTraceInfo(TimerType::Cpu, sig, info, context); - } + static void signalHandler(int sig, siginfo_t * info, void * context); }; } diff --git a/dbms/src/Common/ThreadStatus.h b/dbms/src/Common/ThreadStatus.h index 87ddd32044c..0b737ccb6a7 100644 --- a/dbms/src/Common/ThreadStatus.h +++ b/dbms/src/Common/ThreadStatus.h @@ -119,7 +119,10 @@ public: return thread_state.load(std::memory_order_relaxed); } - StringRef getQueryId() const; + StringRef getQueryId() const + { + return query_id; + } /// Starts new query and create new thread group for it, current thread becomes master thread of the query void initializeQuery(); diff --git a/dbms/src/Common/Throttler.h b/dbms/src/Common/Throttler.h index 6fd5a7f16c0..3ad50215b9e 100644 --- a/dbms/src/Common/Throttler.h +++ b/dbms/src/Common/Throttler.h @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include @@ -77,7 +77,7 @@ public: if (desired_ns > elapsed_ns) { UInt64 sleep_ns = desired_ns - elapsed_ns; - SleepForNanoseconds(sleep_ns); + sleepForNanoseconds(sleep_ns); ProfileEvents::increment(ProfileEvents::ThrottlerSleepMicroseconds, sleep_ns / 1000UL); } diff --git a/dbms/src/Interpreters/TraceCollector.cpp b/dbms/src/Common/TraceCollector.cpp similarity index 65% rename from dbms/src/Interpreters/TraceCollector.cpp rename to dbms/src/Common/TraceCollector.cpp index 671b77eee83..293e4c38e97 100644 --- a/dbms/src/Interpreters/TraceCollector.cpp +++ b/dbms/src/Common/TraceCollector.cpp @@ -2,15 +2,50 @@ #include #include +#include #include #include #include #include +#include +#include #include -#include #include -using namespace DB; +namespace DB +{ + +LazyPipe trace_pipe; + +namespace ErrorCodes +{ + extern const int NULL_POINTER_DEREFERENCE; + extern const int THREAD_IS_NOT_JOINABLE; +} + +TraceCollector::TraceCollector(std::shared_ptr & trace_log) + : log(&Poco::Logger::get("TraceCollector")) + , trace_log(trace_log) +{ + if (trace_log == nullptr) + throw Exception("Invalid trace log pointer passed", ErrorCodes::NULL_POINTER_DEREFERENCE); + + trace_pipe.open(); + thread = ThreadFromGlobalPool(&TraceCollector::run, this); +} + +TraceCollector::~TraceCollector() +{ + if (!thread.joinable()) + LOG_ERROR(log, "TraceCollector thread is malformed and cannot be joined"); + else + { + TraceCollector::notifyToStop(); + thread.join(); + } + + trace_pipe.close(); +} /** * Sends TraceCollector stop message @@ -22,21 +57,13 @@ using namespace DB; * NOTE: TraceCollector will NOT stop immediately as there may be some data left in the pipe * before stop message. */ -void DB::NotifyTraceCollectorToStop() +void TraceCollector::notifyToStop() { WriteBufferFromFileDescriptor out(trace_pipe.fds_rw[1]); - writeIntBinary(true, out); + writeChar(true, out); out.next(); } -TraceCollector::TraceCollector(std::shared_ptr trace_log) - : log(&Poco::Logger::get("TraceCollector")) - , trace_log(trace_log) -{ - if (trace_log == nullptr) - throw Poco::Exception("Invalid trace log pointer passed"); -} - void TraceCollector::run() { ReadBufferFromFileDescriptor in(trace_pipe.fds_rw[0]); @@ -57,7 +84,7 @@ void TraceCollector::run() readPODBinary(timer_type, in); const auto size = stack_trace.getSize(); - const auto& frames = stack_trace.getFrames(); + const auto & frames = stack_trace.getFrames(); Array trace; trace.reserve(size); @@ -69,3 +96,5 @@ void TraceCollector::run() trace_log->add(element); } } + +} diff --git a/dbms/src/Common/TraceCollector.h b/dbms/src/Common/TraceCollector.h new file mode 100644 index 00000000000..7c07f48776f --- /dev/null +++ b/dbms/src/Common/TraceCollector.h @@ -0,0 +1,32 @@ +#pragma once + +#include + +namespace Poco +{ + class Logger; +} + +namespace DB +{ + +class TraceLog; + +class TraceCollector +{ +private: + Poco::Logger * log; + std::shared_ptr trace_log; + ThreadFromGlobalPool thread; + + void run(); + + static void notifyToStop(); + +public: + TraceCollector(std::shared_ptr & trace_log); + + ~TraceCollector(); +}; + +} diff --git a/dbms/src/Core/Settings.h b/dbms/src/Core/Settings.h index e718da6fa99..967615d6da1 100644 --- a/dbms/src/Core/Settings.h +++ b/dbms/src/Core/Settings.h @@ -220,8 +220,8 @@ struct Settings : public SettingsCollection M(SettingBool, empty_result_for_aggregation_by_empty_set, false, "Return empty result when aggregating without keys on empty set.") \ M(SettingBool, allow_distributed_ddl, true, "If it is set to true, then a user is allowed to executed distributed DDL queries.") \ M(SettingUInt64, odbc_max_field_size, 1024, "Max size of filed can be read from ODBC dictionary. Long strings are truncated.") \ - M(SettingUInt64, query_profiler_real_time_period, 500000000, "Period for real clock timer of query profiler (in nanoseconds). Set 0 value to turn off real clock query profiler") \ - M(SettingUInt64, query_profiler_cpu_time_period, 500000000, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off CPU clock query profiler") \ + M(SettingUInt64, query_profiler_real_time_period_ns, 500000000, "Period for real clock timer of query profiler (in nanoseconds). Set 0 value to turn off real clock query profiler") \ + M(SettingUInt64, query_profiler_cpu_time_period_ns, 500000000, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off CPU clock query profiler") \ \ \ /** Limits during query execution are part of the settings. \ diff --git a/dbms/src/DataStreams/IBlockInputStream.cpp b/dbms/src/DataStreams/IBlockInputStream.cpp index a2d52dfc36d..406a660879c 100644 --- a/dbms/src/DataStreams/IBlockInputStream.cpp +++ b/dbms/src/DataStreams/IBlockInputStream.cpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include namespace ProfileEvents { @@ -255,7 +255,7 @@ static void limitProgressingSpeed(size_t total_progress_size, size_t max_speed_i if (desired_microseconds > total_elapsed_microseconds) { UInt64 sleep_microseconds = desired_microseconds - total_elapsed_microseconds; - SleepForMicroseconds(sleep_microseconds); + sleepForMicroseconds(sleep_microseconds); ProfileEvents::increment(ProfileEvents::ThrottlerSleepMicroseconds, sleep_microseconds); } diff --git a/dbms/src/Functions/sleep.h b/dbms/src/Functions/sleep.h index ff7cb36786e..5e9732d59f4 100644 --- a/dbms/src/Functions/sleep.h +++ b/dbms/src/Functions/sleep.h @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include @@ -88,7 +88,7 @@ public: throw Exception("The maximum sleep time is 3 seconds. Requested: " + toString(seconds), ErrorCodes::TOO_SLOW); UInt64 microseconds = seconds * (variant == FunctionSleepVariant::PerBlock ? 1 : size) * 1e6; - SleepForMicroseconds(microseconds); + sleepForMicroseconds(microseconds); } /// convertToFullColumn needed, because otherwise (constant expression case) function will not get called on each block. diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 407cb9366d1..bd2c03cb78a 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -54,8 +54,8 @@ #include #include #include +#include #include -#include "TraceCollector.h" namespace ProfileEvents @@ -155,8 +155,7 @@ struct ContextShared ActionLocksManagerPtr action_locks_manager; /// Set of storages' action lockers std::optional system_logs; /// Used to log queries and operations on parts - Poco::Thread trace_collector_thread; /// Thread collecting traces from threads executing queries - std::unique_ptr trace_collector; + std::unique_ptr trace_collector; /// Thread collecting traces from threads executing queries /// Named sessions. The user could specify session identifier to reuse settings and temporary tables in subsequent requests. @@ -291,17 +290,8 @@ struct ContextShared schedule_pool.reset(); ddl_worker.reset(); - /// Trace collector is only initialized in server program - if (hasTraceCollector()) - { - /// Stop trace collector - NotifyTraceCollectorToStop(); - trace_collector_thread.join(); - - /// Close trace pipe - definitely nobody needs to write there after - /// databases shutdown - trace_pipe.close(); - } + /// Stop trace collector if any + trace_collector.reset(); } bool hasTraceCollector() @@ -314,9 +304,7 @@ struct ContextShared if (trace_log == nullptr) return; - trace_pipe.open(); - trace_collector.reset(new TraceCollector(trace_log)); - trace_collector_thread.start(*trace_collector); + trace_collector = std::make_unique(trace_log); } private: diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index 4f622a0eae1..58745d97add 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -136,7 +136,6 @@ private: Context * session_context = nullptr; /// Session context or nullptr. Could be equal to this. Context * global_context = nullptr; /// Global context or nullptr. Could be equal to this. - UInt64 session_close_cycle = 0; bool session_is_used = false; diff --git a/dbms/src/Interpreters/DDLWorker.cpp b/dbms/src/Interpreters/DDLWorker.cpp index 4a3665f3d50..e445b7503ce 100644 --- a/dbms/src/Interpreters/DDLWorker.cpp +++ b/dbms/src/Interpreters/DDLWorker.cpp @@ -22,7 +22,7 @@ #include #include #include -#include +#include #include #include #include @@ -953,7 +953,7 @@ void DDLWorker::runMainThread() tryLogCurrentException(__PRETTY_FUNCTION__); /// Avoid busy loop when ZooKeeper is not available. - SleepForSeconds(1); + sleepForSeconds(1); } } catch (...) diff --git a/dbms/src/Interpreters/ThreadStatusExt.cpp b/dbms/src/Interpreters/ThreadStatusExt.cpp index d0d1badc8ad..c4f1fa055b3 100644 --- a/dbms/src/Interpreters/ThreadStatusExt.cpp +++ b/dbms/src/Interpreters/ThreadStatusExt.cpp @@ -46,11 +46,6 @@ void ThreadStatus::attachQueryContext(Context & query_context_) initQueryProfiler(); } -StringRef ThreadStatus::getQueryId() const -{ - return query_id; -} - void CurrentThread::defaultThreadDeleter() { if (unlikely(!current_thread)) @@ -161,18 +156,18 @@ void ThreadStatus::initQueryProfiler() if (!global_context->hasTraceCollector()) return; - auto & settings = query_context->getSettingsRef(); + const auto & settings = query_context->getSettingsRef(); - if (settings.query_profiler_real_time_period > 0) + if (settings.query_profiler_real_time_period_ns > 0) query_profiler_real = std::make_unique( /* thread_id */ os_thread_id, - /* period */ static_cast(settings.query_profiler_real_time_period) + /* period */ static_cast(settings.query_profiler_real_time_period_ns) ); - if (settings.query_profiler_cpu_time_period > 0) + if (settings.query_profiler_cpu_time_period_ns > 0) query_profiler_cpu = std::make_unique( /* thread_id */ os_thread_id, - /* period */ static_cast(settings.query_profiler_cpu_time_period) + /* period */ static_cast(settings.query_profiler_cpu_time_period_ns) ); } @@ -291,13 +286,6 @@ void CurrentThread::attachToIfDetached(const ThreadGroupStatusPtr & thread_group current_thread->deleter = CurrentThread::defaultThreadDeleter; } -StringRef CurrentThread::getQueryId() -{ - if (unlikely(!current_thread)) - return {}; - return current_thread->getQueryId(); -} - void CurrentThread::attachQueryContext(Context & query_context) { if (unlikely(!current_thread)) diff --git a/dbms/src/Interpreters/TraceCollector.h b/dbms/src/Interpreters/TraceCollector.h deleted file mode 100644 index 43a712a9816..00000000000 --- a/dbms/src/Interpreters/TraceCollector.h +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include -#include - -namespace Poco -{ - class Logger; -} - -namespace DB -{ - -void NotifyTraceCollectorToStop(); - -class TraceCollector : public Poco::Runnable -{ -private: - Poco::Logger * log; - std::shared_ptr trace_log; - -public: - TraceCollector(std::shared_ptr trace_log); - - void run() override; -}; - -} diff --git a/libs/libcommon/CMakeLists.txt b/libs/libcommon/CMakeLists.txt index 7469aabceec..eb77f43a37f 100644 --- a/libs/libcommon/CMakeLists.txt +++ b/libs/libcommon/CMakeLists.txt @@ -21,9 +21,10 @@ add_library (common src/demangle.cpp src/setTerminalEcho.cpp src/getThreadNumber.cpp - src/Sleep.cpp + src/sleep.cpp src/argsToConfig.cpp src/StackTrace.cpp + src/Pipe.cpp include/common/SimpleCache.h include/common/StackTrace.h @@ -48,7 +49,7 @@ add_library (common include/common/constexpr_helpers.h include/common/Pipe.h include/common/getThreadNumber.h - include/common/Sleep.h + include/common/sleep.h include/common/SimpleCache.h include/ext/bit_cast.h diff --git a/libs/libcommon/include/common/Pipe.h b/libs/libcommon/include/common/Pipe.h index 3904099c9c2..0137c3d97af 100644 --- a/libs/libcommon/include/common/Pipe.h +++ b/libs/libcommon/include/common/Pipe.h @@ -4,56 +4,31 @@ #include #include +/** + * Struct containing a pipe with lazy initialization. + * Use `open` and `close` methods to manipulate pipe and `fds_rw` field to access + * pipe's file descriptors. + */ struct LazyPipe { int fds_rw[2] = {-1, -1}; LazyPipe() = default; - virtual void open() - { - for (int &fd : fds_rw) - { - if (fd >= 0) - { - throw std::logic_error("Pipe is already opened"); - } - } + void open(); -#ifndef __APPLE__ - if (0 != pipe2(fds_rw, O_CLOEXEC)) - throw std::runtime_error("Cannot create pipe"); -#else - if (0 != pipe(fds_rw)) - throw std::runtime_error("Cannot create pipe"); - if (0 != fcntl(fds_rw[0], F_SETFD, FD_CLOEXEC)) - throw std::runtime_error("Cannot setup auto-close on exec for read end of pipe"); - if (0 != fcntl(fds_rw[1], F_SETFD, FD_CLOEXEC)) - throw std::runtime_error("Cannot setup auto-close on exec for write end of pipe"); -#endif - } - - virtual void close() { - for (int fd : fds_rw) - { - if (fd >= 0) - { - ::close(fd); - } - } - } + void close(); virtual ~LazyPipe() = default; }; -struct Pipe : public LazyPipe { - Pipe() - { - open(); - } +/** + * Struct which opens new pipe on creation and closes it on destruction. + * Use `fds_rw` field to access pipe's file descriptors. + */ +struct Pipe : public LazyPipe +{ + Pipe(); - ~Pipe() - { - close(); - } + ~Pipe(); }; diff --git a/libs/libcommon/include/common/Sleep.h b/libs/libcommon/include/common/Sleep.h deleted file mode 100644 index 7c5c955f723..00000000000 --- a/libs/libcommon/include/common/Sleep.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#include - -void SleepForNanoseconds(uint64_t nanoseconds); - -void SleepForMicroseconds(uint64_t microseconds); - -void SleepForMilliseconds(uint64_t milliseconds); - -void SleepForSeconds(uint64_t seconds); diff --git a/libs/libcommon/include/common/sleep.h b/libs/libcommon/include/common/sleep.h new file mode 100644 index 00000000000..6ae99a4a57d --- /dev/null +++ b/libs/libcommon/include/common/sleep.h @@ -0,0 +1,16 @@ +#pragma once + +#include + +/** + * Sleep functions tolerant to signal interruptions (which can happen + * when query profiler is turned on for example) + */ + +void sleepForNanoseconds(uint64_t nanoseconds); + +void sleepForMicroseconds(uint64_t microseconds); + +void sleepForMilliseconds(uint64_t milliseconds); + +void sleepForSeconds(uint64_t seconds); diff --git a/libs/libcommon/src/Pipe.cpp b/libs/libcommon/src/Pipe.cpp new file mode 100644 index 00000000000..83268b76ea6 --- /dev/null +++ b/libs/libcommon/src/Pipe.cpp @@ -0,0 +1,45 @@ +#include "common/Pipe.h" + +void LazyPipe::open() +{ + for (int & fd : fds_rw) + { + if (fd >= 0) + { + throw std::logic_error("Pipe is already opened"); + } + } + +#ifndef __APPLE__ + if (0 != pipe2(fds_rw, O_CLOEXEC)) + throw std::runtime_error("Cannot create pipe"); +#else + if (0 != pipe(fds_rw)) + throw std::runtime_error("Cannot create pipe"); + if (0 != fcntl(fds_rw[0], F_SETFD, FD_CLOEXEC)) + throw std::runtime_error("Cannot setup auto-close on exec for read end of pipe"); + if (0 != fcntl(fds_rw[1], F_SETFD, FD_CLOEXEC)) + throw std::runtime_error("Cannot setup auto-close on exec for write end of pipe"); +#endif +} + +void LazyPipe::close() +{ + for (int fd : fds_rw) + { + if (fd >= 0) + { + ::close(fd); + } + } +} + +Pipe::Pipe() +{ + open(); +} + +Pipe::~Pipe() +{ + close(); +} diff --git a/libs/libcommon/src/Sleep.cpp b/libs/libcommon/src/sleep.cpp similarity index 62% rename from libs/libcommon/src/Sleep.cpp rename to libs/libcommon/src/sleep.cpp index 13b03f2b95e..710b387d62e 100644 --- a/libs/libcommon/src/Sleep.cpp +++ b/libs/libcommon/src/sleep.cpp @@ -1,10 +1,10 @@ -#include "common/Sleep.h" +#include "common/sleep.h" #include #include /** - * Sleep with nanoseconds precision + * Sleep with nanoseconds precision. Tolerant to signal interruptions * * In case query profiler is turned on, all threads spawned for * query execution are repeatedly interrupted by signals from timer. @@ -12,14 +12,14 @@ * problems in this setup and man page for nanosleep(2) suggests * using absolute deadlines, for instance clock_nanosleep(2). */ -void SleepForNanoseconds(uint64_t nanoseconds) +void sleepForNanoseconds(uint64_t nanoseconds) { - const auto clock_type = CLOCK_REALTIME; + constexpr auto clock_type = CLOCK_MONOTONIC; struct timespec current_time; clock_gettime(clock_type, ¤t_time); - const uint64_t resolution = 1'000'000'000; + constexpr uint64_t resolution = 1'000'000'000; struct timespec finish_time = current_time; finish_time.tv_nsec += nanoseconds % resolution; @@ -31,17 +31,17 @@ void SleepForNanoseconds(uint64_t nanoseconds) while (clock_nanosleep(clock_type, TIMER_ABSTIME, &finish_time, nullptr) == EINTR); } -void SleepForMicroseconds(uint64_t microseconds) +void sleepForMicroseconds(uint64_t microseconds) { - SleepForNanoseconds(microseconds * 1000); + sleepForNanoseconds(microseconds * 1000); } -void SleepForMilliseconds(uint64_t milliseconds) +void sleepForMilliseconds(uint64_t milliseconds) { - SleepForMicroseconds(milliseconds * 1000); + sleepForMicroseconds(milliseconds * 1000); } -void SleepForSeconds(uint64_t seconds) +void sleepForSeconds(uint64_t seconds) { - SleepForMilliseconds(seconds * 1000); + sleepForMilliseconds(seconds * 1000); } diff --git a/libs/libmysqlxx/src/Pool.cpp b/libs/libmysqlxx/src/Pool.cpp index 6e35a311b44..a17246e5d6d 100644 --- a/libs/libmysqlxx/src/Pool.cpp +++ b/libs/libmysqlxx/src/Pool.cpp @@ -8,7 +8,7 @@ #include -#include +#include #include #include @@ -135,7 +135,7 @@ Pool::Entry Pool::Get() } lock.unlock(); - SleepForSeconds(MYSQLXX_POOL_SLEEP_ON_CONNECT_FAIL); + sleepForSeconds(MYSQLXX_POOL_SLEEP_ON_CONNECT_FAIL); lock.lock(); } } @@ -195,7 +195,7 @@ void Pool::Entry::forceConnected() const if (first) first = false; else - SleepForSeconds(MYSQLXX_POOL_SLEEP_ON_CONNECT_FAIL); + sleepForSeconds(MYSQLXX_POOL_SLEEP_ON_CONNECT_FAIL); app.logger().information("MYSQL: Reconnecting to " + pool->description); data->conn.connect( From 7382cb41fa6d03467eb548ff0f2a585ad965a6f0 Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Thu, 18 Jul 2019 21:29:49 +0300 Subject: [PATCH 39/57] CREATE TABLE AS table_function() --- dbms/src/Databases/DatabasesCommon.cpp | 8 ++++ .../ClusterProxy/SelectStreamFactory.cpp | 3 +- dbms/src/Interpreters/Context.cpp | 2 +- .../Interpreters/InterpreterCreateQuery.cpp | 37 ++++++++++++++----- .../Interpreters/InterpreterDescribeQuery.cpp | 2 +- .../Interpreters/InterpreterInsertQuery.cpp | 3 +- dbms/src/Parsers/ASTCreateQuery.cpp | 8 +++- dbms/src/Parsers/ASTCreateQuery.h | 1 + dbms/src/Parsers/ParserCreateQuery.cpp | 30 +++++++++------ .../Storages/getStructureOfRemoteTable.cpp | 3 +- dbms/src/TableFunctions/ITableFunction.cpp | 4 +- dbms/src/TableFunctions/ITableFunction.h | 4 +- .../TableFunctions/ITableFunctionFileLike.cpp | 4 +- .../TableFunctions/ITableFunctionFileLike.h | 4 +- .../src/TableFunctions/ITableFunctionXDBC.cpp | 8 ++-- dbms/src/TableFunctions/ITableFunctionXDBC.h | 2 +- dbms/src/TableFunctions/TableFunctionFile.cpp | 4 +- dbms/src/TableFunctions/TableFunctionFile.h | 2 +- dbms/src/TableFunctions/TableFunctionHDFS.cpp | 4 +- dbms/src/TableFunctions/TableFunctionHDFS.h | 2 +- .../src/TableFunctions/TableFunctionMerge.cpp | 4 +- dbms/src/TableFunctions/TableFunctionMerge.h | 2 +- .../src/TableFunctions/TableFunctionMySQL.cpp | 4 +- dbms/src/TableFunctions/TableFunctionMySQL.h | 2 +- .../TableFunctions/TableFunctionNumbers.cpp | 4 +- .../src/TableFunctions/TableFunctionNumbers.h | 2 +- .../TableFunctions/TableFunctionRemote.cpp | 6 +-- dbms/src/TableFunctions/TableFunctionRemote.h | 2 +- dbms/src/TableFunctions/TableFunctionURL.cpp | 4 +- dbms/src/TableFunctions/TableFunctionURL.h | 2 +- 30 files changed, 105 insertions(+), 62 deletions(-) diff --git a/dbms/src/Databases/DatabasesCommon.cpp b/dbms/src/Databases/DatabasesCommon.cpp index 3d2ca472137..27d236678f1 100644 --- a/dbms/src/Databases/DatabasesCommon.cpp +++ b/dbms/src/Databases/DatabasesCommon.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -68,6 +69,13 @@ std::pair createTableFromDefinition( ast_create_query.attach = true; ast_create_query.database = database_name; + if (ast_create_query.as_table_function) + { + const auto * table_function = ast_create_query.as_table_function->as(); + const auto & factory = TableFunctionFactory::instance(); + StoragePtr storage = factory.get(table_function->name, context)->execute(ast_create_query.as_table_function, context, ast_create_query.table); + return {ast_create_query.table, storage}; + } /// We do not directly use `InterpreterCreateQuery::execute`, because /// - the database has not been created yet; /// - the code is simpler, since the query is already brought to a suitable form. diff --git a/dbms/src/Interpreters/ClusterProxy/SelectStreamFactory.cpp b/dbms/src/Interpreters/ClusterProxy/SelectStreamFactory.cpp index ce16a431b37..ba0571d1863 100644 --- a/dbms/src/Interpreters/ClusterProxy/SelectStreamFactory.cpp +++ b/dbms/src/Interpreters/ClusterProxy/SelectStreamFactory.cpp @@ -99,7 +99,8 @@ void SelectStreamFactory::createForShard( if (table_func_ptr) { const auto * table_function = table_func_ptr->as(); - main_table_storage = TableFunctionFactory::instance().get(table_function->name, context)->execute(table_func_ptr, context); + TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().get(table_function->name, context); + main_table_storage = table_function_ptr->execute(table_func_ptr, context, table_function_ptr->getName()); } else main_table_storage = context.tryGetTable(main_table.database, main_table.table); diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 2909de8a60b..7b1d138ce3d 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -963,7 +963,7 @@ StoragePtr Context::executeTableFunction(const ASTPtr & table_expression) TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().get(table_expression->as()->name, *this); /// Run it and remember the result - res = table_function_ptr->execute(table_expression, *this); + res = table_function_ptr->execute(table_expression, *this, table_function_ptr->getName()); } return res; diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 7853e0c0841..7fe907257f7 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -46,6 +46,8 @@ #include #include +#include + namespace DB { @@ -389,6 +391,11 @@ ColumnsDescription InterpreterCreateQuery::setColumns( columns = as_storage->getColumns(); indices = as_storage->getIndices(); } + else if (create.as_table_function) + { + columns = as_storage->getColumns(); + indices = as_storage->getIndices(); + } else if (create.select) { columns = ColumnsDescription(as_select_sample.getNamesAndTypesList()); @@ -518,6 +525,13 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) StoragePtr as_storage; TableStructureReadLockHolder as_storage_lock; + + if (create.as_table_function) + { + const auto * table_function = create.as_table_function->as(); + const auto & factory = TableFunctionFactory::instance(); + as_storage = factory.get(table_function->name, context)->execute(create.as_table_function, context, create.table); + } if (!as_table_name.empty()) { as_storage = context.getTable(as_database_name, as_table_name); @@ -585,15 +599,20 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) else if (context.tryGetExternalTable(table_name) && create.if_not_exists) return {}; - res = StorageFactory::instance().get(create, - data_path, - table_name, - database_name, - context, - context.getGlobalContext(), - columns, - create.attach, - false); + if (create.as_table_function) + res = as_storage; + else + { + res = StorageFactory::instance().get(create, + data_path, + table_name, + database_name, + context, + context.getGlobalContext(), + columns, + create.attach, + false); + } if (create.temporary) context.getSessionContext().addExternalTable(table_name, res, query_ptr); diff --git a/dbms/src/Interpreters/InterpreterDescribeQuery.cpp b/dbms/src/Interpreters/InterpreterDescribeQuery.cpp index d037f87a857..cc3fd51ac50 100644 --- a/dbms/src/Interpreters/InterpreterDescribeQuery.cpp +++ b/dbms/src/Interpreters/InterpreterDescribeQuery.cpp @@ -79,7 +79,7 @@ BlockInputStreamPtr InterpreterDescribeQuery::executeImpl() const auto & table_function = table_expression.table_function->as(); TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().get(table_function.name, context); /// Run the table function and remember the result - table = table_function_ptr->execute(table_expression.table_function, context); + table = table_function_ptr->execute(table_expression.table_function, context, table_function_ptr->getName()); } else { diff --git a/dbms/src/Interpreters/InterpreterInsertQuery.cpp b/dbms/src/Interpreters/InterpreterInsertQuery.cpp index b906d151415..dbb90028316 100644 --- a/dbms/src/Interpreters/InterpreterInsertQuery.cpp +++ b/dbms/src/Interpreters/InterpreterInsertQuery.cpp @@ -48,7 +48,8 @@ StoragePtr InterpreterInsertQuery::getTable(const ASTInsertQuery & query) { const auto * table_function = query.table_function->as(); const auto & factory = TableFunctionFactory::instance(); - return factory.get(table_function->name, context)->execute(query.table_function, context); + TableFunctionPtr table_function_ptr = factory.get(table_function->name, context); + return table_function_ptr->execute(query.table_function, context, table_function_ptr->getName()); } /// Into what table to write. diff --git a/dbms/src/Parsers/ASTCreateQuery.cpp b/dbms/src/Parsers/ASTCreateQuery.cpp index e99c543f5ec..4c84383af8a 100644 --- a/dbms/src/Parsers/ASTCreateQuery.cpp +++ b/dbms/src/Parsers/ASTCreateQuery.cpp @@ -216,7 +216,11 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat << (!database.empty() ? backQuoteIfNeed(database) + "." : "") << backQuoteIfNeed(table); formatOnCluster(settings); } - + if (as_table_function) + { + settings.ostr << (settings.hilite ? hilite_keyword : "") << " AS " << (settings.hilite ? hilite_none : ""); + as_table_function->formatImpl(settings, state, frame); + } if (!to_table.empty()) { settings.ostr @@ -231,7 +235,7 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat << (!as_database.empty() ? backQuoteIfNeed(as_database) + "." : "") << backQuoteIfNeed(as_table); } - if (columns_list) + if (columns_list && !as_table_function) { settings.ostr << (settings.one_line ? " (" : "\n("); FormatStateStacked frame_nested = frame; diff --git a/dbms/src/Parsers/ASTCreateQuery.h b/dbms/src/Parsers/ASTCreateQuery.h index 2755e1a3d78..7e121e4d2d9 100644 --- a/dbms/src/Parsers/ASTCreateQuery.h +++ b/dbms/src/Parsers/ASTCreateQuery.h @@ -63,6 +63,7 @@ public: ASTStorage * storage = nullptr; String as_database; String as_table; + ASTPtr as_table_function; ASTSelectWithUnionQuery * select = nullptr; /** Get the text that identifies this element. */ diff --git a/dbms/src/Parsers/ParserCreateQuery.cpp b/dbms/src/Parsers/ParserCreateQuery.cpp index fd6665a5a2c..be0779c4d52 100644 --- a/dbms/src/Parsers/ParserCreateQuery.cpp +++ b/dbms/src/Parsers/ParserCreateQuery.cpp @@ -319,6 +319,7 @@ bool ParserCreateQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) ParserIdentifier name_p; ParserColumnsOrIndicesDeclarationList columns_or_indices_p; ParserSelectWithUnionQuery select_p; + ParserFunction table_function_p; ASTPtr database; ASTPtr table; @@ -328,6 +329,7 @@ bool ParserCreateQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) ASTPtr storage; ASTPtr as_database; ASTPtr as_table; + ASTPtr as_table_function; ASTPtr select; String cluster_str; bool attach = false; @@ -407,22 +409,25 @@ bool ParserCreateQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) if (!s_as.ignore(pos, expected)) return false; - if (!select_p.parse(pos, select, expected)) /// AS SELECT ... + if (!table_function_p.parse(pos, as_table_function, expected)) { - /// AS [db.]table - if (!name_p.parse(pos, as_table, expected)) - return false; - - if (s_dot.ignore(pos, expected)) + if (!select_p.parse(pos, select, expected)) /// AS SELECT ... { - as_database = as_table; + /// AS [db.]table if (!name_p.parse(pos, as_table, expected)) return false; - } - /// Optional - ENGINE can be specified. - if (!storage) - storage_p.parse(pos, storage, expected); + if (s_dot.ignore(pos, expected)) + { + as_database = as_table; + if (!name_p.parse(pos, as_table, expected)) + return false; + } + + /// Optional - ENGINE can be specified. + if (!storage) + storage_p.parse(pos, storage, expected); + } } } } @@ -526,6 +531,9 @@ bool ParserCreateQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) auto query = std::make_shared(); node = query; + if (as_table_function) + query->as_table_function = as_table_function; + query->attach = attach; query->if_not_exists = if_not_exists; query->is_view = is_view; diff --git a/dbms/src/Storages/getStructureOfRemoteTable.cpp b/dbms/src/Storages/getStructureOfRemoteTable.cpp index 588d6ecc8c5..137abcea649 100644 --- a/dbms/src/Storages/getStructureOfRemoteTable.cpp +++ b/dbms/src/Storages/getStructureOfRemoteTable.cpp @@ -40,7 +40,8 @@ ColumnsDescription getStructureOfRemoteTable( if (shard_info.isLocal()) { const auto * table_function = table_func_ptr->as(); - return TableFunctionFactory::instance().get(table_function->name, context)->execute(table_func_ptr, context)->getColumns(); + TableFunctionPtr table_function_ptr = TableFunctionFactory::instance().get(table_function->name, context); + return table_function_ptr->execute(table_func_ptr, context, table_function_ptr->getName())->getColumns(); } auto table_func_name = queryToString(table_func_ptr); diff --git a/dbms/src/TableFunctions/ITableFunction.cpp b/dbms/src/TableFunctions/ITableFunction.cpp index b15cbbc9fd9..233da7495d8 100644 --- a/dbms/src/TableFunctions/ITableFunction.cpp +++ b/dbms/src/TableFunctions/ITableFunction.cpp @@ -10,10 +10,10 @@ namespace ProfileEvents namespace DB { -StoragePtr ITableFunction::execute(const ASTPtr & ast_function, const Context & context) const +StoragePtr ITableFunction::execute(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { ProfileEvents::increment(ProfileEvents::TableFunctionExecute); - return executeImpl(ast_function, context); + return executeImpl(ast_function, context, table_name); } } diff --git a/dbms/src/TableFunctions/ITableFunction.h b/dbms/src/TableFunctions/ITableFunction.h index 026ff944976..81463f7baa8 100644 --- a/dbms/src/TableFunctions/ITableFunction.h +++ b/dbms/src/TableFunctions/ITableFunction.h @@ -30,12 +30,12 @@ public: virtual std::string getName() const = 0; /// Create storage according to the query. - StoragePtr execute(const ASTPtr & ast_function, const Context & context) const; + StoragePtr execute(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const; virtual ~ITableFunction() {} private: - virtual StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const = 0; + virtual StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const = 0; }; using TableFunctionPtr = std::shared_ptr; diff --git a/dbms/src/TableFunctions/ITableFunctionFileLike.cpp b/dbms/src/TableFunctions/ITableFunctionFileLike.cpp index fe8af831b56..7201af6ca06 100644 --- a/dbms/src/TableFunctions/ITableFunctionFileLike.cpp +++ b/dbms/src/TableFunctions/ITableFunctionFileLike.cpp @@ -19,7 +19,7 @@ namespace ErrorCodes extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH; } -StoragePtr ITableFunctionFileLike::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr ITableFunctionFileLike::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { // Parse args ASTs & args_func = ast_function->children; @@ -60,7 +60,7 @@ StoragePtr ITableFunctionFileLike::executeImpl(const ASTPtr & ast_function, cons } // Create table - StoragePtr storage = getStorage(filename, format, sample_block, const_cast(context)); + StoragePtr storage = getStorage(filename, format, sample_block, const_cast(context), table_name); storage->startup(); diff --git a/dbms/src/TableFunctions/ITableFunctionFileLike.h b/dbms/src/TableFunctions/ITableFunctionFileLike.h index 70637946808..26e046f0b41 100644 --- a/dbms/src/TableFunctions/ITableFunctionFileLike.h +++ b/dbms/src/TableFunctions/ITableFunctionFileLike.h @@ -12,8 +12,8 @@ namespace DB class ITableFunctionFileLike : public ITableFunction { private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; virtual StoragePtr getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const = 0; + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const = 0; }; } diff --git a/dbms/src/TableFunctions/ITableFunctionXDBC.cpp b/dbms/src/TableFunctions/ITableFunctionXDBC.cpp index 32011dc8f8a..b9b767f2d9e 100644 --- a/dbms/src/TableFunctions/ITableFunctionXDBC.cpp +++ b/dbms/src/TableFunctions/ITableFunctionXDBC.cpp @@ -27,7 +27,7 @@ namespace ErrorCodes extern const int LOGICAL_ERROR; } -StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { const auto & args_func = ast_function->as(); @@ -45,18 +45,18 @@ StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Co std::string connection_string; std::string schema_name; - std::string table_name; + //std::string table_name; if (args.size() == 3) { connection_string = args[0]->as().value.safeGet(); schema_name = args[1]->as().value.safeGet(); - table_name = args[2]->as().value.safeGet(); + //table_name = args[2]->as().value.safeGet(); } else if (args.size() == 2) { connection_string = args[0]->as().value.safeGet(); - table_name = args[1]->as().value.safeGet(); + //table_name = args[1]->as().value.safeGet(); } /* Infer external table structure */ diff --git a/dbms/src/TableFunctions/ITableFunctionXDBC.h b/dbms/src/TableFunctions/ITableFunctionXDBC.h index 8676b85debf..b585d8228dc 100644 --- a/dbms/src/TableFunctions/ITableFunctionXDBC.h +++ b/dbms/src/TableFunctions/ITableFunctionXDBC.h @@ -15,7 +15,7 @@ namespace DB class ITableFunctionXDBC : public ITableFunction { private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; /* A factory method to create bridge helper, that will assist in remote interaction */ virtual BridgeHelperPtr createBridgeHelper(Context & context, diff --git a/dbms/src/TableFunctions/TableFunctionFile.cpp b/dbms/src/TableFunctions/TableFunctionFile.cpp index 89531096d35..42ad309ffb4 100644 --- a/dbms/src/TableFunctions/TableFunctionFile.cpp +++ b/dbms/src/TableFunctions/TableFunctionFile.cpp @@ -5,12 +5,12 @@ namespace DB { StoragePtr TableFunctionFile::getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const { return StorageFile::create(source, -1, global_context.getUserFilesPath(), - getName(), + table_name, format, ColumnsDescription{sample_block.getNamesAndTypesList()}, global_context); diff --git a/dbms/src/TableFunctions/TableFunctionFile.h b/dbms/src/TableFunctions/TableFunctionFile.h index 56cd5002ba1..2a508d06339 100644 --- a/dbms/src/TableFunctions/TableFunctionFile.h +++ b/dbms/src/TableFunctions/TableFunctionFile.h @@ -24,6 +24,6 @@ public: private: StoragePtr getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const override; + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const override; }; } diff --git a/dbms/src/TableFunctions/TableFunctionHDFS.cpp b/dbms/src/TableFunctions/TableFunctionHDFS.cpp index 9c09ad9313c..5c1fc26b942 100644 --- a/dbms/src/TableFunctions/TableFunctionHDFS.cpp +++ b/dbms/src/TableFunctions/TableFunctionHDFS.cpp @@ -8,10 +8,10 @@ namespace DB { StoragePtr TableFunctionHDFS::getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const { return StorageHDFS::create(source, - getName(), + table_name, format, ColumnsDescription{sample_block.getNamesAndTypesList()}, global_context); diff --git a/dbms/src/TableFunctions/TableFunctionHDFS.h b/dbms/src/TableFunctions/TableFunctionHDFS.h index 8033034deb8..9dafadc8df8 100644 --- a/dbms/src/TableFunctions/TableFunctionHDFS.h +++ b/dbms/src/TableFunctions/TableFunctionHDFS.h @@ -25,7 +25,7 @@ public: private: StoragePtr getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const override; + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const override; }; } diff --git a/dbms/src/TableFunctions/TableFunctionMerge.cpp b/dbms/src/TableFunctions/TableFunctionMerge.cpp index cd8d906b94d..f4e947a2835 100644 --- a/dbms/src/TableFunctions/TableFunctionMerge.cpp +++ b/dbms/src/TableFunctions/TableFunctionMerge.cpp @@ -47,7 +47,7 @@ static NamesAndTypesList chooseColumns(const String & source_database, const Str } -StoragePtr TableFunctionMerge::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr TableFunctionMerge::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { ASTs & args_func = ast_function->children; @@ -70,7 +70,7 @@ StoragePtr TableFunctionMerge::executeImpl(const ASTPtr & ast_function, const Co String table_name_regexp = args[1]->as().value.safeGet(); auto res = StorageMerge::create( - getName(), + table_name, ColumnsDescription{chooseColumns(source_database, table_name_regexp, context)}, source_database, table_name_regexp, diff --git a/dbms/src/TableFunctions/TableFunctionMerge.h b/dbms/src/TableFunctions/TableFunctionMerge.h index 2fb512ac590..43d4b692bc8 100644 --- a/dbms/src/TableFunctions/TableFunctionMerge.h +++ b/dbms/src/TableFunctions/TableFunctionMerge.h @@ -16,7 +16,7 @@ public: static constexpr auto name = "merge"; std::string getName() const override { return name; } private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; }; diff --git a/dbms/src/TableFunctions/TableFunctionMySQL.cpp b/dbms/src/TableFunctions/TableFunctionMySQL.cpp index 71d195e95ed..51b0a144dcd 100644 --- a/dbms/src/TableFunctions/TableFunctionMySQL.cpp +++ b/dbms/src/TableFunctions/TableFunctionMySQL.cpp @@ -37,7 +37,7 @@ namespace ErrorCodes } -StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { const auto & args_func = ast_function->as(); @@ -55,7 +55,7 @@ StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Co std::string host_port = args[0]->as().value.safeGet(); std::string database_name = args[1]->as().value.safeGet(); - std::string table_name = args[2]->as().value.safeGet(); + // std::string table_name = args[2]->as().value.safeGet(); std::string user_name = args[3]->as().value.safeGet(); std::string password = args[4]->as().value.safeGet(); diff --git a/dbms/src/TableFunctions/TableFunctionMySQL.h b/dbms/src/TableFunctions/TableFunctionMySQL.h index 870b3f75624..fd5b0219df6 100644 --- a/dbms/src/TableFunctions/TableFunctionMySQL.h +++ b/dbms/src/TableFunctions/TableFunctionMySQL.h @@ -19,7 +19,7 @@ public: return name; } private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; }; } diff --git a/dbms/src/TableFunctions/TableFunctionNumbers.cpp b/dbms/src/TableFunctions/TableFunctionNumbers.cpp index a02cd904882..94c618bd9b1 100644 --- a/dbms/src/TableFunctions/TableFunctionNumbers.cpp +++ b/dbms/src/TableFunctions/TableFunctionNumbers.cpp @@ -17,7 +17,7 @@ namespace ErrorCodes } -StoragePtr TableFunctionNumbers::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr TableFunctionNumbers::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { if (const auto * function = ast_function->as()) { @@ -30,7 +30,7 @@ StoragePtr TableFunctionNumbers::executeImpl(const ASTPtr & ast_function, const UInt64 offset = arguments.size() == 2 ? evaluateArgument(context, arguments[0]) : 0; UInt64 length = arguments.size() == 2 ? evaluateArgument(context, arguments[1]) : evaluateArgument(context, arguments[0]); - auto res = StorageSystemNumbers::create(getName(), false, length, offset); + auto res = StorageSystemNumbers::create(table_name, false, length, offset); res->startup(); return res; } diff --git a/dbms/src/TableFunctions/TableFunctionNumbers.h b/dbms/src/TableFunctions/TableFunctionNumbers.h index ed060a6450a..733b4508f51 100644 --- a/dbms/src/TableFunctions/TableFunctionNumbers.h +++ b/dbms/src/TableFunctions/TableFunctionNumbers.h @@ -17,7 +17,7 @@ public: static constexpr auto name = "numbers"; std::string getName() const override { return name; } private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; UInt64 evaluateArgument(const Context & context, ASTPtr & argument) const; }; diff --git a/dbms/src/TableFunctions/TableFunctionRemote.cpp b/dbms/src/TableFunctions/TableFunctionRemote.cpp index 21611500eb7..c4e0cd1866a 100644 --- a/dbms/src/TableFunctions/TableFunctionRemote.cpp +++ b/dbms/src/TableFunctions/TableFunctionRemote.cpp @@ -25,7 +25,7 @@ namespace ErrorCodes extern const int BAD_ARGUMENTS; } -StoragePtr TableFunctionRemote::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr TableFunctionRemote::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { ASTs & args_func = ast_function->children; @@ -162,13 +162,13 @@ StoragePtr TableFunctionRemote::executeImpl(const ASTPtr & ast_function, const C StoragePtr res = remote_table_function_ptr ? StorageDistributed::createWithOwnCluster( - getName(), + table_name, structure_remote_table, remote_table_function_ptr, cluster, context) : StorageDistributed::createWithOwnCluster( - getName(), + table_name, structure_remote_table, remote_database, remote_table, diff --git a/dbms/src/TableFunctions/TableFunctionRemote.h b/dbms/src/TableFunctions/TableFunctionRemote.h index dcd0699b064..c9a98cbbc16 100644 --- a/dbms/src/TableFunctions/TableFunctionRemote.h +++ b/dbms/src/TableFunctions/TableFunctionRemote.h @@ -21,7 +21,7 @@ public: std::string getName() const override { return name; } private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; std::string name; bool is_cluster_function; diff --git a/dbms/src/TableFunctions/TableFunctionURL.cpp b/dbms/src/TableFunctions/TableFunctionURL.cpp index f33e5a92cb3..22ec78648eb 100644 --- a/dbms/src/TableFunctions/TableFunctionURL.cpp +++ b/dbms/src/TableFunctions/TableFunctionURL.cpp @@ -6,10 +6,10 @@ namespace DB { StoragePtr TableFunctionURL::getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const { Poco::URI uri(source); - return StorageURL::create(uri, getName(), format, ColumnsDescription{sample_block.getNamesAndTypesList()}, global_context); + return StorageURL::create(uri, table_name, format, ColumnsDescription{sample_block.getNamesAndTypesList()}, global_context); } void registerTableFunctionURL(TableFunctionFactory & factory) diff --git a/dbms/src/TableFunctions/TableFunctionURL.h b/dbms/src/TableFunctions/TableFunctionURL.h index 6382beee836..f7b723a422c 100644 --- a/dbms/src/TableFunctions/TableFunctionURL.h +++ b/dbms/src/TableFunctions/TableFunctionURL.h @@ -20,6 +20,6 @@ public: private: StoragePtr getStorage( - const String & source, const String & format, const Block & sample_block, Context & global_context) const override; + const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const override; }; } From 1ab08934322f58272466fdfaa91c558ded39d0e3 Mon Sep 17 00:00:00 2001 From: dimarub2000 Date: Thu, 18 Jul 2019 21:59:31 +0300 Subject: [PATCH 40/57] minor fixes --- dbms/src/TableFunctions/TableFunctionFile.cpp | 1 + dbms/src/TableFunctions/TableFunctionHDFS.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/dbms/src/TableFunctions/TableFunctionFile.cpp b/dbms/src/TableFunctions/TableFunctionFile.cpp index 42ad309ffb4..8b2c3313cb8 100644 --- a/dbms/src/TableFunctions/TableFunctionFile.cpp +++ b/dbms/src/TableFunctions/TableFunctionFile.cpp @@ -10,6 +10,7 @@ StoragePtr TableFunctionFile::getStorage( return StorageFile::create(source, -1, global_context.getUserFilesPath(), + getDatabaseName(), table_name, format, ColumnsDescription{sample_block.getNamesAndTypesList()}, diff --git a/dbms/src/TableFunctions/TableFunctionHDFS.cpp b/dbms/src/TableFunctions/TableFunctionHDFS.cpp index 5c1fc26b942..0cdf0a08b26 100644 --- a/dbms/src/TableFunctions/TableFunctionHDFS.cpp +++ b/dbms/src/TableFunctions/TableFunctionHDFS.cpp @@ -11,6 +11,7 @@ StoragePtr TableFunctionHDFS::getStorage( const String & source, const String & format, const Block & sample_block, Context & global_context, const std::string & table_name) const { return StorageHDFS::create(source, + getDatabaseName(), table_name, format, ColumnsDescription{sample_block.getNamesAndTypesList()}, From c9ec16987e2b069d57839000125d0af0b5b8de8d Mon Sep 17 00:00:00 2001 From: dimarub2000 Date: Thu, 18 Jul 2019 22:09:45 +0300 Subject: [PATCH 41/57] minor fixes --- contrib/simdjson | 2 +- dbms/src/TableFunctions/TableFunctionCatBoostPool.cpp | 4 ++-- dbms/src/TableFunctions/TableFunctionCatBoostPool.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/simdjson b/contrib/simdjson index 3bd3116cf8f..2151ad7f34c 160000 --- a/contrib/simdjson +++ b/contrib/simdjson @@ -1 +1 @@ -Subproject commit 3bd3116cf8faf6d482dc31423b16533bfa2696f7 +Subproject commit 2151ad7f34cf773a23f086e941d661f8a8873144 diff --git a/dbms/src/TableFunctions/TableFunctionCatBoostPool.cpp b/dbms/src/TableFunctions/TableFunctionCatBoostPool.cpp index da926dcc906..74fab72fd19 100644 --- a/dbms/src/TableFunctions/TableFunctionCatBoostPool.cpp +++ b/dbms/src/TableFunctions/TableFunctionCatBoostPool.cpp @@ -16,7 +16,7 @@ namespace ErrorCodes } -StoragePtr TableFunctionCatBoostPool::executeImpl(const ASTPtr & ast_function, const Context & context) const +StoragePtr TableFunctionCatBoostPool::executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const { ASTs & args_func = ast_function->children; @@ -45,7 +45,7 @@ StoragePtr TableFunctionCatBoostPool::executeImpl(const ASTPtr & ast_function, c String column_descriptions_file = getStringLiteral(*args[0], "Column descriptions file"); String dataset_description_file = getStringLiteral(*args[1], "Dataset description file"); - return StorageCatBoostPool::create(getDatabaseName(), getName(), context, column_descriptions_file, dataset_description_file); + return StorageCatBoostPool::create(getDatabaseName(), table_name, context, column_descriptions_file, dataset_description_file); } void registerTableFunctionCatBoostPool(TableFunctionFactory & factory) diff --git a/dbms/src/TableFunctions/TableFunctionCatBoostPool.h b/dbms/src/TableFunctions/TableFunctionCatBoostPool.h index 061b5a735f6..cf93308b60c 100644 --- a/dbms/src/TableFunctions/TableFunctionCatBoostPool.h +++ b/dbms/src/TableFunctions/TableFunctionCatBoostPool.h @@ -15,7 +15,7 @@ public: static constexpr auto name = "catBoostPool"; std::string getName() const override { return name; } private: - StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context) const override; + StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name) const override; }; } From d1e6e6ed4b291be2805aef3b49ebb49c8d9e4bb0 Mon Sep 17 00:00:00 2001 From: dimarub2000 Date: Thu, 18 Jul 2019 22:17:02 +0300 Subject: [PATCH 42/57] fix json --- contrib/simdjson | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/simdjson b/contrib/simdjson index 2151ad7f34c..3bd3116cf8f 160000 --- a/contrib/simdjson +++ b/contrib/simdjson @@ -1 +1 @@ -Subproject commit 2151ad7f34cf773a23f086e941d661f8a8873144 +Subproject commit 3bd3116cf8faf6d482dc31423b16533bfa2696f7 From 1aca3da1211f8c3fb5362c5d5a2e98515c10c39d Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Fri, 19 Jul 2019 16:28:28 +0300 Subject: [PATCH 43/57] Fixed inegration --- .../Interpreters/InterpreterCreateQuery.cpp | 7 +------ .../src/TableFunctions/ITableFunctionXDBC.cpp | 13 +++++++----- .../src/TableFunctions/TableFunctionMySQL.cpp | 21 +++++++++++-------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 48bb116736e..a862c7de5c1 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -386,12 +386,7 @@ ColumnsDescription InterpreterCreateQuery::setColumns( indices.indices.push_back( std::dynamic_pointer_cast(index->clone())); } - else if (!create.as_table.empty()) - { - columns = as_storage->getColumns(); - indices = as_storage->getIndices(); - } - else if (create.as_table_function) + else if (!create.as_table.empty() || create.as_table_function) { columns = as_storage->getColumns(); indices = as_storage->getIndices(); diff --git a/dbms/src/TableFunctions/ITableFunctionXDBC.cpp b/dbms/src/TableFunctions/ITableFunctionXDBC.cpp index 8ce56568a1e..9998eb87fcf 100644 --- a/dbms/src/TableFunctions/ITableFunctionXDBC.cpp +++ b/dbms/src/TableFunctions/ITableFunctionXDBC.cpp @@ -45,18 +45,18 @@ StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Co std::string connection_string; std::string schema_name; - //std::string table_name; + std::string remote_table_name; if (args.size() == 3) { connection_string = args[0]->as().value.safeGet(); schema_name = args[1]->as().value.safeGet(); - //table_name = args[2]->as().value.safeGet(); + remote_table_name = args[2]->as().value.safeGet(); } else if (args.size() == 2) { connection_string = args[0]->as().value.safeGet(); - //table_name = args[1]->as().value.safeGet(); + remote_table_name = args[1]->as().value.safeGet(); } /* Infer external table structure */ @@ -68,7 +68,7 @@ StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Co columns_info_uri.addQueryParameter("connection_string", connection_string); if (!schema_name.empty()) columns_info_uri.addQueryParameter("schema", schema_name); - columns_info_uri.addQueryParameter("table", table_name); + columns_info_uri.addQueryParameter("table", remote_table_name); ReadWriteBufferFromHTTP buf(columns_info_uri, Poco::Net::HTTPRequest::HTTP_POST, nullptr); @@ -76,7 +76,10 @@ StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Co readStringBinary(columns_info, buf); NamesAndTypesList columns = NamesAndTypesList::parse(columns_info); - auto result = std::make_shared(getDatabaseName(), table_name, schema_name, table_name, ColumnsDescription{columns}, context, helper); + ///If table_name was not specified by user, it will have the same name as remote_table_name + std::string local_table_name = (table_name == getName()) ? remote_table_name : table_name; + + auto result = std::make_shared(getDatabaseName(), local_table_name, schema_name, remote_table_name, ColumnsDescription{columns}, context, helper); if (!result) throw Exception("Failed to instantiate storage from table function " + getName(), ErrorCodes::UNKNOWN_EXCEPTION); diff --git a/dbms/src/TableFunctions/TableFunctionMySQL.cpp b/dbms/src/TableFunctions/TableFunctionMySQL.cpp index 6775363e02c..ac8d2671878 100644 --- a/dbms/src/TableFunctions/TableFunctionMySQL.cpp +++ b/dbms/src/TableFunctions/TableFunctionMySQL.cpp @@ -54,8 +54,8 @@ StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Co args[i] = evaluateConstantExpressionOrIdentifierAsLiteral(args[i], context); std::string host_port = args[0]->as().value.safeGet(); - std::string database_name = args[1]->as().value.safeGet(); - // std::string table_name = args[2]->as().value.safeGet(); + std::string remote_database_name = args[1]->as().value.safeGet(); + std::string remote_table_name = args[2]->as().value.safeGet(); std::string user_name = args[3]->as().value.safeGet(); std::string password = args[4]->as().value.safeGet(); @@ -74,7 +74,7 @@ StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Co /// 3306 is the default MySQL port number auto parsed_host_port = parseAddress(host_port, 3306); - mysqlxx::Pool pool(database_name, parsed_host_port.first, user_name, password, parsed_host_port.second); + mysqlxx::Pool pool(remote_database_name, parsed_host_port.first, user_name, password, parsed_host_port.second); /// Determine table definition by running a query to INFORMATION_SCHEMA. @@ -95,8 +95,8 @@ StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Co " COLUMN_TYPE LIKE '%unsigned' AS is_unsigned," " CHARACTER_MAXIMUM_LENGTH AS length" " FROM INFORMATION_SCHEMA.COLUMNS" - " WHERE TABLE_SCHEMA = " << quote << database_name - << " AND TABLE_NAME = " << quote << table_name + " WHERE TABLE_SCHEMA = " << quote << remote_database_name + << " AND TABLE_NAME = " << quote << remote_table_name << " ORDER BY ORDINAL_POSITION"; NamesAndTypesList columns; @@ -116,14 +116,17 @@ StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Co } if (columns.empty()) - throw Exception("MySQL table " + backQuoteIfNeed(database_name) + "." + backQuoteIfNeed(table_name) + " doesn't exist.", ErrorCodes::UNKNOWN_TABLE); + throw Exception("MySQL table " + backQuoteIfNeed(remote_database_name) + "." + backQuoteIfNeed(remote_table_name) + " doesn't exist.", ErrorCodes::UNKNOWN_TABLE); + + ///If table_name was not specified by user, it will have the same name as remote_table_name + std::string local_table_name = (table_name == getName()) ? remote_table_name : table_name; auto res = StorageMySQL::create( getDatabaseName(), - table_name, + local_table_name, std::move(pool), - database_name, - table_name, + remote_database_name, + remote_table_name, replace_query, on_duplicate_clause, ColumnsDescription{columns}, From 6ba4408741cb3c4b86e0afa725c14223a0b09082 Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Fri, 19 Jul 2019 16:30:22 +0300 Subject: [PATCH 44/57] Tests added --- ...3_create_table_as_table_function.reference | 26 +++++++++++++++++++ .../00973_create_table_as_table_function.sh | 26 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference create mode 100755 dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh diff --git a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference new file mode 100644 index 00000000000..830a01e609a --- /dev/null +++ b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference @@ -0,0 +1,26 @@ +1 +0 +1 +2 +3 +4 +5 +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +19 +20 +21 +22 +23 +24 +25 diff --git a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh new file mode 100755 index 00000000000..aff96a364d1 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. $CURDIR/../shell_config.sh + +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t1" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t2" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t3" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t4" + +$CLICKHOUSE_CLIENT -q "CREATE TABLE t1 AS remote('127.0.0.1', system.one)" +$CLICKHOUSE_CLIENT -q "SELECT count() FROM t1" + +$CLICKHOUSE_CLIENT -q "CREATE TABLE t2 AS remote('127.0.0.1', system.numbers)" +$CLICKHOUSE_CLIENT -q "SELECT * FROM t2 LIMIT 18" + +$CLICKHOUSE_CLIENT -q "CREATE TABLE t3 AS remote('127.0.0.1', numbers(100))" +$CLICKHOUSE_CLIENT -q "SELECT * FROM t3 where number > 18 and number < 25" + +$CLICKHOUSE_CLIENT -q "CREATE TABLE t4 AS numbers(100)" +$CLICKHOUSE_CLIENT -q "SELECT count() FROM t4 where number > 74" + +$CLICKHOUSE_CLIENT -q "DROP TABLE t1" +$CLICKHOUSE_CLIENT -q "DROP TABLE t2" +$CLICKHOUSE_CLIENT -q "DROP TABLE t3" +$CLICKHOUSE_CLIENT -q "DROP TABLE t4" From f9945494d9d000d81edc972cdb911ed299dd3b7a Mon Sep 17 00:00:00 2001 From: Ivan Lezhankin Date: Fri, 19 Jul 2019 18:01:34 +0300 Subject: [PATCH 45/57] Always resume consumer before subscription. Also add more logs to see the difference between rd_kafka_assignment() vs rd_kafka_subscription() --- .../Kafka/ReadBufferFromKafkaConsumer.cpp | 16 ++++++++++++++++ .../Storages/Kafka/ReadBufferFromKafkaConsumer.h | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.cpp b/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.cpp index fc81e38bb63..5c7a8222a69 100644 --- a/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.cpp +++ b/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.cpp @@ -55,6 +55,22 @@ void ReadBufferFromKafkaConsumer::commit() void ReadBufferFromKafkaConsumer::subscribe(const Names & topics) { + { + String message = "Subscribed to topics:"; + for (const auto & topic : consumer->get_subscription()) + message += " " + topic; + LOG_TRACE(log, message); + } + + { + String message = "Assigned to topics:"; + for (const auto & toppar : consumer->get_assignment()) + message += " " + toppar.get_topic(); + LOG_TRACE(log, message); + } + + consumer->resume(); + // While we wait for an assignment after subscribtion, we'll poll zero messages anyway. // If we're doing a manual select then it's better to get something after a wait, then immediate nothing. if (consumer->get_subscription().empty()) diff --git a/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.h b/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.h index ac6011cfed0..2400357d3c3 100644 --- a/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.h +++ b/dbms/src/Storages/Kafka/ReadBufferFromKafkaConsumer.h @@ -24,7 +24,7 @@ public: void subscribe(const Names & topics); // Subscribe internal consumer to topics. void unsubscribe(); // Unsubscribe internal consumer in case of failure. - auto pollTimeout() { return poll_timeout; } + auto pollTimeout() const { return poll_timeout; } // Return values for the message that's being read. String currentTopic() const { return current[-1].get_topic(); } From 3abf1b278e6b7512e421d315478d262823b09e7d Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sat, 20 Jul 2019 02:51:20 +0300 Subject: [PATCH 46/57] Update DatabasesCommon.cpp --- dbms/src/Databases/DatabasesCommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Databases/DatabasesCommon.cpp b/dbms/src/Databases/DatabasesCommon.cpp index 27d236678f1..de3396b4603 100644 --- a/dbms/src/Databases/DatabasesCommon.cpp +++ b/dbms/src/Databases/DatabasesCommon.cpp @@ -71,7 +71,7 @@ std::pair createTableFromDefinition( if (ast_create_query.as_table_function) { - const auto * table_function = ast_create_query.as_table_function->as(); + const auto & table_function = ast_create_query.as_table_function->as(); const auto & factory = TableFunctionFactory::instance(); StoragePtr storage = factory.get(table_function->name, context)->execute(ast_create_query.as_table_function, context, ast_create_query.table); return {ast_create_query.table, storage}; From f9d1214bbc90390ebfb0f3044af7671a209d2665 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sat, 20 Jul 2019 02:51:43 +0300 Subject: [PATCH 47/57] Update DatabasesCommon.cpp --- dbms/src/Databases/DatabasesCommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Databases/DatabasesCommon.cpp b/dbms/src/Databases/DatabasesCommon.cpp index de3396b4603..3b5654083e0 100644 --- a/dbms/src/Databases/DatabasesCommon.cpp +++ b/dbms/src/Databases/DatabasesCommon.cpp @@ -73,7 +73,7 @@ std::pair createTableFromDefinition( { const auto & table_function = ast_create_query.as_table_function->as(); const auto & factory = TableFunctionFactory::instance(); - StoragePtr storage = factory.get(table_function->name, context)->execute(ast_create_query.as_table_function, context, ast_create_query.table); + StoragePtr storage = factory.get(table_function.name, context)->execute(ast_create_query.as_table_function, context, ast_create_query.table); return {ast_create_query.table, storage}; } /// We do not directly use `InterpreterCreateQuery::execute`, because From b035edefea7e81082697ffe8452dad087cc917b7 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sat, 20 Jul 2019 03:02:18 +0300 Subject: [PATCH 48/57] Update InterpreterCreateQuery.cpp --- dbms/src/Interpreters/InterpreterCreateQuery.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index a862c7de5c1..6ecf789076b 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -523,9 +523,9 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) if (create.as_table_function) { - const auto * table_function = create.as_table_function->as(); + const auto & table_function = create.as_table_function->as(); const auto & factory = TableFunctionFactory::instance(); - as_storage = factory.get(table_function->name, context)->execute(create.as_table_function, context, create.table); + as_storage = factory.get(table_function.name, context)->execute(create.as_table_function, context, create.table); } if (!as_table_name.empty()) { From 1b6a20d7112dca198e6b2758db0f784d81400eb6 Mon Sep 17 00:00:00 2001 From: Vivien Maisonneuve Date: Mon, 22 Jul 2019 14:13:07 +0200 Subject: [PATCH 49/57] Fix typos in docs --- docs/en/operations/server_settings/settings.md | 4 ++-- docs/en/operations/table_engines/mergetree.md | 2 +- docs/en/query_language/select.md | 2 +- docs/ru/operations/server_settings/settings.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/en/operations/server_settings/settings.md b/docs/en/operations/server_settings/settings.md index 2f87233e6a3..bf2a430f7d8 100644 --- a/docs/en/operations/server_settings/settings.md +++ b/docs/en/operations/server_settings/settings.md @@ -322,8 +322,8 @@ Writing to the syslog is also supported. Config example: Keys: -- user_syslog — Required setting if you want to write to the syslog. -- address — The host[:порт] of syslogd. If omitted, the local daemon is used. +- use_syslog — Required setting if you want to write to the syslog. +- address — The host[:port] of syslogd. If omitted, the local daemon is used. - hostname — Optional. The name of the host that logs are sent from. - facility — [The syslog facility keyword](https://en.wikipedia.org/wiki/Syslog#Facility) in uppercase letters with the "LOG_" prefix: (``LOG_USER``, ``LOG_DAEMON``, ``LOG_LOCAL3``, and so on). Default value: ``LOG_USER`` if ``address`` is specified, ``LOG_DAEMON otherwise.`` diff --git a/docs/en/operations/table_engines/mergetree.md b/docs/en/operations/table_engines/mergetree.md index 8610397be7c..f6ee0a7313d 100644 --- a/docs/en/operations/table_engines/mergetree.md +++ b/docs/en/operations/table_engines/mergetree.md @@ -1,6 +1,6 @@ # MergeTree {#table_engines-mergetree} -The `MergeTree` engine and other engines of this family (`*MergeTree`) are the most robust ClickHousе table engines. +The `MergeTree` engine and other engines of this family (`*MergeTree`) are the most robust ClickHouse table engines. The basic idea for `MergeTree` engines family is the following. When you have tremendous amount of a data that should be inserted into the table, you should write them quickly part by part and then merge parts by some rules in background. This method is much more efficient than constantly rewriting data in the storage at the insert. diff --git a/docs/en/query_language/select.md b/docs/en/query_language/select.md index ada90331d6e..b2d17bed674 100644 --- a/docs/en/query_language/select.md +++ b/docs/en/query_language/select.md @@ -694,7 +694,7 @@ The query `SELECT sum(x), y FROM t_null_big GROUP BY y` results in: └────────┴──────┘ ``` -You can see that `GROUP BY` for `У = NULL` summed up `x`, as if `NULL` is this value. +You can see that `GROUP BY` for `y = NULL` summed up `x`, as if `NULL` is this value. If you pass several keys to `GROUP BY`, the result will give you all the combinations of the selection, as if `NULL` were a specific value. diff --git a/docs/ru/operations/server_settings/settings.md b/docs/ru/operations/server_settings/settings.md index c1444668bb2..d0504dd99ca 100644 --- a/docs/ru/operations/server_settings/settings.md +++ b/docs/ru/operations/server_settings/settings.md @@ -319,7 +319,7 @@ ClickHouse проверит условия `min_part_size` и `min_part_size_rat ``` Ключи: -- user_syslog - обязательная настройка, если требуется запись в syslog +- use_syslog - обязательная настройка, если требуется запись в syslog - address - хост[:порт] демона syslogd. Если не указан, используется локальный - hostname - опционально, имя хоста, с которого отсылаются логи - facility - [категория syslog](https://en.wikipedia.org/wiki/Syslog#Facility), From 6c6af60194fd939e33b628ce7cb7434ee66507bc Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Mon, 22 Jul 2019 15:18:53 +0300 Subject: [PATCH 50/57] Fixes after review --- .../Interpreters/InterpreterCreateQuery.cpp | 53 ++++++++++--------- dbms/src/Parsers/ASTCreateQuery.cpp | 2 +- .../src/TableFunctions/ITableFunctionXDBC.cpp | 5 +- .../src/TableFunctions/TableFunctionMySQL.cpp | 5 +- ...3_create_table_as_table_function.reference | 1 + .../00973_create_table_as_table_function.sh | 13 ++++- 6 files changed, 42 insertions(+), 37 deletions(-) diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index a862c7de5c1..6685eab841b 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -386,7 +386,7 @@ ColumnsDescription InterpreterCreateQuery::setColumns( indices.indices.push_back( std::dynamic_pointer_cast(index->clone())); } - else if (!create.as_table.empty() || create.as_table_function) + else if (!create.as_table.empty()) { columns = as_storage->getColumns(); indices = as_storage->getIndices(); @@ -521,40 +521,43 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) StoragePtr as_storage; TableStructureReadLockHolder as_storage_lock; - if (create.as_table_function) - { - const auto * table_function = create.as_table_function->as(); - const auto & factory = TableFunctionFactory::instance(); - as_storage = factory.get(table_function->name, context)->execute(create.as_table_function, context, create.table); - } + ColumnsDescription columns; + StoragePtr res; + if (!as_table_name.empty()) { as_storage = context.getTable(as_database_name, as_table_name); as_storage_lock = as_storage->lockStructureForShare(false, context.getCurrentQueryId()); } - - /// Set and retrieve list of columns. - ColumnsDescription columns = setColumns(create, as_select_sample, as_storage); - - /// Check low cardinality types in creating table if it was not allowed in setting - if (!create.attach && !context.getSettingsRef().allow_suspicious_low_cardinality_types) + if (create.as_table_function) { - for (const auto & name_and_type_pair : columns.getAllPhysical()) + const auto * table_function = create.as_table_function->as(); + const auto & factory = TableFunctionFactory::instance(); + res = factory.get(table_function->name, context)->execute(create.as_table_function, context, create.table); + } + else + { + /// Set and retrieve list of columns. + columns = setColumns(create, as_select_sample, as_storage); + + /// Check low cardinality types in creating table if it was not allowed in setting + if (!create.attach && !context.getSettingsRef().allow_suspicious_low_cardinality_types) { - if (const auto * current_type_ptr = typeid_cast(name_and_type_pair.type.get())) + for (const auto & name_and_type_pair : columns.getAllPhysical()) { - if (!isStringOrFixedString(*removeNullable(current_type_ptr->getDictionaryType()))) - throw Exception("Creating columns of type " + current_type_ptr->getName() + " is prohibited by default due to expected negative impact on performance. It can be enabled with the \"allow_suspicious_low_cardinality_types\" setting.", - ErrorCodes::SUSPICIOUS_TYPE_FOR_LOW_CARDINALITY); + if (const auto * current_type_ptr = typeid_cast(name_and_type_pair.type.get())) + { + if (!isStringOrFixedString(*removeNullable(current_type_ptr->getDictionaryType()))) + throw Exception("Creating columns of type " + current_type_ptr->getName() + " is prohibited by default due to expected negative impact on performance. It can be enabled with the \"allow_suspicious_low_cardinality_types\" setting.", + ErrorCodes::SUSPICIOUS_TYPE_FOR_LOW_CARDINALITY); + } } } + + /// Set the table engine if it was not specified explicitly. + setEngine(create); } - /// Set the table engine if it was not specified explicitly. - setEngine(create); - - StoragePtr res; - { std::unique_ptr guard; @@ -594,9 +597,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) else if (context.tryGetExternalTable(table_name) && create.if_not_exists) return {}; - if (create.as_table_function) - res = as_storage; - else + if (!create.as_table_function) { res = StorageFactory::instance().get(create, data_path, diff --git a/dbms/src/Parsers/ASTCreateQuery.cpp b/dbms/src/Parsers/ASTCreateQuery.cpp index 4c84383af8a..d8656d171e5 100644 --- a/dbms/src/Parsers/ASTCreateQuery.cpp +++ b/dbms/src/Parsers/ASTCreateQuery.cpp @@ -235,7 +235,7 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat << (!as_database.empty() ? backQuoteIfNeed(as_database) + "." : "") << backQuoteIfNeed(as_table); } - if (columns_list && !as_table_function) + if (columns_list) { settings.ostr << (settings.one_line ? " (" : "\n("); FormatStateStacked frame_nested = frame; diff --git a/dbms/src/TableFunctions/ITableFunctionXDBC.cpp b/dbms/src/TableFunctions/ITableFunctionXDBC.cpp index 9998eb87fcf..81e30d0917d 100644 --- a/dbms/src/TableFunctions/ITableFunctionXDBC.cpp +++ b/dbms/src/TableFunctions/ITableFunctionXDBC.cpp @@ -76,10 +76,7 @@ StoragePtr ITableFunctionXDBC::executeImpl(const ASTPtr & ast_function, const Co readStringBinary(columns_info, buf); NamesAndTypesList columns = NamesAndTypesList::parse(columns_info); - ///If table_name was not specified by user, it will have the same name as remote_table_name - std::string local_table_name = (table_name == getName()) ? remote_table_name : table_name; - - auto result = std::make_shared(getDatabaseName(), local_table_name, schema_name, remote_table_name, ColumnsDescription{columns}, context, helper); + auto result = std::make_shared(getDatabaseName(), table_name, schema_name, remote_table_name, ColumnsDescription{columns}, context, helper); if (!result) throw Exception("Failed to instantiate storage from table function " + getName(), ErrorCodes::UNKNOWN_EXCEPTION); diff --git a/dbms/src/TableFunctions/TableFunctionMySQL.cpp b/dbms/src/TableFunctions/TableFunctionMySQL.cpp index ac8d2671878..cee08cf2a1d 100644 --- a/dbms/src/TableFunctions/TableFunctionMySQL.cpp +++ b/dbms/src/TableFunctions/TableFunctionMySQL.cpp @@ -118,12 +118,9 @@ StoragePtr TableFunctionMySQL::executeImpl(const ASTPtr & ast_function, const Co if (columns.empty()) throw Exception("MySQL table " + backQuoteIfNeed(remote_database_name) + "." + backQuoteIfNeed(remote_table_name) + " doesn't exist.", ErrorCodes::UNKNOWN_TABLE); - ///If table_name was not specified by user, it will have the same name as remote_table_name - std::string local_table_name = (table_name == getName()) ? remote_table_name : table_name; - auto res = StorageMySQL::create( getDatabaseName(), - local_table_name, + table_name, std::move(pool), remote_database_name, remote_table_name, diff --git a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference index 830a01e609a..bf96e1a5e55 100644 --- a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference +++ b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.reference @@ -17,6 +17,7 @@ 15 16 17 +18 19 20 21 diff --git a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh index aff96a364d1..2aedc097d40 100755 --- a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh +++ b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh @@ -7,6 +7,8 @@ $CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t1" $CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t2" $CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t3" $CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t4" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t5" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t6" $CLICKHOUSE_CLIENT -q "CREATE TABLE t1 AS remote('127.0.0.1', system.one)" $CLICKHOUSE_CLIENT -q "SELECT count() FROM t1" @@ -15,12 +17,19 @@ $CLICKHOUSE_CLIENT -q "CREATE TABLE t2 AS remote('127.0.0.1', system.numbers)" $CLICKHOUSE_CLIENT -q "SELECT * FROM t2 LIMIT 18" $CLICKHOUSE_CLIENT -q "CREATE TABLE t3 AS remote('127.0.0.1', numbers(100))" -$CLICKHOUSE_CLIENT -q "SELECT * FROM t3 where number > 18 and number < 25" +$CLICKHOUSE_CLIENT -q "SELECT * FROM t3 where number > 17 and number < 25" $CLICKHOUSE_CLIENT -q "CREATE TABLE t4 AS numbers(100)" -$CLICKHOUSE_CLIENT -q "SELECT count() FROM t4 where number > 74" +$CLICKHOUSE_CLIENT -q "SELECT count() FROM t4 where number > 74 " + +#Syntax Errors +$CLICKHOUSE_CLIENT -q "CREATE TABLE t5 (d UInt68) AS numbers(10) -- { clientError 62 }" +$CLICKHOUSE_CLIENT -q "CREATE TABLE t6 (d UInt68) Engine = TinyLog AS numbers(10) -- { clientError 62 }" $CLICKHOUSE_CLIENT -q "DROP TABLE t1" $CLICKHOUSE_CLIENT -q "DROP TABLE t2" $CLICKHOUSE_CLIENT -q "DROP TABLE t3" $CLICKHOUSE_CLIENT -q "DROP TABLE t4" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t5" +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t6" + From 42efc1051ed3624c005f7c842fbafd693ae1cfb8 Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Mon, 22 Jul 2019 15:35:29 +0300 Subject: [PATCH 51/57] fixes of bad commit --- dbms/src/Interpreters/InterpreterCreateQuery.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index f3f5701c90e..6685eab841b 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -531,13 +531,9 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) } if (create.as_table_function) { - const auto & table_function = create.as_table_function->as(); + const auto * table_function = create.as_table_function->as(); const auto & factory = TableFunctionFactory::instance(); -<<<<<<< HEAD res = factory.get(table_function->name, context)->execute(create.as_table_function, context, create.table); -======= - as_storage = factory.get(table_function.name, context)->execute(create.as_table_function, context, create.table); ->>>>>>> b035edefea7e81082697ffe8452dad087cc917b7 } else { From 9ffbf65741b0a52a43309de7323346b4e11f5a9c Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Mon, 22 Jul 2019 15:50:10 +0300 Subject: [PATCH 52/57] Fix of a fix --- dbms/src/Interpreters/InterpreterCreateQuery.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 6685eab841b..4234ff78da9 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -531,9 +531,9 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) } if (create.as_table_function) { - const auto * table_function = create.as_table_function->as(); + const auto & table_function = create.as_table_function->as(); const auto & factory = TableFunctionFactory::instance(); - res = factory.get(table_function->name, context)->execute(create.as_table_function, context, create.table); + res = factory.get(table_function.name, context)->execute(create.as_table_function, context, create.table); } else { From 0320de91321752e1a7d39f2a5183da77ae72c046 Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Mon, 22 Jul 2019 16:49:16 +0300 Subject: [PATCH 53/57] test and fixes --- .../Interpreters/InterpreterCreateQuery.cpp | 7 ++-- .../00973_create_table_as_table_function.sh | 35 ------------------- .../00973_create_table_as_table_function.sql | 21 +++++++++++ 3 files changed, 25 insertions(+), 38 deletions(-) delete mode 100755 dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh create mode 100644 dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sql diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 4234ff78da9..b44f42f0f13 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -521,14 +521,15 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) StoragePtr as_storage; TableStructureReadLockHolder as_storage_lock; - ColumnsDescription columns; - StoragePtr res; - if (!as_table_name.empty()) { as_storage = context.getTable(as_database_name, as_table_name); as_storage_lock = as_storage->lockStructureForShare(false, context.getCurrentQueryId()); } + + ColumnsDescription columns; + StoragePtr res; + if (create.as_table_function) { const auto & table_function = create.as_table_function->as(); diff --git a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh deleted file mode 100755 index 2aedc097d40..00000000000 --- a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sh +++ /dev/null @@ -1,35 +0,0 @@ -#!/usr/bin/env bash - -CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) -. $CURDIR/../shell_config.sh - -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t1" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t2" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t3" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t4" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t5" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t6" - -$CLICKHOUSE_CLIENT -q "CREATE TABLE t1 AS remote('127.0.0.1', system.one)" -$CLICKHOUSE_CLIENT -q "SELECT count() FROM t1" - -$CLICKHOUSE_CLIENT -q "CREATE TABLE t2 AS remote('127.0.0.1', system.numbers)" -$CLICKHOUSE_CLIENT -q "SELECT * FROM t2 LIMIT 18" - -$CLICKHOUSE_CLIENT -q "CREATE TABLE t3 AS remote('127.0.0.1', numbers(100))" -$CLICKHOUSE_CLIENT -q "SELECT * FROM t3 where number > 17 and number < 25" - -$CLICKHOUSE_CLIENT -q "CREATE TABLE t4 AS numbers(100)" -$CLICKHOUSE_CLIENT -q "SELECT count() FROM t4 where number > 74 " - -#Syntax Errors -$CLICKHOUSE_CLIENT -q "CREATE TABLE t5 (d UInt68) AS numbers(10) -- { clientError 62 }" -$CLICKHOUSE_CLIENT -q "CREATE TABLE t6 (d UInt68) Engine = TinyLog AS numbers(10) -- { clientError 62 }" - -$CLICKHOUSE_CLIENT -q "DROP TABLE t1" -$CLICKHOUSE_CLIENT -q "DROP TABLE t2" -$CLICKHOUSE_CLIENT -q "DROP TABLE t3" -$CLICKHOUSE_CLIENT -q "DROP TABLE t4" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t5" -$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS t6" - diff --git a/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sql b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sql new file mode 100644 index 00000000000..147cb87ffcc --- /dev/null +++ b/dbms/tests/queries/0_stateless/00973_create_table_as_table_function.sql @@ -0,0 +1,21 @@ +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; +DROP TABLE IF EXISTS t3; +DROP TABLE IF EXISTS t4; + +CREATE TABLE t1 AS remote('127.0.0.1', system.one); +SELECT count() FROM t1; + +CREATE TABLE t2 AS remote('127.0.0.1', system.numbers); +SELECT * FROM t2 LIMIT 18; + +CREATE TABLE t3 AS remote('127.0.0.1', numbers(100)); +SELECT * FROM t3 where number > 17 and number < 25; + +CREATE TABLE t4 AS numbers(100); +SELECT count() FROM t4 where number > 74; + +DROP TABLE t1; +DROP TABLE t2; +DROP TABLE t3; +DROP TABLE t4; From 21ebb72579ac8193c94a253422d12db1ef0705d8 Mon Sep 17 00:00:00 2001 From: Dmitry Rubashkin Date: Mon, 22 Jul 2019 18:02:03 +0300 Subject: [PATCH 54/57] documentation --- docs/en/query_language/create.md | 6 ++++++ docs/ru/query_language/create.md | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/docs/en/query_language/create.md b/docs/en/query_language/create.md index 94d32cc49f5..bd2228efa94 100644 --- a/docs/en/query_language/create.md +++ b/docs/en/query_language/create.md @@ -52,6 +52,12 @@ CREATE TABLE [IF NOT EXISTS] [db.]table_name AS [db2.]name2 [ENGINE = engine] Creates a table with the same structure as another table. You can specify a different engine for the table. If the engine is not specified, the same engine will be used as for the `db2.name2` table. +``` sql +CREATE TABLE [IF NOT EXISTS] [db.]table_name AS table_fucntion() +``` + +Creates a table with the same structure and data as the value returned by table function. + ``` sql CREATE TABLE [IF NOT EXISTS] [db.]table_name ENGINE = engine AS SELECT ... ``` diff --git a/docs/ru/query_language/create.md b/docs/ru/query_language/create.md index 9d1149ba466..be75bb9ce51 100644 --- a/docs/ru/query_language/create.md +++ b/docs/ru/query_language/create.md @@ -34,6 +34,12 @@ CREATE TABLE [IF NOT EXISTS] [db.]table_name AS [db2.]name2 [ENGINE = engine] Создаёт таблицу с такой же структурой, как другая таблица. Можно указать другой движок для таблицы. Если движок не указан, то будет выбран такой же движок, как у таблицы `db2.name2`. +``` sql +CREATE TABLE [IF NOT EXISTS] [db.]table_name AS table_fucntion() +``` + +Создаёт таблицу с такой же структурой и данными, как результат соотвествующей табличной функцией. + ```sql CREATE TABLE [IF NOT EXISTS] [db.]table_name ENGINE = engine AS SELECT ... ``` From a652a8bb6c34d08053fb467fd5cd0d188a71d7e6 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 23 Jul 2019 03:04:04 +0300 Subject: [PATCH 55/57] Disabled query profiler by default #4247 --- dbms/src/Core/Settings.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Core/Settings.h b/dbms/src/Core/Settings.h index 06c82101956..27e06ffec10 100644 --- a/dbms/src/Core/Settings.h +++ b/dbms/src/Core/Settings.h @@ -221,8 +221,8 @@ struct Settings : public SettingsCollection M(SettingBool, empty_result_for_aggregation_by_empty_set, false, "Return empty result when aggregating without keys on empty set.") \ M(SettingBool, allow_distributed_ddl, true, "If it is set to true, then a user is allowed to executed distributed DDL queries.") \ M(SettingUInt64, odbc_max_field_size, 1024, "Max size of filed can be read from ODBC dictionary. Long strings are truncated.") \ - M(SettingUInt64, query_profiler_real_time_period_ns, 500000000, "Period for real clock timer of query profiler (in nanoseconds). Set 0 value to turn off real clock query profiler") \ - M(SettingUInt64, query_profiler_cpu_time_period_ns, 500000000, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off CPU clock query profiler") \ + M(SettingUInt64, query_profiler_real_time_period_ns, 0, "Period for real clock timer of query profiler (in nanoseconds). Set 0 value to turn off real clock query profiler") \ + M(SettingUInt64, query_profiler_cpu_time_period_ns, 0, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off CPU clock query profiler") \ \ \ /** Limits during query execution are part of the settings. \ From 8c71c109a65b93fe90678050799182089e3d026b Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Tue, 23 Jul 2019 11:20:19 +0300 Subject: [PATCH 56/57] Auto version update to [19.12.1.889] [54424] --- dbms/cmake/version.cmake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbms/cmake/version.cmake b/dbms/cmake/version.cmake index 5714b5207b8..66f3f2a3ef4 100644 --- a/dbms/cmake/version.cmake +++ b/dbms/cmake/version.cmake @@ -3,9 +3,9 @@ set(VERSION_REVISION 54424) set(VERSION_MAJOR 19) set(VERSION_MINOR 12) set(VERSION_PATCH 1) -set(VERSION_GITHASH a584f0ca6cb5df9b0d9baf1e2e1eaa7d12a20a44) -set(VERSION_DESCRIBE v19.12.1.1-prestable) -set(VERSION_STRING 19.12.1.1) +set(VERSION_GITHASH adfc36917222bdb03eba069f0cad0f4f5b8f1c94) +set(VERSION_DESCRIBE v19.12.1.889-prestable) +set(VERSION_STRING 19.12.1.889) # end of autochange set(VERSION_EXTRA "" CACHE STRING "") From 58e03ad7b9fa54b069be71ebbdef206488100f38 Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Tue, 23 Jul 2019 11:20:52 +0300 Subject: [PATCH 57/57] Auto version update to [19.13.1.1] [54425] --- dbms/cmake/version.cmake | 8 ++++---- debian/changelog | 4 ++-- docker/client/Dockerfile | 2 +- docker/server/Dockerfile | 2 +- docker/test/Dockerfile | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dbms/cmake/version.cmake b/dbms/cmake/version.cmake index 66f3f2a3ef4..d4c7b376b89 100644 --- a/dbms/cmake/version.cmake +++ b/dbms/cmake/version.cmake @@ -1,11 +1,11 @@ # This strings autochanged from release_lib.sh: -set(VERSION_REVISION 54424) +set(VERSION_REVISION 54425) set(VERSION_MAJOR 19) -set(VERSION_MINOR 12) +set(VERSION_MINOR 13) set(VERSION_PATCH 1) set(VERSION_GITHASH adfc36917222bdb03eba069f0cad0f4f5b8f1c94) -set(VERSION_DESCRIBE v19.12.1.889-prestable) -set(VERSION_STRING 19.12.1.889) +set(VERSION_DESCRIBE v19.13.1.1-prestable) +set(VERSION_STRING 19.13.1.1) # end of autochange set(VERSION_EXTRA "" CACHE STRING "") diff --git a/debian/changelog b/debian/changelog index 6abe0effc06..f1db1b81185 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,5 +1,5 @@ -clickhouse (19.12.1.1) unstable; urgency=low +clickhouse (19.13.1.1) unstable; urgency=low * Modified source code - -- clickhouse-release Wed, 10 Jul 2019 22:57:50 +0300 + -- clickhouse-release Tue, 23 Jul 2019 11:20:49 +0300 diff --git a/docker/client/Dockerfile b/docker/client/Dockerfile index 1afc097aafd..68cdf3f0204 100644 --- a/docker/client/Dockerfile +++ b/docker/client/Dockerfile @@ -1,7 +1,7 @@ FROM ubuntu:18.04 ARG repository="deb http://repo.yandex.ru/clickhouse/deb/stable/ main/" -ARG version=19.12.1.* +ARG version=19.13.1.* RUN apt-get update \ && apt-get install --yes --no-install-recommends \ diff --git a/docker/server/Dockerfile b/docker/server/Dockerfile index fed4295b76c..934c1921a67 100644 --- a/docker/server/Dockerfile +++ b/docker/server/Dockerfile @@ -1,7 +1,7 @@ FROM ubuntu:18.04 ARG repository="deb http://repo.yandex.ru/clickhouse/deb/stable/ main/" -ARG version=19.12.1.* +ARG version=19.13.1.* ARG gosu_ver=1.10 RUN apt-get update \ diff --git a/docker/test/Dockerfile b/docker/test/Dockerfile index 488a2554483..5c2bd25b48c 100644 --- a/docker/test/Dockerfile +++ b/docker/test/Dockerfile @@ -1,7 +1,7 @@ FROM ubuntu:18.04 ARG repository="deb http://repo.yandex.ru/clickhouse/deb/stable/ main/" -ARG version=19.12.1.* +ARG version=19.13.1.* RUN apt-get update && \ apt-get install -y apt-transport-https dirmngr && \