Merge pull request #24833 from ClickHouse/simplify-pipe-size

Simplify code around TraceCollector, alternative to #24829
This commit is contained in:
alexey-milovidov 2021-06-04 01:13:26 +03:00 committed by GitHub
commit c58fb0ec3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 23 deletions

View File

@ -37,18 +37,11 @@ public:
std::optional<UInt64> line;
};
static constexpr size_t capacity =
#ifndef NDEBUG
/* The stacks are normally larger in debug version due to less inlining.
*
* NOTE: it cannot be larger then 56 right now, since otherwise it will
* not fit into minimal PIPE_BUF (512) in TraceCollector.
*/
56
#else
32
#endif
;
/* NOTE: It cannot be larger right now, since otherwise it
* will not fit into minimal PIPE_BUF (512) in TraceCollector.
*/
static constexpr size_t capacity = 45;
using FramePointers = std::array<void *, capacity>;
using Frames = std::array<Frame, capacity>;

View File

@ -28,6 +28,7 @@ namespace
/// `aggregating_merge_tree_simple_aggregate_function_string.query100.profile100`,
/// so make some allowance for them as well.
constexpr size_t QUERY_ID_MAX_LEN = 128;
static_assert(QUERY_ID_MAX_LEN <= std::numeric_limits<uint8_t>::max());
}
LazyPipeFDs pipe;
@ -61,17 +62,19 @@ TraceCollector::~TraceCollector()
void TraceCollector::collect(TraceType trace_type, const StackTrace & stack_trace, Int64 size)
{
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(UInt8) + // number of stack frames
sizeof(StackTrace::FramePointers) + // collected stack trace, maximum capacity
sizeof(TraceType) + // trace type
sizeof(UInt64) + // thread_id
sizeof(Int64); // size
constexpr size_t buf_size = sizeof(char) /// TraceCollector stop flag
+ sizeof(UInt8) /// String size
+ QUERY_ID_MAX_LEN /// Maximum query_id length
+ sizeof(UInt8) /// Number of stack frames
+ sizeof(StackTrace::FramePointers) /// Collected stack trace, maximum capacity
+ sizeof(TraceType) /// trace type
+ sizeof(UInt64) /// thread_id
+ sizeof(Int64); /// size
/// Write should be atomic to avoid overlaps
/// (since recursive collect() is possible)
static_assert(buf_size < PIPE_BUF, "Only write of PIPE_BUF to pipe is atomic");
static_assert(PIPE_BUF >= 512);
static_assert(buf_size <= 512, "Only write of PIPE_BUF to pipe is atomic and the minimal known PIPE_BUF across supported platforms is 512");
char buffer[buf_size];
WriteBufferFromFileDescriptorDiscardOnFailure out(pipe.fds_rw[1], buf_size, buffer);
@ -92,7 +95,9 @@ void TraceCollector::collect(TraceType trace_type, const StackTrace & stack_trac
}
writeChar(false, out); /// true if requested to stop the collecting thread.
writeStringBinary(query_id, out);
writeBinary(static_cast<uint8_t>(query_id.size), out);
out.write(query_id.data, query_id.size);
size_t stack_trace_size = stack_trace.getSize();
size_t stack_trace_offset = stack_trace.getOffset();
@ -137,7 +142,10 @@ void TraceCollector::run()
break;
std::string query_id;
readStringBinary(query_id, in);
UInt8 query_id_size = 0;
readBinary(query_id_size, in);
query_id.resize(query_id_size);
in.read(query_id.data(), query_id_size);
UInt8 trace_size = 0;
readIntBinary(trace_size, in);