* perf(telemetry): remove the shared lock from TraceLayer
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* fix(telemetry): stop collecting span data while tracing is disabled
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(servers): stream Prometheus HTTP responses incrementally
Replace try_collect + full-buffer conversion in the Stream branch of
from_query_result with incremental per-batch merging via the shared
merge_batch path, so record batches are dropped as soon as they are
consumed instead of all coexisting in memory.
- Add ColumnLayout to dedupe schema inference shared by both paths
- Add SeriesKeyLookup (Equivalent-based borrow lookup) so series keys
are looked up by borrowed label slices without materializing owned
keys per lookup
- Empty streams short-circuit to empty data before schema inference
- Add stream-side mixed histogram + Vector dual-path equivalence test
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(servers): address review comments on stream response tests
- Inline single-caller tags_capacity helper
- Drop timestamp column from empty-stream test so it pins the
short-circuit happens before schema inference
- Fix cross-batch merge test data so a series actually spans the
batch boundary (second batch carries earlier timestamps)
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(servers): add lazy stream release assertion and pinned cross-batch content
- LazyDropCheckingStream creates one batch per poll and asserts via
Weak that the previous batch was released before the next poll,
protecting the core property that batches are dropped eagerly
- Pin series "a" merged samples to exact timestamps and values in
stream_result_matches_record_batches_result, so a regression in
shared merge_batch logic cannot make both paths agree on wrong output
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
---------
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* perf(prom): release the remote write v1 decode buffer before writing
remote_write_v1 kept the decoded builder alive until the handler returned,
so the decompressed request payload stayed resident across the downstream
write or pipeline await. With 8 concurrent large requests that is one extra
copy of every payload held for the whole write.
Rows and pipeline values own their data, so the builder can be dropped as
soon as the conversion is done.
Sustained-write A/B, 8 runs per side, 50M samples each: jemalloc allocated
median drops 7.8% (155.0-162.7 MiB -> 141.4-158.6 MiB); samples per CPU
second is unchanged (-0.6%, fully overlapping ranges).
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(servers): move row values into SQL JSON responses
The format=json renderer cloned every serde_json::Value and kept the whole
row set alive while building the response. Move the values out instead, so
each row is released as soon as it is converted.
Slicing the row to the schema width keeps the panic on rows narrower than
the schema; a plain zip would silently truncate them. Duplicate column names
still resolve to the last value and extra row values are still ignored.
Isolated conversion measurements: live peak drops 33% on a 4096-row 16 KiB
string fixture and 36% on a nested-JSON fixture, with no measured slowdown.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(prom): release the compressed remote write body after decompression
Both the v1 and v2 decoders held the compressed Bytes until they returned,
which spans the whole protobuf decode and row conversion. Decompression
copies the payload into an independent buffer, so the body can go as soon
as it succeeds.
The compression fallback, decode errors and request counting are unchanged.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* revert(prom): keep the remote write v1 decode buffer until the write finishes
This reverts commit 8232c5b3bb.
The v1 decoder fabricates `&'static [u8]` pointing into its own decode buffer
(prom_remote_write/types.rs), so the compiler checks nothing about that
buffer's lifetime. Holding the builder until the handler returns is what keeps
the decoder safe by construction; releasing it early made that safety depend on
every consumer copying out of the buffer, which holds today but nothing
enforces.
Document the requirement at the binding instead.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(prom): release the remote write v1 decode buffer before writing
This reverts commit ca3af722d0, restoring
8232c5b3bb.
The decoder borrows the decompressed buffer while parsing, but copies
everything out when it builds rows: tag values through
`PromValidationMode::decode_string`, column names through `to_owned`, and the
only live borrows (`TableBuilder::col_indexes`) are dropped inside
`as_insert_requests`. The resulting `ContextReq` holds prost types with no
lifetime parameters, so it cannot reference the buffer.
Record that at the binding so the next reader does not have to re-derive it
from three files.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(otlp): share trace resource and scope attributes
Parsing an OTLP trace request copied the resource attributes and the
scope attributes once per span. A request with wide resource attributes
kept one full copy per span alive until the rows were built.
Spans and their group now share one `Arc` per resource and per scope.
Empty attributes stay `None`, so a resource or scope without attributes
costs no allocation and no refcounting. v0 takes ownership back when it
encodes, v1 clones attributes item by item instead of rebuilding a Vec,
and v2 borrows the span instead of cloning it for every row.
Column values, attribute order, the `service.name` lookup and the
auxiliary table writes are unchanged.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(otlp): stop copying v1 attribute keys
The v1 row writer rebuilds every column name as `{prefix}.{key}`, so the
key string it cloned from the shared resource and scope attributes was
dropped unused. `resource_attributes.service.name` was also cloned in
full before being skipped for the top level service name column.
Shared attributes are now read in place and only values that reach a row
are copied. Span attributes keep moving their values as before.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* fix(prometheus): honor label matchers in __name__ values query
`/api/v1/label/__name__/values?match[]={pod="abc"}` dropped every matcher
other than `__name__` and returned all metrics in the schema. No error,
just the wrong list. Grafana's metrics browser sends this request, so
picking a label value there did nothing.
Selectors that only constrain `__name__` keep answering from table
metadata. A selector constraining an ordinary label now goes to the data:
scan each metric engine physical table for distinct `__table_id` in the
time range, map the ids back to metric names, then apply the selector's
own `__name__` matchers.
Only metric engine tables are covered; other engines share no column space
to scan.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* refactor(prometheus): batch-resolve metric names by table id
Building a full table-id-to-name map meant walking every table in the
schema and holding all of them in memory, just to name the handful the
scan returned. Use `tables_by_ids` instead — one batch KV read over the
ids the scan actually produced.
The catalog walk stays, but only to find the physical tables to scan.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* fix(promql): read an absent label as the empty string
A matcher on a label the series does not carry only worked when the table
had no column for it at all. Where the column exists but is NULL on that
row -- the norm for logical metrics sharing a metric engine physical
table, which holds the union of their label columns -- three-valued logic
dropped the row, so `host!="host1"` and `host=""` missed every metric
without a host label.
Coalesce nullable string label columns to "" for matchers that accept the
empty string, rather than only for the OTLP temporality marker. Equality
matchers are untouched; they cannot match NULL either way.
This is the Prometheus compatibility fix#8970 deliberately kept out of
its own scope. The cost is visible in the regex sqlness plan: the
predicate becomes a CASE, so the scan loses its LastRow selector and
grows a FilterExec. Only negative and empty-accepting matchers pay it.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* fix(promql): don't panic on pre-epoch label value bounds
`rewrite_label_values_query` unwrapped `duration_since(UNIX_EPOCH)`, which
returns an error for an instant before the epoch. `start=1969-12-31T23:59:59Z`
parses as valid RFC3339, so the request panicked instead of answering.
Recover the sign from the error branch, and report a value beyond i64
milliseconds as an error rather than wrapping the cast.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* refactor(prometheus): drop applicable_matchers, share the distinct scan
With the planner reading an absent label as empty, the frontend no longer
needs to pre-filter matchers per physical table. Removing that exposed a
second problem: a physical table that never took a column from a logical
table exposes no `__table_id`, and projecting it failed the whole request.
Skip those tables; the only thing that can miss is a metric with no labels.
Also pulls out the plan-build-execute-collect sequence the two label value
scans had in common.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(servers): prototype ready-only Flight batch coalescing
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(servers): bound ready Flight batch admission and cover lifecycle
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(servers): fix redundant error pattern assertion
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor(servers): simplify Flight coalescing with append-then-check accumulator
Remove the lookahead/pending machinery and coalesce with an
append-then-check soft budget. Extract the group state into a private
BatchAccumulator so the ordinary stream loop reads as a short control
flow: append each ready batch, flush when a budget is reached.
Budgets are flush thresholds, not memory limits, so a group may exceed
a budget by the final appended batch. First-batch direct send and
singleton passthrough for a batch that starts a group already at a
budget are unchanged.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor(servers): split Flight stream into dispatcher and per-path units
FlightRecordBatchStream::flight_data_stream held two large side-by-side
branches (the verbose/ANALYZE metrics path and the plain/coalescing
path) inline, making the function hard to read. Split it into a small
dispatcher that keeps only the shared prologue (schema send, metrics
init) and the shared EOF final-metrics tail, and move each branch into a
self-contained unit:
- verbose path -> Self::verbose_metrics_stream
- plain/coalescing path -> private CoalescingBatcher (owns the
BatchAccumulator, sent_first_batch, and schema)
Each path returns whether it reached normal EOF; the dispatcher skips
the shared final-metrics tail on any early error/failed-send exit,
preserving the exact pre-split behavior (an early return previously
exited the whole function and bypassed the tail). No behavior, metrics
ordering, or coalescing semantics change.
Add a regression test that drives flight_data_stream directly and
asserts the verbose error path does not invoke the final-metrics tail
(producer-side metrics() call count), since the public message stream
hides the tail after an error.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* docs(servers): describe Flight stream units by behavior, not refactor history
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* perf(servers): avoid copying over-budget batches and release merged inputs
Three review-driven improvements to Flight batch coalescing:
- After a successful merge, reuse the drained vector in place
(clear + push the merged batch) so the source batches are dropped
before the send loop instead of staying alive across a backpressured
send.
- A batch that is itself at/over a budget is forwarded as a singleton
even when encountered inside an accumulation group: the accumulated
group is flushed and sent first, then the over-budget batch. This
avoids copying a large batch into an aggregate just to merge the small
batches ahead of it. Under-budget append-then-check is unchanged
(600+600 still merges).
- Strengthen two tests: the first-batch test now drives the producer
directly and asserts the upstream poll count is exactly one at the
first-batch send; the dictionary test now decodes the merged
dictionary keys to assert the logical values, so a key-remapping
regression is caught.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* perf(servers): raise Flight coalesce row budget to 4096
mito2 commonly emits ~2000-row batches (~32-94KiB), so a 1024-row budget
marked every such batch oversized and the coalescer forwarded them
unbatched. Measured on a local distributed cluster (2M-row table):
- full selector: 1000 -> 337 batches, p50 latency 4133 -> 3316 ms (-20%)
- range scan: 501 -> 206 batches, 1466 -> 1162 ms (-21%)
- small/olap queries (single-series, top-k, group-by): unchanged
Larger budgets (8192 rows, or 1-4MiB bytes) coalesce more batches but do
not improve latency further and slightly regress it, so 4096 rows is the
sweet spot; MAX_BYTES stays at 256KiB and MAX_BATCHES at 16.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
---------
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(mysql): strip leading comments before the federated statement filter
JDBC clients prefix every statement with a comment. DataGrip sends
`/* ApplicationName=DataGrip <version> */` in front of each one, and every
pattern in the federated filter is anchored with `^`, so the prefix makes all
of them miss.
Two failures follow. `SET TRANSACTION READ WRITE` reaches the SQL parser and is
rejected. Worse, the DBeaver-specific entries such as
`^(/\* ApplicationName=(.*)SELECT @@(.*))` do match DataGrip's prefix, so
`SELECT @@GLOBAL.event_scheduler` and `SHOW VARIABLES LIKE ...` are absorbed
into a zero-column output, which the MySQL writer sends as an OK packet. The
JDBC driver reports that the statement returned no cursor.
Strip leading whitespace and comments before matching, and drop the
ApplicationName-specific entries the stripping makes redundant. The three that
had no unprefixed counterpart (`SHOW PLUGINS`, `SHOW ENGINES`, `SHOW @@...`)
keep their behaviour as plain patterns.
Also covers gaps found while probing the same path:
- `BEGIN` is absorbed like `START TRANSACTION`/`COMMIT`/`ROLLBACK`.
- `SHOW [GLOBAL|SESSION|LOCAL] VARIABLES|STATUS` parses; the scope is ignored
because GreptimeDB keeps no global/session split.
- `USER()`, `CURRENT_USER()`, `SYSTEM_USER()` and `SCHEMA()` are registered.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* fix(mysql): keep executable comments, reject multi-statement absorption
Review follow-up on the comment stripping, plus the remaining MySQL
compatibility gaps from #9155.
`/*!...*/` is an executable comment: mysqldump emits its initialization as
`/*!40101 SET NAMES ... */`, and the patterns match those verbatim. Stripping
it left an empty statement, so nothing matched and the original SQL reached the
parser, which rejects `SetNames` and `MultipleAssignments`. Leave executable
comments in place.
Absorbing a request also has to stop at a statement boundary, because every
pattern ends in `(.*)`. `BEGIN; INSERT INTO t VALUES (1)` used to fail on the
unsupported `BEGIN`; once `BEGIN` became absorbable the whole request would
report success and write nothing. A request is now scanned for a second
statement and handed to the query engine if it has one. The scan skips plain
comments and string literals so a `;` inside either is not a boundary, and
treats a `/*!...*/` that is not the request itself as a statement, since it
carries SQL.
`check()` now dispatches on the leading statement keyword, so INSERT, UPDATE,
CREATE and ordinary SELECTs run no regex at all — this replaces the narrower
hand-rolled INSERT shortcut. The statement scan runs only for a request the
patterns already matched, which is always a short one.
New compatibility surface:
- `SELECT CURRENT_USER|SESSION_USER|SYSTEM_USER|USER` without parentheses, and
`SELECT @var`, are answered here rather than in the parser. Both are anchored
to the whole statement, so `SELECT user FROM t` still reads the column.
- `information_schema.plugins`, `user_privileges` and `processlist` are
registered as empty tables, like the other MySQL-shape tables around them.
Sessions are reported through `information_schema.process_list` and
`SHOW PROCESSLIST`; `processlist` carries the column shape only.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* perf(servers): group Prometheus response rows by label runs
Query output tends to be clustered by series, but a matrix response read
the same label values out of the tag columns once per row, and allocated
a key vector per row to look the series up.
Use `arrow::compute::partition` to find the runs of rows that share their
labels and build the series key once per run. The key buffer is hoisted
out of the row loop and handed to the map only when the series is new,
through the raw entry API so the key is hashed once either way.
Partitioning does not pay off when rows are not clustered, so a few
adjacent row pairs are probed first to pick between the run path and the
row-by-row path. Both paths produce the same series.
Drop the per-row "same labels as the previous row" check from #8815. Runs
cover the clustered case it was written for, and it now costs more than
it saves: 10% on a result with one row per series, 1-3% on clustered ones.
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
* refactor(servers): find label runs with cmp::distinct
`arrow::compute::partition` computes the same ranges on the same kernel,
but its contract takes lexicographically sorted columns, and query output
is not sorted: range queries run without the plan's output sort since
#9090, `sort`/`topk` order by value, and the tag column order in the
schema does not have to match any sort key. An implementation that
exploited the precondition would merge `a, b, a` into one run and
attribute one series' samples to another, without failing.
`cmp::distinct` is element-wise, so it holds for any row order, and its
null handling is the one a series key needs: a null label and an empty
one are distinct, two nulls are not. Building the ranges from the
boundary bitmask also folds away the `tag_columns.is_empty()` case, since
no columns means no boundaries means a single run.
Same kernel, so the benchmark does not move: -1.4% to +1.4% across shapes
with no consistent sign, against +-3% run-to-run drift.
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
---------
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
* feat(pprof): switch CPU profiler to framehop unwinder
Replace the default libgcc-based unwinder in pprof-rs with the
framehop unwinder, which is designed to be async-signal-safe:
- framehop performs no heap allocation during unwinding
(MustNotAllocateDuringUnwind)
- It handles prologue/epilogue interruption correctly
- It falls back to frame-pointer unwinding when CFI is unavailable
- It does not depend on libgcc's unwind implementation, which is
documented as not signal-safe (see tikv/pprof-rs#36)
Bump pprof from 0.14 to 0.15 in all three consumers (common-pprof,
cmd, servers) to unify on a single version. pprof 0.15 also replaces
parking_lot with spin-rs to avoid a potential profiler deadlock (#268).
This addresses the libgcc_s.so.1 #GP crash observed in production
when CPU profiling is active, by eliminating the libgcc unwinder
from the signal handler path entirely.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* fix(pprof): gate framehop-unwinder to supported targets
framehop-unwinder is only available on x86_64/aarch64 Linux/macOS.
Enabling it unconditionally for all Unix targets breaks the build on
riscv64 and other platforms: pprof disables its backtrace-rs fallback
when framehop-unwinder is set, but the framehop module is not compiled
on unsupported targets, leaving no TraceImpl implementation.
Split the pprof dependency: the base target.'cfg(unix)' block carries
the common features (flamegraph, prost-codec, protobuf), and a separate
target block adds framehop-unwinder only on supported targets.
Cargo unions features from both blocks on matching targets, so x86_64
and aarch64 Linux/macOS get the full feature set while other Unix
targets fall back to the default backtrace-rs implementation.
Addresses review comment discussion_r3987313632.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
---------
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* fix: bump jemalloc crates to 0.7 and patch tikv-jemalloc-sys with tcache init fix
Upgrade tikv-jemallocator / tikv-jemalloc-ctl / tikv-jemalloc-sys from
0.6 to 0.7, which embeds jemalloc 5.3.1 (includes a056c20d 'Handle
tcache init failures gracefully').
On top of that, patch tikv-jemalloc-sys to the GreptimeTeam fork that
adds the remaining upstream fix 54f22c83 'Initialize TSD tcache before
enabling it' (GreptimeTeam/jemalloc#1, GreptimeTeam/jemallocator#1).
Without the ordering fix, a reentrant allocation during TSD bootstrap
(e.g. heap-profiling prof_tdata init / sampled backtrace when prof:true
is active) can observe an enabled-but-uninitialized tcache, corrupting
per-thread tcache metadata and crashing the process in arena_stats_merge,
calloc, or the libgcc unwinder.
The patch is pinned by rev and should be removed once tikv/jemallocator
ships a jemalloc snapshot that includes 54f22c83.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* chore: bump tikv-jemalloc-sys patch rev to merged release-5.3.1
GreptimeTeam/jemalloc#1 has been merged; point the patch at the
jemallocator commit referencing the merge commit on release-5.3.1.
Jemalloc source content is unchanged.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* chore: point tikv-jemalloc-sys patch at GreptimeTeam/jemallocator main
GreptimeTeam/jemallocator#1 has been merged; reference the merge
commit e1846d8c on main instead of the PR head branch. Jemalloc
source content is unchanged.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* fix(servers): bump tikv-jemallocator dev-dependency to 0.7
main added a target.'cfg(not(windows))'.dev-dependencies entry on
tikv-jemallocator 0.6 for servers after this branch diverged. On the
merge ref it pulled tikv-jemalloc-sys 0.6 from crates.io, which
conflicts with the patched 0.7 (links = "jemalloc" may only appear
once in the dependency graph), failing version selection in CI.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
---------
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* perf(servers): drop the output sort for Prometheus range queries
Matrix responses sort samples by timestamp and series by labels while
serializing, so the sort the PromQL planner puts at the root of a range
query plan never reaches the client.
Mark the query as not requiring output ordering in `do_range_query`, and
let the frontend strip the root sort (through projections, and only when
it carries no fetch) before execution. Instant queries are unchanged, so
`sort()` and `sort_desc()` keep their observable order.
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
* test(servers): pin the matrix response against input row order
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
* test(servers): drop redundant response assertions in the ordering test
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
* perf(servers): drop the output sort for gRPC gateway range queries
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
---------
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
Building a matrix response allocated one `String` per sample while the
record batches were scanned, then dropped it after the JSON body was
written. Keep the `f64` in `PromSampleValue::Number` instead and format
it with ryu while serializing, so no per-sample string is allocated.
`PromSampleValue::Text` keeps values parsed from a JSON body, so
deserializing and re-serializing a response is unchanged. Vector and
scalar results still expose `String`, since they hold a single sample.
Signed-off-by: Dennis Zhuang <xzhuang@greptime.com>
* fix(prometheus): align batch flush deadline with creation
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* test(servers): sync pending worker submission with explicit ack
Replace the mpsc capacity() polling in the pending rows batcher deadline
test with a test-only WorkerCommand::Ack round trip. The FIFO channel
guarantees the worker has dequeued and processed the submission (and
anchored the flush deadline) before the test advances virtual time,
removing reliance on an implementation detail that can be flaky under
scheduling variance.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* test(servers): await pending worker flush results with bounded timeout
Replace the try_recv() yield-polling loop in the pending rows batcher
deadline test with a direct await bounded by tokio::time::timeout. Under
paused time the timeout auto-advances the clock and fires
deterministically, so the test fails reliably instead of intermittently
missing the result on slower CI or under contention.
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
---------
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
discord9
Weny Xu
LFC
jeremyhi
shuiyisongandLei, HUANG
dennis zhuang
discord9
Ning Sun
Mohd Quamar Tyagianddennis zhuang
Lanqing Yang
Immanuel Tikhonov
Lei, HUANG
sun