From 947dd6f912d72fccd313f2ad0841faad23f1ddbd Mon Sep 17 00:00:00 2001 From: Anthonin Bonnefoy Date: Tue, 11 Jun 2024 11:00:54 +0200 Subject: [PATCH] Rename parameters, missing initialisation and new constants --- src/pg_tracing.c | 12 ++++++------ src/pg_tracing.h | 7 ++++--- src/pg_tracing_planstate.c | 32 +++++++++++++++++--------------- src/pg_tracing_query_process.c | 4 ++-- src/pg_tracing_span.c | 8 ++++---- 5 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/pg_tracing.c b/src/pg_tracing.c index 2d68ac0..b768f0c 100644 --- a/src/pg_tracing.c +++ b/src/pg_tracing.c @@ -195,9 +195,6 @@ int nested_level = 0; /* Whether we're in a cursor declaration */ static bool within_declare_cursor = false; -/* Number of spans initially allocated at the start of a trace. */ -static int pg_tracing_initial_allocated_spans = 25; - /* Commit span used in xact callbacks */ static Span commit_span; @@ -210,6 +207,9 @@ static uint64 current_query_id; static pgTracingQueryIdFilter * query_id_filter = NULL; static pgTracingPerLevelInfos * per_level_infos = NULL; +/* Number of spans initially allocated at the start of a trace. */ +#define INITIAL_ALLOCATED_SPANS 25 + static void pg_tracing_shmem_request(void); static void pg_tracing_shmem_startup(void); @@ -226,7 +226,7 @@ static ProcessUtility_hook_type prev_ProcessUtility = NULL; static void pg_tracing_post_parse_analyze(ParseState *pstate, Query *query, JumbleState *jstate); -static PlannedStmt *pg_tracing_planner_hook(Query *parse, +static PlannedStmt *pg_tracing_planner_hook(Query *query, const char *query_string, int cursorOptions, ParamListInfo params); @@ -1133,8 +1133,8 @@ initialize_trace_level(void) per_level_infos = palloc0(allocated_nested_level * sizeof(pgTracingPerLevelInfos)); current_trace_spans = palloc0(sizeof(pgTracingSpans) + - pg_tracing_initial_allocated_spans * sizeof(Span)); - current_trace_spans->max = pg_tracing_initial_allocated_spans; + INITIAL_ALLOCATED_SPANS * sizeof(Span)); + current_trace_spans->max = INITIAL_ALLOCATED_SPANS; current_trace_text = makeStringInfo(); MemoryContextSwitchTo(oldcxt); diff --git a/src/pg_tracing.h b/src/pg_tracing.h index cb7ab48..5b7d239 100644 --- a/src/pg_tracing.h +++ b/src/pg_tracing.h @@ -247,7 +247,7 @@ typedef struct TracedPlanstate extern Span create_span_node(PlanState *planstate, const planstateTraceContext * planstateTraceContext, - uint64 *span_id, uint64 parent_id, uint64 queryId, SpanType span_type, + uint64 *span_id, uint64 parent_id, uint64 query_id, SpanType span_type, char *subplan_name, TimestampTz span_start, TimestampTz span_end); extern TimestampTz generate_span_from_planstate(PlanState *planstate, planstateTraceContext * planstateTraceContext, @@ -279,8 +279,9 @@ extern const char *qtext_load_file(Size *buffer_size); extern const char *qtext_load_file(Size *buffer_size); /* pg_tracing_span.c */ -extern void begin_span(TraceId trace_id, Span * span, - SpanType type, uint64 *span_id, uint64 parent_id, uint64 query_id, const TimestampTz *start_span); +extern void begin_span(TraceId trace_id, Span * span, SpanType type, + uint64 *span_id, uint64 parent_id, uint64 query_id, + const TimestampTz *start_span); extern void end_span(Span * span, const TimestampTz *end_time); extern void reset_span(Span * span); extern const char *get_span_type(const Span * span, const char *qbuffer, Size qbuffer_size); diff --git a/src/pg_tracing_planstate.c b/src/pg_tracing_planstate.c index 5f133e4..5c230ee 100644 --- a/src/pg_tracing_planstate.c +++ b/src/pg_tracing_planstate.c @@ -15,6 +15,8 @@ #include "utils/timestamp.h" #define US_IN_S INT64CONST(1000000) +#define INITIAL_NUM_PLANSTATE 10 +#define INCREMENT_NUM_PLANSTATE 5 /* List of planstate to start found for the current query */ static TracedPlanstate * traced_planstates = NULL; @@ -92,7 +94,7 @@ ExecProcNodeFirstPgTracing(PlanState *node) int old_max_planstart = max_planstart; Assert(traced_planstates != NULL); - max_planstart += 5; + max_planstart += INCREMENT_NUM_PLANSTATE; oldcxt = MemoryContextSwitchTo(pg_tracing_mem_ctx); traced_planstates = repalloc0(traced_planstates, old_max_planstart * sizeof(TracedPlanstate), @@ -216,7 +218,7 @@ setup_ExecProcNode_override(QueryDesc *queryDesc) { MemoryContext oldcxt; - max_planstart = 10; + max_planstart = INITIAL_NUM_PLANSTATE; oldcxt = MemoryContextSwitchTo(pg_tracing_mem_ctx); traced_planstates = palloc0(max_planstart * sizeof(TracedPlanstate)); MemoryContextSwitchTo(oldcxt); @@ -232,14 +234,14 @@ setup_ExecProcNode_override(QueryDesc *queryDesc) */ static TimestampTz generate_member_nodes(PlanState **planstates, int nplans, planstateTraceContext * planstateTraceContext, uint64 parent_id, - uint64 query_id, TimestampTz fallback_start, TimestampTz root_end, TimestampTz *latest_end) + uint64 query_id, TimestampTz parent_start, TimestampTz root_end, TimestampTz *latest_end) { int j; - TimestampTz child_end; + TimestampTz last_end = 0; for (j = 0; j < nplans; j++) - child_end = generate_span_from_planstate(planstates[j], planstateTraceContext, parent_id, query_id, fallback_start, root_end, latest_end); - return child_end; + last_end = generate_span_from_planstate(planstates[j], planstateTraceContext, parent_id, query_id, parent_start, root_end, latest_end); + return last_end; } /* @@ -247,12 +249,12 @@ generate_member_nodes(PlanState **planstates, int nplans, planstateTraceContext */ static TimestampTz generate_bitmap_nodes(PlanState **planstates, int nplans, planstateTraceContext * planstateTraceContext, uint64 parent_id, - uint64 query_id, TimestampTz fallback_start, TimestampTz root_end, TimestampTz *latest_end) + uint64 query_id, TimestampTz parent_start, TimestampTz root_end, TimestampTz *latest_end) { int j; /* We keep track of the end of the last sibling end to use as start */ - TimestampTz sibling_end = fallback_start; + TimestampTz sibling_end = parent_start; for (j = 0; j < nplans; j++) sibling_end = generate_span_from_planstate(planstates[j], planstateTraceContext, parent_id, query_id, sibling_end, root_end, latest_end); @@ -264,13 +266,13 @@ generate_bitmap_nodes(PlanState **planstates, int nplans, planstateTraceContext */ static TimestampTz generate_span_from_custom_scan(CustomScanState *css, planstateTraceContext * planstateTraceContext, uint64 parent_id, - uint64 query_id, TimestampTz fallback_start, TimestampTz root_end, TimestampTz *latest_end) + uint64 query_id, TimestampTz parent_start, TimestampTz root_end, TimestampTz *latest_end) { ListCell *cell; - TimestampTz last_end; + TimestampTz last_end = 0; foreach(cell, css->custom_ps) - last_end = generate_span_from_planstate((PlanState *) lfirst(cell), planstateTraceContext, parent_id, query_id, fallback_start, root_end, latest_end); + last_end = generate_span_from_planstate((PlanState *) lfirst(cell), planstateTraceContext, parent_id, query_id, parent_start, root_end, latest_end); return last_end; } @@ -361,7 +363,7 @@ get_parent_traced_planstate_index(int nested_level) TimestampTz generate_span_from_planstate(PlanState *planstate, planstateTraceContext * planstateTraceContext, uint64 parent_id, uint64 query_id, - TimestampTz fallback_start, TimestampTz root_end, TimestampTz *latest_end) + TimestampTz parent_start, TimestampTz root_end, TimestampTz *latest_end) { ListCell *l; uint64 span_id; @@ -374,7 +376,7 @@ generate_span_from_planstate(PlanState *planstate, planstateTraceContext * plans /* The node was never executed, skip it */ if (planstate->instrument == NULL) - return fallback_start; + return parent_start; if (!planstate->state->es_finished && !INSTR_TIME_IS_ZERO(planstate->instrument->starttime)) { @@ -394,7 +396,7 @@ generate_span_from_planstate(PlanState *planstate, planstateTraceContext * plans if (planstate->instrument->total == 0) /* The node was never executed, ignore it */ - return fallback_start; + return parent_start; switch (nodeTag(planstate->plan)) { @@ -406,7 +408,7 @@ generate_span_from_planstate(PlanState *planstate, planstateTraceContext * plans * Those nodes won't go through ExecProcNode so we won't have * their start. Fallback to the parent start. */ - span_start = fallback_start; + span_start = parent_start; break; case T_Hash: /* For hash node, use the child's start */ diff --git a/src/pg_tracing_query_process.c b/src/pg_tracing_query_process.c index aa6ec63..938e5b9 100644 --- a/src/pg_tracing_query_process.c +++ b/src/pg_tracing_query_process.c @@ -105,7 +105,7 @@ parse_traceparent_value(const char *traceparent_str) * Or "SELECT 1 /\*traceparent='00-00000000000000000000000000000009-0000000000000005-01'*\/;" */ void -extract_trace_context_from_query(pgTracingTraceparent * trace_context, +extract_trace_context_from_query(pgTracingTraceparent * traceparent, const char *query) { const char *start_sqlcomment = NULL; @@ -137,7 +137,7 @@ extract_trace_context_from_query(pgTracingTraceparent * trace_context, return; } - parse_trace_context(trace_context, start_sqlcomment, + parse_trace_context(traceparent, start_sqlcomment, end_sqlcomment - start_sqlcomment); } diff --git a/src/pg_tracing_span.c b/src/pg_tracing_span.c index 71f913c..171b3c1 100644 --- a/src/pg_tracing_span.c +++ b/src/pg_tracing_span.c @@ -18,7 +18,7 @@ * Initialize span fields */ static void -initialize_span_fields(Span * span, SpanType type, TraceId trace_id, uint64 *span_id, uint64 parent_id, uint64 query_id) +initialize_span_fields(Span * span, SpanType type, TraceId trace_id, const uint64 *span_id, uint64 parent_id, uint64 query_id) { span->trace_id = trace_id; span->type = type; @@ -69,17 +69,17 @@ initialize_span_fields(Span * span, SpanType type, TraceId trace_id, uint64 *spa void begin_span(TraceId trace_id, Span * span, SpanType type, uint64 *span_id, uint64 parent_id, uint64 query_id, - const TimestampTz *input_start_span_time) + const TimestampTz *start_span) { TimestampTz start_span_time; initialize_span_fields(span, type, trace_id, span_id, parent_id, query_id); /* If no start span is provided, get the current one */ - if (input_start_span_time == NULL) + if (start_span == NULL) start_span_time = GetCurrentTimestamp(); else - start_span_time = *input_start_span_time; + start_span_time = *start_span; span->start = start_span_time; }