From 1b30f91c421917734cfbd8d970265f43f23dab9b Mon Sep 17 00:00:00 2001 From: Guillaume Tassery Date: Mon, 13 May 2019 11:48:09 +0700 Subject: [PATCH] Return to old implementation of read and write values for progress --- dbms/src/IO/Progress.cpp | 102 +++++++----------- dbms/src/IO/Progress.h | 99 +---------------- .../IO/WriteBufferFromHTTPServerResponse.cpp | 6 +- ...copatch_progress_in_http_headers.reference | 32 +++--- 4 files changed, 62 insertions(+), 177 deletions(-) diff --git a/dbms/src/IO/Progress.cpp b/dbms/src/IO/Progress.cpp index bdff0ce0d92..95c01f16e0c 100644 --- a/dbms/src/IO/Progress.cpp +++ b/dbms/src/IO/Progress.cpp @@ -8,7 +8,7 @@ namespace DB { -void AllProgressValueImpl::read(ProgressValues & value, ReadBuffer & in, UInt64 /*server_revision*/) +void ProgressValues::read(ReadBuffer & in, UInt64 server_revision) { size_t new_rows = 0; size_t new_bytes = 0; @@ -19,92 +19,70 @@ void AllProgressValueImpl::read(ProgressValues & value, ReadBuffer & in, UInt64 readVarUInt(new_rows, in); readVarUInt(new_bytes, in); readVarUInt(new_total_rows, in); - readVarUInt(new_write_rows, in); - readVarUInt(new_write_bytes, in); + if (DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO >= server_revision) + { + readVarUInt(new_write_rows, in); + readVarUInt(new_write_bytes, in); + } - value.rows = new_rows; - value.bytes = new_bytes; - value.total_rows = new_total_rows; - value.write_rows = new_write_rows; - value.write_bytes = new_write_bytes; + this->rows = new_rows; + this->bytes = new_bytes; + this->total_rows = new_total_rows; + this->write_rows = new_write_rows; + this->write_bytes = new_write_bytes; } -void AllProgressValueImpl::write(const ProgressValues & value, WriteBuffer & out, UInt64 /*client_revision*/) +void ProgressValues::write(WriteBuffer & out, UInt64 client_revision) const { - writeVarUInt(value.rows, out); - writeVarUInt(value.bytes, out); - writeVarUInt(value.total_rows, out); - writeVarUInt(value.write_rows, out); - writeVarUInt(value.write_bytes, out); + writeVarUInt(this->rows, out); + writeVarUInt(this->bytes, out); + writeVarUInt(this->total_rows, out); + if (client_revision >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) + { + writeVarUInt(this->write_rows, out); + writeVarUInt(this->write_bytes, out); + } } -void AllProgressValueImpl::writeJSON(const ProgressValues & value, WriteBuffer & out) +void ProgressValues::writeJSON(WriteBuffer & out) const { /// Numbers are written in double quotes (as strings) to avoid loss of precision /// of 64-bit integers after interpretation by JavaScript. writeCString("{\"read_rows\":\"", out); - writeText(value.rows, out); + writeText(this->rows, out); writeCString("\",\"read_bytes\":\"", out); - writeText(value.bytes, out); - writeCString("\",\"write_rows\":\"", out); - writeText(value.write_rows, out); - writeCString("\",\"write_bytes\":\"", out); - writeText(value.write_bytes, out); + writeText(this->bytes, out); + writeCString("\",\"written_rows\":\"", out); + writeText(this->write_rows, out); + writeCString("\",\"written_bytes\":\"", out); + writeText(this->write_bytes, out); writeCString("\",\"total_rows\":\"", out); - writeText(value.total_rows, out); + writeText(this->total_rows, out); writeCString("\"}", out); } -void ReadProgressValueImpl::read(ProgressValues & value, ReadBuffer & in, UInt64 /*server_revision*/) +void Progress::read(ReadBuffer & in, UInt64 server_revision) { - size_t new_rows = 0; - size_t new_bytes = 0; - size_t new_total_rows = 0; + ProgressValues values; + values.read(in, server_revision); - readVarUInt(new_rows, in); - readVarUInt(new_bytes, in); - readVarUInt(new_total_rows, in); - - value.rows = new_rows; - value.bytes = new_bytes; - value.total_rows = new_total_rows; + rows.store(values.rows, std::memory_order_relaxed); + bytes.store(values.bytes, std::memory_order_relaxed); + total_rows.store(values.total_rows, std::memory_order_relaxed); + write_rows.store(values.write_rows, std::memory_order_relaxed); + write_bytes.store(values.write_bytes, std::memory_order_relaxed); } - -void ReadProgressValueImpl::write(const ProgressValues & value, WriteBuffer & out, UInt64 /*client_revision*/) +void Progress::write(WriteBuffer & out, UInt64 client_revision) const { - writeVarUInt(value.rows, out); - writeVarUInt(value.bytes, out); - writeVarUInt(value.total_rows, out); + getValues().write(out, client_revision); } - -void ReadProgressValueImpl::writeJSON(const ProgressValues & value, WriteBuffer & out) +void Progress::writeJSON(WriteBuffer & out) const { - /// Numbers are written in double quotes (as strings) to avoid loss of precision - /// of 64-bit integers after interpretation by JavaScript. - - writeCString("{\"read_rows\":\"", out); - writeText(value.rows, out); - writeCString("\",\"read_bytes\":\"", out); - writeText(value.bytes, out); - writeCString("\",\"total_rows\":\"", out); - writeText(value.total_rows, out); - writeCString("\"}", out); -} - -void WriteProgressValueImpl::writeJSON(const ProgressValues & value, WriteBuffer & out) -{ - /// Numbers are written in double quotes (as strings) to avoid loss of precision - /// of 64-bit integers after interpretation by JavaScript. - - writeCString("{\"write_rows\":\"", out); - writeText(value.write_rows, out); - writeCString("\",\"write_bytes\":\"", out); - writeText(value.write_bytes, out); - writeCString("\"}", out); + getValues().writeJSON(out); } } diff --git a/dbms/src/IO/Progress.h b/dbms/src/IO/Progress.h index e2b31eeb421..05053619557 100644 --- a/dbms/src/IO/Progress.h +++ b/dbms/src/IO/Progress.h @@ -12,8 +12,6 @@ namespace DB class ReadBuffer; class WriteBuffer; -struct AllProgressValueImpl; - /// See Progress. struct ProgressValues { @@ -23,34 +21,9 @@ struct ProgressValues size_t write_rows; size_t write_bytes; - - template - inline void read(ReadBuffer & in, UInt64 server_revision); - - template - inline void write(WriteBuffer & out, UInt64 client_revision) const; - - template - inline void writeJSON(WriteBuffer & out) const; -}; - -struct AllProgressValueImpl -{ - static void read(ProgressValues & value, ReadBuffer & in, UInt64 server_revision); - static void write(const ProgressValues & value, WriteBuffer & out, UInt64 client_revision); - static void writeJSON(const ProgressValues & value, WriteBuffer & out); -}; - -struct ReadProgressValueImpl : public AllProgressValueImpl -{ - static void read(ProgressValues & value, ReadBuffer & in, UInt64 server_revision); - static void write(const ProgressValues & value, WriteBuffer & out, UInt64 client_revision); - static void writeJSON(const ProgressValues & value, WriteBuffer & out); -}; - -struct WriteProgressValueImpl : public AllProgressValueImpl -{ - static void writeJSON(const ProgressValues & value, WriteBuffer & out); + void read(ReadBuffer & in, UInt64 server_revision); + void write(WriteBuffer & out, UInt64 client_revision) const; + void writeJSON(WriteBuffer & out) const; }; struct ReadProgress @@ -99,14 +72,9 @@ struct Progress Progress(WriteProgress write_progress) : write_rows(write_progress.write_rows), write_bytes(write_progress.write_bytes) {} - template void read(ReadBuffer & in, UInt64 server_revision); - - template void write(WriteBuffer & out, UInt64 client_revision) const; - /// Progress in JSON format (single line, without whitespaces) is used in HTTP headers. - template void writeJSON(WriteBuffer & out) const; /// Each value separately is changed atomically (but not whole object). @@ -173,65 +141,4 @@ struct Progress } }; -template -void Progress::read(ReadBuffer & in, UInt64 server_revision) -{ - ProgressValues values; - values.read(in, server_revision); - - rows.store(values.rows, std::memory_order_relaxed); - bytes.store(values.bytes, std::memory_order_relaxed); - total_rows.store(values.total_rows, std::memory_order_relaxed); - write_rows.store(values.write_rows, std::memory_order_relaxed); - write_bytes.store(values.write_bytes, std::memory_order_relaxed); -} - -template -void Progress::write(WriteBuffer & out, UInt64 client_revision) const -{ - getValues().write(out, client_revision); -} - -template -void Progress::writeJSON(WriteBuffer & out) const -{ - getValues().writeJSON(out); -} - -template -inline void ProgressValues::read(ReadBuffer & in, UInt64 server_revision) -{ - if (server_revision >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) - ReadImpl::read(*this, in, server_revision); - else - read(in, server_revision); -} - -template <> -inline void ProgressValues::read(ReadBuffer & in, UInt64 server_revision) -{ - ReadProgressValueImpl::read(*this, in, server_revision); -} - -template -inline void ProgressValues::write(WriteBuffer & out, UInt64 client_revision) const -{ - if (client_revision >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) - WriteImpl::write(*this, out, client_revision); - else - write(out, client_revision); -} - -template <> -inline void ProgressValues::write(WriteBuffer & out, UInt64 client_revision) const -{ - ReadProgressValueImpl::write(*this, out, client_revision); -} - -template -void ProgressValues::writeJSON(WriteBuffer & out) const -{ - WriteJSONImpl::writeJSON(*this, out); -} - } diff --git a/dbms/src/IO/WriteBufferFromHTTPServerResponse.cpp b/dbms/src/IO/WriteBufferFromHTTPServerResponse.cpp index e00f7273141..43c406ef11b 100644 --- a/dbms/src/IO/WriteBufferFromHTTPServerResponse.cpp +++ b/dbms/src/IO/WriteBufferFromHTTPServerResponse.cpp @@ -38,13 +38,13 @@ void WriteBufferFromHTTPServerResponse::startSendHeaders() void WriteBufferFromHTTPServerResponse::writeHeaderSummary() { +#if defined(POCO_CLICKHOUSE_PATCH) if (headers_finished_sending) return; WriteBufferFromOwnString progress_string_writer; accumulated_progress.writeJSON(progress_string_writer); -#if defined(POCO_CLICKHOUSE_PATCH) if (response_header_ostr) *response_header_ostr << "X-ClickHouse-Summary: " << progress_string_writer.str() << "\r\n" << std::flush; #endif @@ -52,13 +52,13 @@ void WriteBufferFromHTTPServerResponse::writeHeaderSummary() void WriteBufferFromHTTPServerResponse::writeHeaderProgress() { +#if defined(POCO_CLICKHOUSE_PATCH) if (headers_finished_sending) return; WriteBufferFromOwnString progress_string_writer; - accumulated_progress.writeJSON(progress_string_writer); + accumulated_progress.writeJSON(progress_string_writer); -#if defined(POCO_CLICKHOUSE_PATCH) *response_header_ostr << "X-ClickHouse-Progress: " << progress_string_writer.str() << "\r\n" << std::flush; #endif } diff --git a/dbms/tests/queries/0_stateless/00416_pocopatch_progress_in_http_headers.reference b/dbms/tests/queries/0_stateless/00416_pocopatch_progress_in_http_headers.reference index 673feb24753..a2236e9deda 100644 --- a/dbms/tests/queries/0_stateless/00416_pocopatch_progress_in_http_headers.reference +++ b/dbms/tests/queries/0_stateless/00416_pocopatch_progress_in_http_headers.reference @@ -1,19 +1,19 @@ -< X-ClickHouse-Progress: {"read_rows":"0","read_bytes":"0","total_rows":"10"} -< X-ClickHouse-Progress: {"read_rows":"5","read_bytes":"40","total_rows":"10"} -< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","total_rows":"10"} -< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","total_rows":"10"} +< X-ClickHouse-Progress: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows":"10"} +< X-ClickHouse-Progress: {"read_rows":"5","read_bytes":"40","written_rows":"0","written_bytes":"0","total_rows":"10"} +< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","written_rows":"0","written_bytes":"0","total_rows":"10"} +< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","written_rows":"0","written_bytes":"0","total_rows":"10"} 9 -< X-ClickHouse-Progress: {"read_rows":"1","read_bytes":"8","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"2","read_bytes":"16","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"3","read_bytes":"24","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"4","read_bytes":"32","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"5","read_bytes":"40","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"6","read_bytes":"48","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"7","read_bytes":"56","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"8","read_bytes":"64","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"9","read_bytes":"72","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","total_rows":"0"} -< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"1","read_bytes":"8","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"2","read_bytes":"16","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"3","read_bytes":"24","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"4","read_bytes":"32","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"5","read_bytes":"40","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"6","read_bytes":"48","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"7","read_bytes":"56","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"8","read_bytes":"64","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"9","read_bytes":"72","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","written_rows":"0","written_bytes":"0","total_rows":"0"} +< X-ClickHouse-Progress: {"read_rows":"10","read_bytes":"80","written_rows":"0","written_bytes":"0","total_rows":"0"} 0 1 2 @@ -34,4 +34,4 @@ 7 8 9 -< X-ClickHouse-Summary: {"read_rows":"10","read_bytes":"80","write_rows":"10","write_bytes":"40","total_rows":"0"} +< X-ClickHouse-Summary: {"read_rows":"10","read_bytes":"80","written_rows":"10","written_bytes":"40","total_rows":"0"}