* fix(partition): coerce partition exprs against the batch schema before routing
Batch partition routing (MultiDimPartitionRule::split_record_batch) builds
physical expressions via PartitionExpr::try_as_physical_expr, which did no
type coercion. After widening the time index unit (e.g. ms -> ns via
ALTER TABLE ... MODIFY COLUMN ts TIMESTAMP_NS), the persisted partition
bounds keep their original unit, so evaluating the bounds against the batch
failed with:
Invalid comparison operation: Timestamp(ns) < Timestamp(ms)
Row inserts were unaffected because find_region compares Values and
Timestamp::cmp converts units; scans were unaffected because
PredicateGroup::new coerces the region partition expr against the region
schema first.
Coerce the logical partition expression against the evaluation schema in
try_as_physical_expr before creating the physical expression, the same way
PredicateGroup does. This also covers the region partition filter built for
SST pruning (src/mito2/src/sst/parquet/reader.rs), which shares this
conversion.
Fixes#9747
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: regenerate change_col_type result after comment update
The ts_widen_part comment in change_col_type.sql was reworded after the
result file was last generated; sqlness echoes SQL comments into the
result, so the stale comment made the case fail. Regenerate it.
Signed-off-by: Ning Sun <sunning@greptime.com>
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
* feat: add column encoding option and set it for trace span ids
COMPRESSION WITH accepts encoding = 'plain' | 'delta_binary_packed'.
Either disables the dictionary for the column; delta_binary_packed is
limited to integer and timestamp columns.
OTLP trace v1/v2 tables now create span_id and parent_span_id with plain
encoding and 64KiB pages, and timestamp_end with delta_binary_packed.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* test: add compatibility cases for column encoding options
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* test: move column encoding downgrade checks into downgrade_compatibility
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
The flow and nested-MergeScan ingress tests hardcoded "follower" among
the read-preference values that must be rejected, coupling them to the
current leader-only ReadPreference enum. Assert only genuinely invalid
values there, matching the wire-values test which already limits itself
to ["", "unknown"]. This keeps the ingress tests valid for builds that
extend ReadPreference with more variants.
Signed-off-by: Ning Sun <sunning@greptime.com>
* refactor(operator): route shared requests through NodeManager
Remove the concrete NodeClients requester API and use the existing node manager for owned and shared requests. Preserve the original write hint while the legacy error adapter remains in place.
Signed-off-by: WenyXu <wenymedia@gmail.com>
* fix(meta): preserve retry policy through error hints
Propagate client errors through External and let DDL read retry hints while preserving cleanup and terminal-child policies. Region transport failures retain the legacy Retryable classification, which now also participates in bounded write retries.
Signed-off-by: WenyXu <wenymedia@gmail.com>
* fix(operator): restore sparse partition tags when rebuilding writes
Decode retained primary keys when refreshed partition expressions need different tags. Keep the partitioner and multi-region sparse write regression tests with the fix.
Signed-off-by: WenyXu <wenymedia@gmail.com>
* fix(operator): stop detached write retries when callers cancel
Signed-off-by: WenyXu <wenymedia@gmail.com>
* feat(frontend): configure retries across distributed write paths
Wire the shared dispatcher through frontend and logical batching, preserving partition columns and request headers. Keep standalone retries disabled and include configuration, batching, and storage regression coverage.
Signed-off-by: WenyXu <wenymedia@gmail.com>
---------
Signed-off-by: WenyXu <wenymedia@gmail.com>
* feat(datanode): execute nested MergeScan plans on the datanode
Give a datanode the ability to decode and execute a plan whose MergeScan
carries a nested plan (e.g. an inner MergeScan over a build table), so a
distributed sub-plan pushed to a datanode can itself fan out to other
datanodes. This is the execution/codec capability only; it is not wired
to any user-facing switch and produces no plan by itself.
Codec (src/query/src/query_engine/default_serializer.rs):
- MergeScanAwareSerializer keeps the request's SessionState and the
engine catalog manager, and decodes a MergeScan payload's tables by
(catalog, schema, table) name through the engine catalog rather than
the region-aware request catalog list (which binds every table to the
request's region and breaks on mismatched columns).
Datanode (src/datanode):
- New DatanodeRegionQueryHandler (region_query.rs) serves region queries
for the MergeScan nodes of a received plan, resolving region leaders
via PartitionRuleManager and dispatching to peer datanodes.
- Wire KvBackendCatalogManager + PartitionRuleManager + NodeClients into
the datanode's catalog/cache registry (with_dist_planner stays false:
a datanode never rewrites its own plan).
Bug fixes uncovered while wiring this up:
- cache registry: register the table/table_info/table_name/table_route/
partition_info caches the datanode's catalog manager and partition
rule manager require (previously missing -> panic on lookup).
- default_serializer: decode_sub_plan bridge no longer drives a future
on a current-thread runtime from within another executor (spawn a
dedicated thread + runtime when not on a multi-thread runtime).
Tested by query_engine::default_serializer unit tests: nested MergeScan
round-trip and engine-catalog resolution with mismatched column names.
No user-facing behavior change.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(query): keep GreptimeDB function bindings after rebuilding decode state
SessionStateBuilder::new_from_existing collects the function registry
(HashMap) into a Vec and build() re-registers each entry; register_udf
registers aliases before the function's own name and the Vec order is
unstable. Rebuilding the session state to install MergeScanAwareSerializer
after the GreptimeDB functions were registered could therefore rebind a
name/alias (e.g. date_format, an alias of DataFusion's to_char) back to
the built-in function. This path runs for every decoded plan, not only
nested MergeScan ones.
Extract the GreptimeDB function registration into register_greptime_functions
and re-apply it after the final build of both the top-level decode state and
the MergeScan payload state. Add regression tests asserting date_format stays
bound to the GreptimeDB implementation on both paths.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(datanode): address review on nested MergeScan capability
Three follow-ups from review of the nested MergeScan execution capability:
1. Layer the datanode caches (P2b). table_cache derives from
table_info_cache + table_name_cache, and partition_info_cache derives
from table_route_cache, but they were all registered in a single flat
CacheRegistry whose invalidate() runs concurrently with no ordering.
A derived cache could be invalidated and refilled from a base cache
that had not been invalidated yet, leaving stale metadata. Build the
base caches and the derived caches as separate layers of a
LayeredCacheRegistry (reusing the existing layering mechanism) so base
caches finish invalidating before derived ones. Both the production
path (cmd) and the test scaffolding now build the same layered
registry; the flat build_datanode_cache_registry is removed.
2. Don't let a nested MergeScan's internal region query compete for the
concurrency permit (P2a). An outer query holds its permit until its
stream is fully consumed, and a nested MergeScan makes the datanode
issue region queries to peer datanodes as execution stages of that
same query. If such an inner stage also took a permit, the outer query
waiting on it would deadlock against its own held permit (or time out
when max_concurrent_queries > 0). Mark inner DN->DN region queries as
internal stages: QueryRequest gains an flag carried over
the wire in the request header's query_context extensions (reserved
key, not injectable via hints), and the region server skips acquiring
a concurrency permit for internal-stage requests on both acquire
sites. Outer queries still acquire normally.
3. Add an end-to-end capability test
(tests-integration/.../nested_merge_scan_capability_test.rs): hand-build
Join(<local probe region>, MergeScan(build regions on both datanodes))
with mismatched column names and duplicate build keys, ship it to a
datanode over a real TCP Flight endpoint, consume the stream fully and
compare the row multiset against the frontend's reference join result,
assert a real cross-datanode DoGet via an RPC counter, and assert an
inner region failure fails the whole query. Also migrate the
with_real_datanode_grpc_addr + DatanodeRpcStats test scaffolding.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(query): adapt date_format regression test to the updated DataFusion fork
ScalarUDFImpl's trait bounds changed so its impl can no longer be
downcast via as_any(); assert the function equals the GreptimeDB
implementation instead, which to_char can never satisfy.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(datanode): cover the internal stage under the concurrency limit
Add an end-to-end regression test for the nested MergeScan capability
under max_concurrent_queries = 1: both datanodes' only permit is held by
the outer queries' streams while their inner MergeScans fetch the build
regions from the same datanodes over real Flight connections. The result
consumption is bounded by a timeout, so a broken link anywhere in the
chain (marking the inner request internal, carrying the marker in the
request header, honoring it at the receiving acquire sites) fails the
test instead of deadlocking.
Also add a minimal with_datanode_options_override hook to the cluster
builder, and fix the test module doc: the outer request is handed to
RegionServer::handle_remote_read in process; only the inner region
queries go over the wire.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(datanode): cover cold route loads under unrelated invalidations
The table route cache uses InitStrategy::VersionChecked, whose version
counter belongs to the whole CacheContainer rather than to a single
table: any TableId invalidation bumps the shared version, so a cold
load of one table's route retries when an unrelated table is
invalidated concurrently. Document this as a known limitation on the
region query handler, and pin the behavior with an integration test:
run the nested plans while a background task invalidates an unrelated
table id on every datanode, assert the results still match the expected
multiset (retries only add work, they never return a stale route), and
report the observed load amplification via the cache miss counter
(2 loads without invalidations, 8 over three runs with 24 unrelated
invalidations in this run). Also expose the datanode cache registries
on the test cluster so a test can drive the local invalidation path
that the heartbeat handler drives after a metasrv broadcast.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* docs(query): trim nested plan decoder commentary
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor: remove redundant nested scan support plumbing
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(query): decode nested MergeScan payloads asynchronously
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(query): avoid unnecessary extension name allocation
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(query): retain extension decoding for manual session states
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor(datanode): keep nested scan query admission unchanged
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(datanode): honor query handler factories and trim nested scan fixtures
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(datanode): retain required cache error import
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor(query): reuse registered state for nested scan decoding
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(query): verify payload information schema table functions
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(query): retain nested scan runtime and cold-route guards
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor(query): address nested scan review feedback
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(session): preserve read preference across region query hops
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(session): verify strict context rejection at Flow ingress
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test(query): remove redundant serializer error case
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
---------
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test: support plugins and flownodes in integration cluster builder
Allow the meta-srv mock and the tests-integration distributed cluster
builder to install metasrv Plugins and to start flownodes that register
with the mock metasrv, so integration tests can exercise plugin-provided
DDL handlers and flows against a real multi-datanode cluster.
The default path is unchanged: mock()/mock_with_client_channel_config()
keep their signatures, and GreptimeDbClusterBuilder builds the metasrv
plugin-free and starts no flownode unless with_plugins/with_flownodes is
called.
Signed-off-by: discord9 <discord9@163.com>
* test: narrow cluster plugins to metasrv injection
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* test: remove unnecessary plugin container mutability
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
---------
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* feat(log-store): skip corrupted object store WAL segments per region
A segment that does not decode, because its checksum does not match or
its content disagrees with its footer entry, is fetched once more. If it
still does not decode, the read follows `on_corrupted_segment`: `skip`
records a WAL hole for the region with the object key, sequence and the
footer's entry id range, increments
`greptime_logstore_object_store_wal_skipped_segments_total`, logs a
warning and continues with the next object; `fail` fails the read. A
failed fetch fails the read in both modes. Holes are kept in memory per
region and can be listed from the store.
The standalone test builder allocates WAL options through
`build_standalone_wal_provider`, and integration tests run a standalone
instance on the object store WAL across restarts, on S3 when the
`GT_S3_*` variables are set. The standalone shutdown runs every step and
returns the first error.
Signed-off-by: jeremyhi <fengjiachun@gmail.com>
* test(cmd): cover the best-effort standalone stop
Build a standalone instance whose leader services fail to stop and whose
datanode servers fail afterwards, and check that `Instance::stop` still
stops the object store WAL and returns the leader services error.
Rename `skip_segment` to `record_skipped_segment` and the file store
integration test to `test_standalone_object_store_wal_survives_restarts_on_file`.
Signed-off-by: jeremyhi <fengjiachun@gmail.com>
---------
Signed-off-by: jeremyhi <fengjiachun@gmail.com>
* fix: bound decompressed request size and close memory-admission gaps for compressed requests
The request-memory accounting only bounds and charges the encoded bytes on
the wire, but a compressed body can expand far beyond that during
decompression, so tiny requests could allocate disproportionate frontend
memory before any protobuf validation or quota charge.
- Handler-level decompression (Prometheus remote read/write v1+v2, Loki)
now enforces a hard 512 MiB decoded-size cap, checked before any output
buffer is allocated, and charges the decoded bytes to the shared
ServerMemoryLimiter, holding the permits for the lifetime of the
decompressed buffer.
- gRPC requests with transport compression reserve the configured
max_recv_message_size before tonic decompresses, so the decoding phase
is admitted against max_in_flight_write_bytes; the later per-message
charge is skipped to avoid double accounting.
- The HTTP memory-limit middleware keeps its upfront Content-Length charge
but now also accounts the bytes actually streamed beyond it, so chunked
requests and understated Content-Length headers no longer bypass the
aggregate quota.
- Routes that decompress request bodies via RequestDecompressionLayer
(InfluxDB, OTLP, Loki, Splunk, Elasticsearch, pipelines, dashboards)
now charge the decompressed bytes as handlers consume them: the global
middleware marks Content-Encoding requests, and a route-local
accounting layer inside the decompression layer charges the decoded
stream. Plain requests are skipped to avoid double-counting.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: address review comments on memory admission guard lifetimes and 429 mapping
- Retain the gRPC pre-decode reservation for the whole request: the
extensions holding the guard were dropped when the request was consumed
(into_inner), releasing the reservation while per-message charges
stayed skipped. Both the unary and streaming handlers now clone and
hold the reservation marker for the duration of request handling.
- Retain the HTTP body permits across the handler: the AccountedBody
wrapper and the request extensions are dropped once the extractors
finish collecting the body, before the handler is done with the decoded
data. Both accounting middlewares now keep their own accounting handle
alive across next.run(req).await.
- Return a ChargedBuffer from the Loki snappy decompressor so the
reservation outlives the decompressed bytes through the caller's
protobuf decoding, matching the Prometheus path.
- Map mid-stream quota exhaustion to 429 instead of the generic body
error (400): the accounting flags exhaustion and the middlewares
rewrite the extractor rejection, so clients can distinguish
backpressure from malformed input.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: resolve the in-flight body acquisition before trying a new one
A parked acquisition in AccountedBody::charge always belongs to the
currently buffered frame, so it must be resolved before any new
acquisition is tried. Letting the fast-path try_acquire succeed while a
waiter is parked left the waiter alive, and its late completion would
then be credited with a later frame's byte count, under-reserving memory
relative to what was marked charged.
Adds a regression test that reproduces the misattribution: with the
buggy ordering the test delivers a body the quota cannot cover; with the
fix the over-quota frame waits for its own acquisition and times out.
Signed-off-by: Ning Sun <sunning@greptime.com>
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
* feat: allow customized time index unit for metric engine table
* test: provide query tests
* refactor: revert unnecessary change
* refactor: share timestamp unit conversions in api helper
Address review feedback on the time index unit changeset:
- Add shared timestamp_unit/timestamp_datatype helpers to api::helper
(the only crate that sees both proto ColumnDataType and TimeUnit due
to layering; common-time and datatypes have no greptime-proto dep).
This removes the ColumnDataType -> TimeUnit match duplicated between
operator's insert path and the OTLP logs path.
- Collapse the two TimeUnit <-> ValueData matches in
convert_timestamp_value_data by reusing api::helper::to_grpc_value
for the construction side.
- Note that convert_rows_time_unit rewrites the schema before the
values, so an overflow mid-batch leaves the request half-converted;
harmless because the error aborts the whole insert request.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: align time units per destination table and floor remote-read timestamps
Address review feedback on PR #9236:
- Align each metric insert request to the unit of the table it actually
targets: an existing logical table keeps its own unit (it may be bound
to a different physical table than the one selected by the request),
and only new tables use the selected physical table's unit. The
previous blanket conversion rewrote valid millisecond samples to the
selected physical table's unit and the engine rejected them.
Regression test: writing an existing millisecond logical table and a
new table in one request that selects a microsecond physical table.
- Remote read now floors narrowing timestamp conversions towards
negative infinity (div_euclid), consistent with
Timestamp::convert_to on the ingestion path; arrow's cast truncates
towards zero and returned -1ms for a stored -1001us. Widening
(second -> millisecond) keeps the exact arrow cast. Regression test:
a negative, non-aligned timestamp round-trips as -2ms.
Signed-off-by: Ning Sun <sunning@greptime.com>
* perf: fold time unit alignment into existing table lookups
Address review feedback on PR #9236:
- The per-destination unit alignment no longer runs its own pass of
table lookups: create_or_alter_tables_on_demand gains an
align_time_index_unit parameter (metric engine path only) and
converts each request inside the lookups it already performs —
existing tables to their own unit, new tables to the selected
physical table's. Default ingest paths now issue zero additional
catalog lookups compared to main; the separate alignment pass remains
only in the opt-in logical batcher pre-gate, next to the eligibility
check that already looks up the same tables.
- convert_rows_time_unit indexes the time index position directly
(validate_column_count_match guarantees row widths) instead of
Optional get_mut; the gate-side alignment validates widths itself.
Signed-off-by: Ning Sun <sunning@greptime.com>
* perf: resolve the batcher time index guard once per write target
All batches of one remote write request share the same write target
(catalog, schema, physical table), so the batcher time index guard now
resolves each distinct target once instead of once per batch.
Signed-off-by: Ning Sun <sunning@greptime.com>
* feat: support non-millisecond time index units in the logical batcher
Make the logical table batcher's bulk encode path unit-aware so physical
metric tables with a non-millisecond time index (e.g. TIMESTAMP(6)) can
use logical batching instead of falling back to the ordinary insert path.
- rows_to_aligned_record_batch builds the time index column in the
TARGET schema's unit, converting any timestamp encoding via
Timestamp::convert_to (flooring on narrowing, consistent with the
ordinary insert path).
- New tables created by the batcher use the selected physical table's
time index unit (resolved once per submit; a missing physical table
keeps the millisecond auto-create default).
- columns_taxonomy and the can_batch_metric_rows schema whitelist accept
any timestamp unit; the prometheus remote write v1/v2 batcher gates
and the OTLP pre-gate alignment are removed together with
Inserter::align_metric_row_inserts_time_unit, as the batcher now
converts internally.
Closes#9342
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: address review comments
* fix: address review issue
* refactor: drop the OTLP pre-gate unit alignment made redundant by the bulk path
The main merge of #9236 (squash) resurrected the OTLP pre-gate
alignment and Inserter::align_metric_row_inserts_time_unit, which this
branch had removed. Drop them again:
- The pre-gate existed because the #9236-era bulk eligibility gate only
accepted millisecond schemas, so nanosecond-encoded OTLP requests had
to be converted before the check. This branch makes the bulk path
unit-aware (the gate accepts all time index units and batch alignment
converts each request to its destination's unit), so the pre-gate is
redundant and only added N+1 catalog lookups per batched request —
the very lookup-count overhead raised in the #9236 review.
- The per-destination unit semantics it implemented remain enforced in
the two paths that need them: the ordinary insert path
(create_or_alter_tables_on_demand converts inside its existing table
lookups) and the batched path (batch alignment resolves each
destination schema and converts to it).
test_otlp_logical_batcher_alignment (the test the pre-gate originally
fixed) and the mixed-physical-table regression both pass without it.
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: cover OTLP batcher cross-physical fallback and nanosecond physical
Extend the logical batcher integration coverage for the cases
previously guarded by the removed OTLP pre-gate alignment:
- test_otlp_logical_batcher_fallback_for_cross_physical_destination:
with the batcher enabled, an OTLP request targeting an existing
logical table bound to another physical table must NOT enter the
batcher (the bulk eligibility check rejects the destination binding)
and the ordinary insert path must convert it to the destination's
unit (60s -> 60_000_000us).
- test_otlp_logical_batcher_non_millisecond_physical_table now covers
both microsecond and nanosecond physical tables (parameterized),
asserting batcher submissions and unit-precise stored values.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: validate per-batch physical bindings and avoid intermediate timestamp buffers
Address review feedback on PR #9346:
- accepts_bulk_destinations dedupes on (schema, table, selected physical)
instead of (schema, table): one request can select different physical
tables per batch (per-series x_greptime_physical_table labels), and the
old key let a second selection skip validation and flush rows through
the wrong physical's regions. Missing tables additionally reject
conflicting physical selections within the same request. Regression
test covers an existing destination, a missing destination, and a
consistent selection (which must still batch).
- The timestamp column builder appends each value directly into the
target-unit Arrow builder; values already in the target unit (the
unchanged millisecond fast path) are appended without conversion, so
the default millisecond physical pays no Timestamp construction or
intermediate Vec allocation.
- The non-millisecond batching tests assert the submit_build_and_align
counter, which increments on every batcher submission in both
acknowledgement modes, so a silent fallback to ordinary insertion
fails the tests instead of passing on stored values alone.
Signed-off-by: Ning Sun <sunning@greptime.com>
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
* feat: allow customized time index unit for metric engine table
* test: provide query tests
* refactor: revert unnecessary change
* refactor: share timestamp unit conversions in api helper
Address review feedback on the time index unit changeset:
- Add shared timestamp_unit/timestamp_datatype helpers to api::helper
(the only crate that sees both proto ColumnDataType and TimeUnit due
to layering; common-time and datatypes have no greptime-proto dep).
This removes the ColumnDataType -> TimeUnit match duplicated between
operator's insert path and the OTLP logs path.
- Collapse the two TimeUnit <-> ValueData matches in
convert_timestamp_value_data by reusing api::helper::to_grpc_value
for the construction side.
- Note that convert_rows_time_unit rewrites the schema before the
values, so an overflow mid-batch leaves the request half-converted;
harmless because the error aborts the whole insert request.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: align time units per destination table and floor remote-read timestamps
Address review feedback on PR #9236:
- Align each metric insert request to the unit of the table it actually
targets: an existing logical table keeps its own unit (it may be bound
to a different physical table than the one selected by the request),
and only new tables use the selected physical table's unit. The
previous blanket conversion rewrote valid millisecond samples to the
selected physical table's unit and the engine rejected them.
Regression test: writing an existing millisecond logical table and a
new table in one request that selects a microsecond physical table.
- Remote read now floors narrowing timestamp conversions towards
negative infinity (div_euclid), consistent with
Timestamp::convert_to on the ingestion path; arrow's cast truncates
towards zero and returned -1ms for a stored -1001us. Widening
(second -> millisecond) keeps the exact arrow cast. Regression test:
a negative, non-aligned timestamp round-trips as -2ms.
Signed-off-by: Ning Sun <sunning@greptime.com>
* perf: fold time unit alignment into existing table lookups
Address review feedback on PR #9236:
- The per-destination unit alignment no longer runs its own pass of
table lookups: create_or_alter_tables_on_demand gains an
align_time_index_unit parameter (metric engine path only) and
converts each request inside the lookups it already performs —
existing tables to their own unit, new tables to the selected
physical table's. Default ingest paths now issue zero additional
catalog lookups compared to main; the separate alignment pass remains
only in the opt-in logical batcher pre-gate, next to the eligibility
check that already looks up the same tables.
- convert_rows_time_unit indexes the time index position directly
(validate_column_count_match guarantees row widths) instead of
Optional get_mut; the gate-side alignment validates widths itself.
Signed-off-by: Ning Sun <sunning@greptime.com>
* perf: resolve the batcher time index guard once per write target
All batches of one remote write request share the same write target
(catalog, schema, physical table), so the batcher time index guard now
resolves each distinct target once instead of once per batch.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: address review comments
* fix: address review issue
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix(client): complete transport lane isolation
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* refactor(client): deprecate legacy single-manager constructors
Per review: mark the legacy single-manager constructors and helper as
deprecated so callers move to the isolated query/control manager pair.
Tests intentionally exercising the legacy path are annotated with
`#[allow(deprecated)]`.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* fix(client): update deprecated constructor callers for CI
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
---------
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
* feat: serialize struct to json in postgres
* fix: support view scalars and preserve null structs in scalar-to-value conversion
Address PR review:
- Utf8View/BinaryView ScalarValues now convert like their non-view forms
instead of failing row extraction for struct columns
- a null struct scalar converts to Value::Null so a null struct inside a
list stays null in the serialized JSON
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: return errors instead of panics for unsupported arrow field types
Struct-typed query results with arrow field types greptimedb cannot
represent (e.g. Decimal256) used to panic during schema conversion and
row extraction, dropping the client connection. They now surface as
query errors:
- ConcreteDataType::try_from builds struct types fallibly via the new
StructType::try_from_arrow_fields
- Value::try_from(ScalarValue::Struct) uses the same fallible path
- new try_value_from_array converts an arrow element to Value with
error propagation, used by the postgres struct encoding
Signed-off-by: Ning Sun <sunning@greptime.com>
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: cut integration test time and make the storage matrix meaningful
tests-integration is ~85% of workspace test CPU, and 81% of that is the
S3/S3WithCache variants of the HTTP and gRPC suites. Those suites do not
touch the object store: of the 70 matrix HTTP tests only one flushed and
read back an SST, so the matrix was paying real AWS round trips to
re-prove protocol parsing.
- Point the PR CI object-store matrix at the MinIO already started by
tests-integration/fixtures. Three GT_S3_* consumers did not read
GT_S3_ENDPOINT_URL and would have hit real AWS with MinIO credentials;
they now do.
- Add a nightly Linux job against real AWS S3, and pass GT_S3_* into the
release integration-test container. The release previously ran every
remote-backend case as a skip and only exercised the file backend.
- Give each S3WithCache test its own read cache directory. They shared
/tmp/greptimedb_cache, which the datanode wipes on startup, so a
starting test deleted the read cache of a running one.
- Add flush -> read-back assertions to the tests whose columns have a
non-trivial SST representation: JSON/JSON2 columns, native histograms,
metric-engine logical tables, and tables carrying fulltext or skipping
indexes whose puffin files only exist after a flush.
- Move eight tests that create no table out of the storage matrix.
- Make the event recorder flush interval a constructor parameter and
shorten it in the event tests, which otherwise wait a 5s window per DDL
they assert on. It is skipped by serde and never reaches config files.
- Drop duplicates: test_grpc_zstd_compression was a verbatim copy of
test_grpc_message_size_ok and is now rewritten to assert the negotiated
grpc-encoding; test_execute_copy_to_{s3,oss,gcs,azblob} were strict
prefixes of their copy_from siblings; two standalone/distributed event
test pairs shared one assertion body.
- Fix and un-ignore stddev_by_label. stddev_pop merges partial aggregates
in a parallelism-dependent order, so its last digits are unstable; the
test now compares values with a tolerance.
- Rebase the jaeger v1 fixture on the current instant. It carries
ttl=7d with 2025 timestamps, so its rows were only readable as long as
they stayed in the memtable.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
* test: address review — wire nightly real-S3 job into check-status, keep the short event interval
The nightly `check-status` job did not depend on the new real-S3 job, so a
failure there would not have reached the status or Slack notification.
In database_ddl_event the short interval was set by a first
`with_event_recorder_options` call and then overwritten by the pre-existing
one, which carries `..Default::default()`. Merged into a single call.
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
---------
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
Ning Sun
fys
dennis zhuang
Weny Xu
shuiyisong
discord9
jeremyhi
Lei, HUANG
Yingwen
LFC
XuanwoandWenyXu