mirror of
https://github.com/quickwit-oss/quickwit.git
synced 2026-10-28 10:45:05 +08:00
main
548
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Luca Cominardi
|
4871a556c7 |
Flush every WAL write to the OS by default (#6926)
The ingest v2 WAL used `PersistPolicy::OnDelay { 5s, Flush }`, which
mrecordlog only checks on the next write. The last frame of an
acknowledged append could stay in the process indefinitely, and recovery
drops a record whose last frame is missing, so a process crash lost the
whole append.
Define `DEFAULT_WAL_PERSIST_POLICY` = `Always(Flush)` and use it
explicitly for every WAL: ingest v2 (the behavior change),
`MultiRecordLogAsync::open` (ingest v1, already `Always(Flush)`) and
`mrecord_bench`. It does not fsync, so host failure or power loss is not
covered.
|
||
|
David Yaffe
|
84f9324395 |
fix(Security): Apply patch update 0.9.2 to main (#6907)
* Restrict Kafka client params that execute commands or load libraries Kafka source configs can be submitted through the REST API, so they should not be able to make librdkafka run shell commands or load native libraries. Kerberos ticket refresh is now opt-in through the operator-set `QW_KAFKA_KINIT_CMD` environment variable. * Bump tantivy to e1acb3e * Bump version to 0.9.2 * Add security note to REST API docs * Treat an empty QW_KAFKA_KINIT_CMD as unset An empty or whitespace-only value now behaves like an unset variable and keeps Kerberos ticket refresh disabled. |
||
|
Luca Cominardi
|
d140cfc4bb |
Use signed i64 generated tie breakers + bump Tantivy (#6882)
* Use signed i64 generated tie breakers Pin Tantivy to quickwit-oss/tantivy#3158 and treat tie_breaker fields as signed integers in query parsing and JSON conversion. Validate the signed generation range, term/range queries, and pagination in both sort directions. * Separate tie breaker pagination coverage from sorting test * Update Tantivy to main after signed tie breakers merge |
||
|
Luca Cominardi
|
8f865ff986 |
Add tie breaker field mapping + Tantivy bump (#6859)
* Add tie breaker field mapping * Update Tantivy lockfile * Bump Tantivy to latest mallets/tiebreaker * Update Tantivy lockfile * Document tie_breaker field type * Pin Tantivy to merged tie-breaker commit * Reject tie_breaker options and tie_breaker additions on doc mapping updates * Clarify tie_breaker uniqueness and update constraints in docs * Limit Cargo.lock changes to Tantivy * Escape dotted tie_breaker paths and ignore tie_breaker values during validation * Test tie_breaker sorting, queries and Elasticsearch mapping * Allow adding tie_breaker fields through doc mapping updates * Convert tie_breaker values with value_to_u64 and use is_sorted_by_key in test * Support term queries on tie_breaker fields * Reject documents providing tie_breaker values in strict mode * Clarify tie_breaker docs and drop _shard_doc recommendation |
||
|
François Massot
|
d18e34ee07 |
Serve MCP endpoint at /mcp instead of /api/v1/mcp (#6863)
MCP negotiates its protocol revision in-band (initialize and the MCP-Protocol-Version header), so nesting it under the versioned REST API adds no value. /mcp matches the convention used by MCP clients and servers. |
||
|
Arthur
|
432d2d1ebc |
Support rolling cluster ID migration (#6852)
Expose extra_cluster_ids and QW_EXTRA_CLUSTER_IDS, use native Chitchat support, and allow configured IDs during gRPC cluster-state exchange. Pin the merged Chitchat HashSet change and cover configuration, membership, ingestion, and search. Implemented with OpenAI gpt-6-astra. |
||
|
François Massot
|
7a0df70a42 | Add read-only MCP endpoint backed by Elasticsearch API (#6858) | ||
|
nadav-govari
|
7b256dd338 |
Remove Parquet code (#6809)
* Remove Parquet code * update 3p licenses * Remove datafusion too |
||
|
PSeitz
|
f2f2a5c7ed |
In memory indexing (#6807)
* Use in-memory directory for indexing * Make in-memory indexing opt-in Default split indexing to MMAP and enable RAM indexing through QW_ENABLE_IN_MEMORY_INDEXING. Persist RAM directories through a controlled MMAP destination and make controlled directories mandatory. * Add node-level indexing write limit Add max_indexing_write_throughput to the indexer configuration and share its limiter across indexing pipelines, including serialization and RAM-to-disk persistence. * Simplify RAM directory memory accounting * Configure indexing write limit via environment |
||
|
nadav-govari
|
13d5e04f65 | Rework readiness/liveness; exit on server failures (#6673) | ||
|
trinity-1686a
|
6a2c5392f4 |
add retry and concurrency limit to gcs storage (#6771)
* add retry layer to gcs * add concurrencty control to gcs * update license file |
||
|
congx4
|
a5d9790530 |
feat(search): use stable node IDs for split affinity to keep disk cache warm when searcher pods restart (#6711)
* feat(search): use stable node IDs for split affinity Rendezvous hashing previously keyed off gRPC addresses, so a searcher restart that kept QW_NODE_ID but changed listen address remapped every split. Hash the cluster node ID instead, while keeping the searcher pool keyed by address for RPC. * refactor(search): rename SocketAddrAndClient to NodeIdAndClient The helper no longer stores a socket address; rendezvous hashing uses node_id only. * refactor(common): drop unused SocketAddrLegacyHash Rendezvous hashing now keys off node IDs, so the pre-Rust-1.81 socket address hash wrapper is unused. |
||
|
David Yaffe
|
9fd01432a7 | fix(docs): update install instructions and the AWS S3 tutorial (#6715) | ||
|
Adrien Guillo
|
30c211f5d7 | Store extra metadata in splits for disaster recovery (#6689) | ||
|
PSeitz
|
7782e2f9fb |
remove component:shortlived metric cache counter (#6675)
* remove component:shortlived metric cache counter * update docs |
||
|
Recoordinate
|
c56125284f | docs: fix typos in upgrades and query-language docs (#6671) | ||
|
PSeitz-ddandPascal Seitz
|
fb99535bcc |
Categorize split search errors in outcome metrics (#6663)
Add bounded error labels for reader creation, warmup, Tantivy search, and panic failures. Expose these categories in documentation and Grafana. Co-authored-by: Pascal Seitz <pascal.seitz@gmail.com> |
||
|
PSeitz
|
e3bed43de5 |
split search failures in outcome metrics (#6662)
Classify storage, warmup, Tantivy execution failures, and panics as category="error" rather than cancellation phases. |
||
|
Deenkar MahulkarandCursor
|
601960219a |
feat: support custom Azure Blob Storage endpoints for sovereign clouds (#6661)
* feat: support custom Azure Blob Storage endpoints for sovereign clouds Add endpoint and endpoint_suffix configuration options so Quickwit can connect to Azure Government, Azure China, Azure Stack, and other non-public blob storage deployments. Fixes #6624 Co-authored-by: Cursor <cursoragent@cursor.com> * chore: apply rustfmt fixes for CI Lints job Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: use quickwit_common env helper in AzureStorageConfig Use get_from_env_opt for Azure storage env var resolution so overrides are logged consistently with the rest of Quickwit. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Luca CominardiandCursor
|
f49ce1ee44 |
Add global metrics labels env var (#6649)
* Add global metrics labels env var Co-authored-by: Cursor <cursoragent@cursor.com> * Name metrics label separators Co-authored-by: Cursor <cursoragent@cursor.com> * Rename metrics labels env var constant Co-authored-by: Cursor <cursoragent@cursor.com> * Move metrics labels docs to CLI reference Co-authored-by: Cursor <cursoragent@cursor.com> * Validate global metrics labels during telemetry startup Reject malformed label configuration before installing recorders so metrics cannot start with silently incomplete global labels. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Adrien Guillo
|
9ad552be68 | Make ingester and compactor decommission timeouts configurable (#6640) | ||
|
Arthur
|
73e4c521ca | Emit metastore max connections metric (#6643) | ||
|
François MassotandClaude Opus 4.7
|
5a9739fcc1 |
Release 0.9.0 (#6316)
* docs(release): CHANGELOG and upgrade guide for 0.9.0 - Flesh out the CHANGELOG [0.9.0] section with the ~400 user-facing PRs merged since v0.8.2: new Breaking / Migration block, ingest v2 as default, AWS Lambda search offloading (distinct from the removed standalone Lambda mode), Jaeger v2, Trino compat, composite aggregation, and many small fixes/changes. - Add docs/get-started/upgrade.md covering the 0.8.x -> 0.9 migration path: ingest v2 default, `rest_listen_port` deprecation, stemming behind `multilang`, metastore migration, rollback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(release): bump workspace version to 0.9.0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(release): refresh 0.9 release notes * docs: fix tag pruning link * docs: sync quickstart index config * docs: fix 0.9 tutorial workflows * chore(release): bump Quickwit UI version to 0.9.0 * docs: correct 0.9 tutorial behavior --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
176f0b29f9 |
Add a metastore read replica role for read-only routing (#6548)
* Add metastore read replica role and config
Introduce the `metastore_read_replica` service role and the
`metastore_read_replica_uri` node config option: the foundation for
routing read-only metastore traffic to a PostgreSQL read replica.
- Add `QuickwitService::MetastoreReadReplica`, parsed from
`metastore_read_replica` / `metastore-read-replica`.
- Split `QuickwitService::default_services()` from `supported_services()`
so the new role is opt-in and never enabled implicitly on all-in-one
nodes.
- Add the optional `metastore_read_replica_uri` field (env
`QW_METASTORE_READ_REPLICA_URI`), redacted alongside `metastore_uri`.
- Validate that the role requires a PostgreSQL `metastore_read_replica_uri`
and cannot be co-located with the `metastore` role.
* Support resolving a read-only PostgreSQL metastore
Add the resolution plumbing for connecting to a PostgreSQL read replica
over a read-only connection.
- Add `MetastoreFactoryOptions { read_only }` and thread it through
`MetastoreFactory::resolve`.
- Add `MetastoreResolver::resolve_read_only`, which rejects any non-
PostgreSQL backend.
- Key the PostgreSQL factory cache on `(uri, options)` so the read-write
and read-only clients get distinct connection pools.
- Add `PostgresqlMetastore::new_read_only`: a read-only connection pool
with migrations skipped (the replica is migrated by the primary).
* Serve a read-only metastore for the read replica role
Resolve and expose a metastore gRPC server when the
`metastore_read_replica` role is enabled, backed by a read-only
connection to `metastore_read_replica_uri`.
- The read replica server reuses the metrics + load-shed layers but omits
the control-plane event layers, which only wrap write RPCs.
- A read-replica node is exempted from the control-plane connectivity
wait, like a primary metastore node, so dedicated replica pods start
independently.
- Extract `metastore_max_in_flight_requests` shared by both roles.
* Route search-path metastore reads to the read replica
Add `ReadReplicaRoutingMetastore`, a `MetastoreService` wrapper that
routes the stale-tolerant reads issued by the search and analytics paths
(`index_metadata`, `indexes_metadata`, `list_indexes_metadata`,
`list_splits`, `list_metrics_splits`, `list_sketch_splits`) to read
replica nodes when any are connected, and everything else (writes and
non-hot-path reads) to the primary.
- Routing is decided per request from the read replica balance channel's
live connection set, so it degrades to the primary when no replica is
deployed. The check is a synchronous `watch` read with no borrow held
across an await.
- Wire it into the searcher service and the DataFusion session builder, so
all search (REST, Elasticsearch, gRPC) and metrics analytics benefit,
while REST admin handlers keep read-your-writes against the primary.
* Document the metastore read replica and test read-only resolution
- Document `metastore_read_replica_uri` in the node config reference and
the example `quickwit.yaml`.
- Add a PostgreSQL-gated test that resolves a read-only metastore against
a real database and verifies it serves read RPCs.
* Use a read-only MetastoreReadService trait for read routing
Replace the full `MetastoreService` implementation on
`ReadReplicaRoutingMetastore` (which forced ~45 delegating methods, most
of them writes) with a narrow, read-only `MetastoreReadService` trait —
the read-only subset of the metastore RPCs (`index_metadata`,
`list_indexes_metadata`, `list_splits`, `list_metrics_splits`,
`list_sketch_splits`).
- `MetastoreServiceClient` implements `MetastoreReadService`;
`MetastoreReadServiceClient = Arc<dyn MetastoreReadService>`.
- `ReadReplicaRoutingMetastore` now implements only the 5-method trait, so
writes are excluded at the type level rather than delegated.
- The search and DataFusion read paths take `MetastoreReadServiceClient` /
`&dyn MetastoreReadService`; `list_parquet_splits_*` take
`&dyn MetastoreReadService`. `single_node_search` keeps its concrete
`MetastoreServiceClient` parameter and adapts internally.
Addresses review feedback that the wrapper should expose a Go-style
read-only interface instead of reimplementing the whole service.
* Avoid primary fallback for read replica routing
* Fix clippy warning in read replica config validation
* Add searcher metastore read replica setting
* Remove config URI validation for read replica
* Fix metastore gRPC mount comment
* Update metastore readiness checks
* Require metastore read replica standalone role
* Remove unnecessary unit tests
* Refine metastore read replica wiring
* Fix default service startup in tests
* Simplify metastore read replica validation
* Trim metastore read replica config tests
* Classify read-only Postgres errors
* Fix search test clippy warnings
* Simplify readiness test mock setup
* Remove metastore read service
* Default metastore read replica routing to false
* Update Quickwit env var index test
* Exclude read replica role from TLS test nodes
* Use ensure for metastore checks
* Simplify read replica URI assertion
* Test read-only Postgres writes
* Use service name in metastore connection log
* Use read replica metastore directly
* Fix read replica metastore readiness
* Refactor local metastore server handling
* Validate metastore read replica searcher config
* improve errors
Co-authored-by: Adrien Guillo <adrien.guillo@gmail.com>
* make test deterministic
* move metastore to own file
* Cache PostgreSQL metastore environment flags
* Make read replica readiness selection explicit
* Format PostgreSQL metastore options
* Fix read-only resolver error assertion
* add more logging
* Apply service overrides before config validation
* Label metastore metrics by kind
---------
Co-authored-by: PSeitz-dd <pascal.seitz@datadoghq.com>
Co-authored-by: Adrien Guillo <adrien.guillo@gmail.com>
Co-authored-by: Pascal Seitz <pascal.seitz@gmail.com>
|
||
|
François MassotandPaul Masurel
|
934bab3088 |
Metastore inflight default (#6591)
* Tune PostgreSQL metastore in-flight requests * Simplify metastore in-flight sizing change --------- Co-authored-by: Paul Masurel <paul@quickwit.io> |
||
|
Adrien Guillo
|
6635a03dfd |
Rename cert_reload_interval to cert_poll_interval
|
||
|
Adrien Guillo
|
9e38033b09 | Expose health check API on dedicated HTTP server (#6528) | ||
|
Adrien Guillo
|
9a060dd5a2 | Hot-reload TLS cert periodically or on SIGHUP (#6508) | ||
|
nadav-govari
|
f5f5cebde0 | Use CRC32C for checksums instead of MD5 (#6442) | ||
|
George TalbotandClaude Opus 4.7
|
43b034511c |
feat(merge-executor): YAML flag to route regular merges through streaming engine (default off, no-op rollout) (#6441)
* test(merge): engine-parity tests + share MS-7 serial lock Adds `merge::tests::parity` with two tests that run the same realistic input fixture through both `merge_sorted_parquet_files` (in-memory engine) and `execute_merge_operation` (streaming engine over the same `LocalFileByteSource` the executor uses in production), then asserts row-by-row equivalence on every visible column. These gate the upcoming YAML flag that flips regular merges to the streaming engine: parity must hold before the default is flipped in production. The streaming engine writes a process-global atomic (`PEAK_BODY_COL_PAGE_CACHE_LEN`) that the MS-7 tests reset-then-read. Any test that runs a streaming merge must serialise against MS-7 or inflate its readings. Move `ms7_serial_lock` from the streaming-tests submodule to module scope (still `#[cfg(test)] pub(crate)`) so the new parity tests acquire the same lock. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(merge-executor): YAML flag to route regular merges through the streaming engine Wires the streaming Parquet merge engine into the regular (non-promotion) merge path behind a node-level YAML flag, `indexer.parquet_merge_use_streaming_engine`, defaulted to false. When true, `ParquetMergeExecutor::handle` runs every merge through `execute_merge_operation` (the column-major, page-bounded streaming engine) instead of the in-memory `merge_sorted_parquet_files`. Promotion merges (`target_prefix_len_override.is_some()`) continue to take the streaming path unconditionally — the in-memory engine can't handle mixed `rg_partition_prefix_len` inputs. The in-memory engine stays in place as the runtime fallback. If the streaming engine hits a bug in production, an operator can flip the flag back to `false` via YAML without redeploying. Once the streaming path has soaked, the fallback branch and `merge_sorted_parquet_files` itself can be removed. The flag is plumbed `IndexerConfig` → `IndexingService` → `ParquetMergePipelineParams` → `ParquetMergeExecutor::new`, and exercised end-to-end by the engine parity tests in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(config): exercise parquet_merge_use_streaming_engine in YAML/JSON/TOML fixtures Adds the new flag to the YAML, JSON, and TOML node-config test fixtures and bumps the expected `IndexerConfig` in `node_config_parse_*` to `parquet_merge_use_streaming_engine: true`. Catches parse / serde regressions on the field — e.g., a rename or a default-fn typo would fail the test instead of silently parsing as `false`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(merge-pipeline): end-to-end streaming-engine flag verification Adds `test_merge_pipeline_end_to_end_with_streaming_engine_flag`, an integration test that runs the full actor chain (planner → downloader → executor → uploader → publisher) with `ParquetMergePipelineParams::use_streaming_engine = true`. Asserts: 1. Publish fired with the right replaced_split_ids (merge ran end-to-end through the executor). 2. `PEAK_BODY_COL_PAGE_CACHE_LEN > 0` after the merge. The streaming engine increments this on every body-col page assembly; the in-memory engine never touches it. Non-zero is direct evidence the streaming path executed — not a silent fallback to in-memory. 3. The merge output row count and metric names are correct. To make assertion (2) work cross-crate, exposes `PEAK_BODY_COL_PAGE_CACHE_LEN` as `pub` under `#[cfg(any(test, feature = "testsuite"))]`. The visibility widening is test-only — production builds never see the symbol. This is the closest analog to the sesh-mode "production-path" rule that is feasible today: the metrics pipeline's OTLP gRPC ingest path is not yet wired into `quickwit-serve`, so the closest end-to-end test is the actor-chain integration test that this PR adds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(adr): record dual Parquet merge engines as deviation #001 Captures the intentional, time-bounded divergence from ADR-003 §4 introduced by the streaming-engine wire-in: two engines coexist in production behind `IndexerConfig::parquet_merge_use_streaming_engine`, with the in-memory engine retained as the runtime fallback. Documents: - The ADR-003 §4 quote the deviation diverges from (page-granular streaming, bounded memory). - The current dual-engine implementation and routing logic. - Why this exists (production safety, staged rollout, parity is strong-but-not-total). - Explicit exit criteria: default flipped to `true`, ≥ 2-week production soak with no merge-correctness incidents, no rollback. When met, a follow-up PR deletes the in-memory branch and engine, the flag, and the parity tests. This is the first deviation recorded under the EVOLUTION.md framework. Indexes the doc in `deviations/README.md`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(merge-pipeline): share full correctness contract between both engine tests Extracts the steps-5-through-8 assertions (replaced_split_ids, staged metadata, Parquet file content, Parquet KV headers) into `assert_cpu_mem_merge_outputs_correct` and calls it from both `test_merge_pipeline_end_to_end` (in-memory engine) and `test_merge_pipeline_end_to_end_with_streaming_engine_flag` (streaming engine). The streaming-engine test had been doing only a small subset of the checks — row count and metric names. It now runs the full contract: time_range, num_merge_ops, sort_fields, row_keys_proto, zonemap_regexes, low_cardinality_tags, all 100 timestamps, sorted_series monotonicity, cpu/mem sort-order semantics, and every `qh.*` Parquet KV header. By construction both engines must produce a file that satisfies the same contract — the helper is the executable parity between engines at the pipeline-integration level, complementing the column-level parity tests in `quickwit-parquet-engine::merge::tests::parity`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(merge-pipeline,merge-engine): multi-metric + multi-RG + m:n disjointness Expands test coverage along three axes the existing helpers didn't hit: 1. **Multi-input, multi-metric pipeline tests** (new file `parquet_merge_pipeline_multi_metric_test.rs`). Three inputs, each carrying three metrics with overlapping per-metric timeseries IDs and overlapping-but-distinct timestamps — the merge must row-by-row interleave across all three inputs. Output writer uses `row_group_size = 50` so the 180-row merge output breaks into four row groups, exercising the writer's multi-RG path in both engines. Both engine variants (in-memory + streaming) covered. Streaming-engine test asserts `PEAK_BODY_COL_PAGE_CACHE_LEN > 0` to confirm the flag routed through the streaming path. 2. **Engine-level multi-output contract** in `merge::tests::parity::assert_engine_parity`. Beyond the existing engine-vs-engine column equivalence, every parity test now also verifies on the in-memory engine's outputs (equivalent to the streaming engine's): sum of per-output row counts equals total input rows, each output internally monotonic on `sorted_series`, and across outputs the partition is disjoint (no two outputs share any `sorted_series` value). This is the m:n non-overlap contract. 3. **Multi-metric overlapping-input m:n** test `parity_multi_metric_overlapping_inputs_multi_output` exercises the strengthened contract with three inputs × three metrics where per-metric keyspaces overlap across inputs. n = 3 outputs target. Honest scope note in the new pipeline test module's doc: the actor pipeline today hardcodes `num_outputs = 1` in `ParquetMergeExecutor`, so n > 1 is not reachable end-to-end through the actor system. The new engine-level test covers the n > 1 correctness contract for now; when the executor is taught to accept `num_outputs > 1` from the merge policy, the pipeline tests can grow an n > 1 variant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(merge-executor): compute num_outputs from target_split_size_bytes Replaces the hardcoded `MergeConfig { num_outputs: 1, ... }` in `ParquetMergeExecutor::handle` with a per-merge computation: num_outputs = max(1, ceil(total_input_bytes / target_split_size_bytes)) So a merge that ingests more than one target's worth of data spreads across multiple output files; merges that fit in one target keep producing a single output (preserving today's behavior for the common case). The engine clamps the request to the number of `sorted_series` boundaries actually available, so the value is an upper bound, not an exact count. Plumbing: `IndexerConfig` already carries `target_split_size_bytes` in `ParquetMergePolicyConfig`. Pass that through `ParquetMergePipelineParams.target_split_size_bytes` → `ParquetMergeExecutor::new`. Default for tests: `256 * 1024 * 1024` (matches the production default). Latent multi-output bug fixed at the same time: with n>1, the executor used to assign the planner-supplied `merge_split_id` to **every** output split, which would have collided on the rename to `{split_id}.parquet`. First output keeps the planner ID for observability continuity; subsequent outputs use the fresh IDs generated by `merge_parquet_split_metadata`. Also exposes `quickwit_parquet_engine::merge::streaming::ms7_serial_lock` as `pub` under the `testsuite` feature so cross-crate streaming tests (in `quickwit-indexing`) can serialise against the same global lock the in-crate MS-7 tests use. The streaming engine writes to a process-global atomic on every merge — without shared locking, the existing pipeline streaming-engine test races `store(0)` against other tests' merges. Adds the appropriate `#[allow(clippy::await_holding_lock)]` to the in-crate `test_merge_pipeline_end_to_end_with_streaming_engine_flag` to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(merge-pipeline): bonus — prefix_len=1 multi-RG inputs + m:n outputs Adds the bonus scenario: three multi-metric inputs each written with `rg_partition_prefix_len = 1` and one row group per distinct metric_name (via `row_group_size = ROWS_PER_METRIC_PER_INPUT` so the writer flushes at every metric boundary after sorting). Merged with a small `ParquetMergePipelineParams::target_split_size_bytes = 500` that forces the executor's `num_outputs` calculation to ask the engine for multiple outputs — exercising the m:n merge path now reachable through the actor pipeline (PR's earlier commit removed the `num_outputs = 1` hardcode). Both engines covered: - `test_prefix_aligned_multi_metric_three_input_multi_output_in_memory_engine` - `test_prefix_aligned_multi_metric_three_input_multi_output_streaming_engine` The streaming-engine variant also asserts `PEAK_BODY_COL_PAGE_CACHE_LEN > 0` (under `ms7_serial_lock`) so a silent fallback to the in-memory path would fail. The shared assertion helper `assert_three_input_three_metric_multi_output_correct` checks the m:n contract end-to-end at the pipeline level: - All three input splits replaced. - ≥ 2 output splits staged (proves splitting happened). - Sum of per-output row counts = total input rows. - Each output internally monotonic on `sorted_series`. - Across outputs, the `sorted_series` partition is disjoint — no two outputs share any key, which is the "non-overlapping output" contract the engine promises. - Union of metric_names / services across outputs = full input set. - Every output has `num_merge_ops = 1`, `row_keys_proto`, and a `metric_name` zonemap regex. To pin the test to exactly one merge (not a cascade of merges over the now-multiple staged outputs), `make_pipeline_params` now takes `max_merge_ops` and the bonus tests set it to `1`: outputs land at `num_merge_ops = 1`, equal to the policy ceiling, and the planner refuses to merge them again. The existing n=1 tests stay at 5 (headroom — they produce a single output that can't trigger another merge anyway, since `merge_factor = 3`). Updates the module doc to drop the now-stale scope note about m:n not being reachable through the pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: nightly rustfmt + drop useless borrows in assert! Reformats doc comments / format strings under nightly rustfmt (`wrap_comments`, `format_strings`), and removes two redundant `&` in `assert!` arguments flagged by clippy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
George TalbotandClaude Opus 4.7
|
ead4848593 |
feat(merge): legacy promotion path + body-col schema evolution (#6423)
* fix(merge): adapter rejects unsorted input; consumer honors SS-3; stronger test verifiers Three adversarial-review findings on the prefix/RG machinery, bundled because they touch the same producer/consumer contract: **F8: Legacy adapter rejects SS-1-violating input upfront.** The adapter walked rows in physical order and emitted one RG per prefix-value run. An unsorted legacy input (rows `[A,A,B,B,A,A]`) produced a 3-RG file where two RGs shared prefix `A`, violating PA-3. The streaming merge engine would later reject it mid-merge — but only after a quietly-bad file had been built. Now `compute_prefix_value_slices` tracks each slice's composite prefix-value bytes and bails with `LegacyAdapterError::InputNotSorted` on duplicates, surfacing the SS-1 violation before any file lands on disk. **F12: Consumer-side SS-3 (cross-layer divergence, discovered while wiring F2's chunk-level verifier into the SS-3 test).** The adapter implements SS-3 correctly (missing-from-schema → synthesized NullArray during slice computation, file stamps `prefix_len = N`). The streaming engine's reader did not: `find_prefix_parquet_col_indices` hard-required every named prefix column to be physically present, so a file the adapter produced from an SS-3 input was unreadable by the merge engine. Now `find_prefix_parquet_col_indices` returns `Vec<Option<PrefixColumn>>` and `extract_rg_composite_prefix_key` emits a constant null marker (`encode_byte_array_prefix(&[])`) for None slots. The column contributes no cross-RG ordering signal (constant everywhere) so region boundaries are driven entirely by the present columns. Both halves of SS-3 now agree end-to-end. Known limitation: cross-file SS-3 — where some inputs have a sort column and others don't — uses [0x00, 0x00] for the null contribution, which sorts BEFORE non-null per the encoded-empty-string convention. That weakly violates SS-2 (nulls sort last). Single-file SS-3 is correct because every RG in such a file contributes the same constant. If cross-file SS-3 becomes a production scenario, the encoding needs a leading-0xff sentinel instead. Not exercised today. **F2/F9/F11: Wire `assert_unique_rg_prefix_keys` into prefix-claiming tests.** Tests asserting `num_row_groups == N` + KV stamped to N would have passed even with an off-by-one in slice-boundary detection or column-content scrambling. The verifier reads chunk-level statistics directly: PA-1 (intra-RG `min == max`) + PA-3 (inter-RG uniqueness) on the composite key. Wired into six tests: - streaming engine: `test_streaming_merge_with_prefix_len_two`, `test_multi_rg_metric_aligned_input_produces_multi_rg_output`, `test_streaming_merge_with_desc_prefix_col` - legacy adapter: `test_target_prefix_len_two_splits_by_metric_and_service`, `test_legacy_input_with_sort_fields_produces_prefix_aligned_multi_rg`, `test_missing_prefix_col_treated_as_null_satisfies_alignment` (now passes thanks to F12). Also: `assert_unique_rg_prefix_keys` no longer short-circuits on single-RG files — they still go through PA-1 because an unsorted single-RG file CAN have `min != max` on a prefix column. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(merge): legacy-prefix promotion path + schema-evolution body cols Two adversarial-review follow-ups grouped because they share the streaming engine's input-routing and union-schema seams. ## (b) Legacy-prefix promotion A new operation type pairs a prefix_len=0 split with prefix_len>0 peers in one merge, so legacy splits can be folded into prefix- aligned buckets instead of aging out via retention. Adds: - `ParquetMergeOperation::promote_legacy(splits, target_prefix_len)`: relaxes MP-3 to allow mixed `rg_partition_prefix_len` as long as every input is `<= target`. Sort_fields + window equality unchanged. - `ParquetMergeOperation::target_prefix_len_override: Option<u32>` field records the promotion target; `None` is the default regular-merge form. - `merge_parquet_split_metadata(..., mixed_prefix_ok)`: skips the input-side prefix-len equality check in promotion mode. The output prefix_len still comes from the writer's KV stamp via `MergeOutputFile.output_rg_partition_prefix_len` (CS-1 holds by construction post-F1). - `merge::execute_merge_operation(op, sources, ...)`: new thin executor that opens each input as either `LegacyInputAdapter` (when `split.rg_partition_prefix_len < target`) or `StreamingParquetReader` (otherwise), then feeds them to the streaming engine. Becomes the seam PR-7 will wire from above. Tests: - `test_promote_legacy_pairs_legacy_with_aligned_peer`, `test_promote_legacy_rejects_higher_prefix_input`, `test_promote_legacy_still_enforces_sort_fields`, `test_promote_legacy_all_at_target_is_valid`. - `test_mixed_prefix_ok_skips_input_equality_check`. - `test_promote_legacy_executor_end_to_end`: legacy single-RG + aligned multi-RG → 3-RG output passing `assert_unique_rg_prefix_keys` with `prefix_len = 1`, plus metastore CS-1. - `test_executor_mismatched_sources_count_bails`. ## F6 + F13: Schema evolution for body columns The merger now supports MC-4 across heterogeneous body-col schemas: - F6: `normalize_type` collapses `Binary`/`LargeBinary` (and dict variants) to `Binary`, analogous to the existing string-flavour collapse. Two inputs whose body col differs only by byte-array flavour merge cleanly; before this they hit a "type conflict" at alignment time. - F13: `streaming_writer.rs::write_list_via_serialized_column_writer` (renamed from `..._non_nullable_...`) now handles nullable outer `List<T>` / `LargeList<T>`. MC-4 forces the union to be nullable when a List col is present in only some inputs; before this the writer rejected the merged output. Uses Dremel max_def_level = 2 (0 = outer null, 1 = empty list, 2 = element present) for nullable outer; non-nullable path unchanged. Test: `test_mc2_mixed_schemas_round_trip` builds two inputs A and B with the same sort schema but different body cols (Utf8 vs Dict<Utf8>, LargeBinary vs Binary, List<Float64> in A only, Int32 A-only, Int64 B-only, common Float64). The merge produces the union schema; per-row rendering via `render_cell` matches across flavour boundaries; List cells from B render as nulls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(indexing): re-fmt parquet_merge_executor to latest nightly rustfmt Same nightly-rustfmt drift as the storekey commit on #6424 (local nightly 2026-05-11 vs CI's 2026-05-17): the `mixed_prefix_ok` binding and the `merge_parquet_split_metadata` call now fit single-line under the newer width heuristics. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(merge-executor): route promotion merges through execute_merge_operation Codex P1 on PR #6423: the executor unconditionally called the in-memory `merge_sorted_parquet_files` path, which routes through `extract_and_validate_input_metadata` and bails on mixed `qh.rg_partition_prefix_len` before any output is produced. So a real promotion merge — `prefix_len = 0` plus `prefix_len = 1` with `target_prefix_len_override = Some(1)` — failed before reaching the downstream `mixed_prefix_ok` plumbing in `merge_parquet_split_metadata`. The escape hatch existed but was unreachable for actual promotion inputs. Fix: branch in the executor's handle on `target_prefix_len_override.is_some()`. Promotion merges go through the engine's streaming entry point `quickwit_parquet_engine::merge::execute_merge_operation`, which opens each below-target input via `LegacyInputAdapter` and each at-target input directly. The streaming merge then sees a homogeneous stream advertising `prefix_len = target` on every input. Regular (non-promotion) merges keep the in-memory path. `execute_merge_operation` expects `Vec<Arc<dyn RemoteByteSource>>` parallel to `op.splits` — the engine deliberately doesn't depend on `quickwit-storage` (would invert layering and pull cloud SDKs into a pure parquet library). So this commit adds `LocalFileByteSource`: a tiny `RemoteByteSource` impl backed by `tokio::fs::File`, one instance per downloaded split, each bound to its scratch-directory path. The `path: &Path` argument on the trait surface is ignored — the downloader has already resolved each split to a concrete local file before the executor runs. Coverage: - Library-level: `quickwit-parquet-engine::merge::streaming::tests::test_promote_legacy_executor_end_to_end` already exercises `execute_merge_operation` with a `prefix_len = 0` + `prefix_len = 1` pair, verifying the output advertises `prefix_len = 1` and passes PA-1 + PA-3 on the composite key. That's now the same code path the in-tree executor takes. - Module doc on the executor rewritten to spell out which path runs when. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(adr): track legacy promotion planner gap as GAP-011 The streaming Parquet merge stack landing in #6424–#6428 ships the full legacy-promotion *mechanism* (engine + adapter + executor wiring) but not the planner-level *trigger*. In production today, `MergePolicyState::record_split` buckets by `CompactionScope::from_split` which includes `rg_partition_prefix_len`, so legacy (prefix=0) and aligned (prefix>0) splits are separated before `ParquetMergePolicy::operations` runs. The policy only emits `ParquetMergeOperation::new`; a repo-wide search finds `promote_legacy` only in tests. Legacy splits therefore never migrate without an explicit trigger. Tracking this as GAP-011 so we pick it up at the right time. The gap doc walks three resolution options (merge buckets in the scope key, dedicated promotion pass, or hybrid prefer-multi-input-promotion) and the cost trade-offs between them, so the eventual implementation PR has a starting point. Raised by Codex review comment id 4311184497 on PR #6423. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(adr): track download-vs-streaming merge executor gap as GAP-012 The Parquet streaming merge engine is built around `RemoteByteSource` and was designed to pull pages directly from object storage — two GETs per input, overlap fetch with merge, no scratch disk. The production actor pipeline doesn't take that path: a downloader actor materializes every input on local disk first, and the executor wraps the local files in a `LocalFileByteSource` to feed `execute_merge_operation` (or just calls the in-memory `merge_sorted_parquet_files` path). The streaming engine's central design benefit is unused. This isn't a correctness bug — both paths give the same result. It's a perf/architecture gap: every merge pays 2× I/O per input (network → scratch + scratch → merger), serializes phases (`max(input download time)` first-byte latency), and consumes scratch disk that scales with concurrent merges. Tracking as GAP-012 so we pick it up at the right time. The gap doc walks four options (stream-directly with download fallback, stream- by-default with circuit breaker, eliminate in-memory path only, stream-directly for promotion merges only) and the trade-offs between them — including the mid-merge retry surface, which is the main reason download-first is the current default. Surfaced during PR #6423 code walkthrough. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Adrien Guillo
|
a2dd31c1d2 |
Remove telemetry (#6431)
* Remove telemetry * Update LICENSE-3rdparty.csv |
||
|
George TalbotandClaude Opus 4.7
|
8f504e366c |
test: Parquet merge pipeline verification suite (#6369)
* test: add proptest for planner maturity filtering + shared test helpers Property test verifies that no merge task ever contains a split that should have been filtered: mature by ops (>= max_merge_ops), mature by size (>= target), time-matured (created_at + maturation_period <= now), or missing a window. Generates random splits across the maturity boundary and tests at the actor level. Also makes test helpers pub(super) so sibling test modules (sketch, crash, multi-round) can reuse them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add sketch split integration test for Parquet merge pipeline Verifies that sketch splits dispatch to the correct metastore RPCs (stage_sketch_splits, publish_sketch_splits) and that the merged output has ParquetSplitKind::Sketches with correct metadata. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add crash/restart and multi-round merge integration tests Crash/restart test: - Injects publish failure on 2nd call → pipeline detects failure, kills actors, respawns - Verifies list_metrics_splits called on respawn (re-seeding) - Verifies pipeline generation >= 2 (respawn occurred) - Verifies original splits eventually replaced (no data loss) Multi-round merge test: - 4 input splits → 2 round-1 merges → 1 round-2 merge - Verifies num_merge_ops=2 on final output - Verifies all original + intermediate splits replaced - Verifies MC-1: total rows preserved across lifecycle Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * spec: TLA+ specification for merge pipeline shutdown drain protocol Models the two-phase shutdown: DisconnectMergePlanner breaks feedback, RunFinalizeMergePolicyAndQuit drains cold windows, then in-flight merges complete. Safety invariants: - NoSplitLoss: every merge input is published or in-flight - NoDuplicateMerge: no split in multiple concurrent merges - FinalizeWithinBound: at most MaxFinalizeOps finalize operations - ShutdownOnlyWhenDrained: shutdown requires empty in-flight set Liveness: - ShutdownEventuallyCompletes under weak fairness Two configs: _small (hundreds of states) and full (larger space). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add Stateright DST model for merge pipeline with crash/restart Model checks 5 properties across ingest, merge, crash, and restart: - MC-1 lifecycle: total rows preserved (no loss/duplication) - Bounded WA: merge_ops never exceeds max_merge_ops - No duplicate merge: no split in multiple concurrent merges - No orphan after restart: all immature splits re-seeded - MP-1: level homogeneity (by construction) Small model (~instant), full model (~seconds, gated behind #[ignore]). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: nightly rustfmt import ordering across all new test files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: update TLA+ CLAUDE.md with macOS setup and spec catalog - Document the actual tla2tools.jar path from brew cask tla+-toolbox - Add "Run All Specs" one-liner for quick verification - Add spec catalog with state counts and invariant names - Remove stale references to brew formula (doesn't exist) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: replace match with `?` operator in CompleteMerge handler CI clippy lint (`clippy::question-mark`, -D warnings) rejected the explicit match-and-return-None pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec: extend MergePipelineShutdown to cover crash/restart and multi-lifetime Phase A of the verification-gap closure (PR #6369). The original spec only modeled orderly two-phase shutdown — no Crash action, no Restart, no row-conservation claim. The four invariants were either trivially true by construction or static set-membership, so the spec proved nothing about the failure modes that motivated it. This rewrite: - Adds Crash and Restart actions. Crash invalidates in-memory state at any point in the pipeline; uploaded-but-unpublished merge outputs become orphan_outputs. Restart re-seeds cold_windows from durable published_splits (models fetch_immature_splits). - Splits CompleteMerge into UploadMergeOutput then PublishMergeAndFeedback so a Crash between the two phases is reachable. This is what catches the "leaked but invisible" failure mode — output blob in S3 with no metastore reference. The model proves these are object-store-only garbage (LeakIsObjectStoreOnly), not durable data loss. - Adds split_rows / split_merge_ops / split_window ghost functions so RowsConserved expresses the strong "no data loss, no duplication" property: total ingested rows equal sum of rows in published_splits in every reachable state. - Adds NoOrphanWhenConnected (state invariant), RestartReSeedsAllImmature (action property) and NoPersistentOrphan (liveness leads-to). Together these capture: while connected the planner sees every immature split; every Restart correctly recovers all immature splits; in any run with restart budget remaining, orphans always eventually clear. - Extends Restart to fire after graceful shutdown too (multi-lifetime), so the cross-process recovery claim is explicit. Bounded by MaxRestarts. The replaced NoOrphanAfterRestart invariant fired during initial TLC runs — the failing trace was a *legitimate* state in production (publish during shutdown disconnect leaves the output untracked by the planner until the next process invocation). The fix wasn't to remove the invariant but to split the safety claim from the recovery claim: NoOrphanWhenConnected for the steady state, RestartReSeedsAllImmature for the recovery transition, NoPersistentOrphan for cross-lifetime liveness. Recorded as a sesh-mode rule: never silently weaken an invariant that produced a counter-example. Single config drops the small/full split — full config now runs in ~12s on a workstation (217,854 distinct states, 11 invariants + 3 temporal properties verified). The states/ directory dropped by TLC is now gitignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec: add merge-chain config alongside multi-lifetime primary The primary `MergePipelineShutdown.cfg` is now optimized for multi-lifetime exercise (MaxIngests=3, MaxRestarts=2) — 15,732 states in ~1s — covering the cross-process recovery claim added in the previous commit. The new `MergePipelineShutdown_chains.cfg` keeps MaxIngests=4 with MaxRestarts=1 for deeper merge-chain coverage (217,854 states, ~10s): exercises level-0 → level-1 → level-2-mature compaction so BoundedWriteAmp is checked across the full chain and concurrent in-flight merges interact. Combined run is ~11s. Both share the same invariant + property set; only the constants differ. The TLA+ run-all loop in CLAUDE.md now globs every .cfg and resolves the matching .tla, supporting any number of configs per spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: share state and predicates between Stateright model and invariants Phase B of the verification-gap closure (PR #6369). Eliminates drift between the merge-pipeline model and any production-side checks by making them evaluate the *same* Rust functions on the *same* state. quickwit-dst/src/invariants/merge_pipeline.rs (new): - MergePipelineState struct mirroring the TLA+ VARIABLES block (planner_alive, in_flight_merges, cold_windows, published_splits, splits, orphan_outputs, lifecycle counters, total_ingested_rows ghost) - Pure-function predicates corresponding 1:1 to TLA+ invariants: rows_conserved, bounded_write_amp, no_split_loss, no_duplicate_merge, no_orphan_in_planner, no_orphan_when_connected, leak_is_object_store_only, mp1_level_homogeneity, restart_re_seeds_all_immature - Helper orphan_set mirrors the TLA+ OrphanSet operator - 17 unit tests covering passing/failing paths for each predicate quickwit-dst/src/invariants/registry.rs: - Adds MP4..MP11 invariant IDs (preserved short-code naming convention) - Each maps to a TLA+ invariant from MergePipelineShutdown.tla quickwit-dst/src/models/merge_pipeline.rs: - State type is now invariants::merge_pipeline::MergePipelineState (literal sharing — no parallel definition, no conversion layer) - CompleteMerge split into UploadMergeOutput + PublishMergeAndFeedback to match the TLA+ multi-phase merge completion. A Crash between phases orphans the upload (orphan_outputs) without losing data. - Restart re-seeds cold_windows from durable published_splits (mirrors fetch_immature_splits) and resets shutdown_complete / finalize state, modelling a fresh process invocation. Bounded by max_restarts. - DrainComplete added so graceful-shutdown is a distinct terminal action (matching TLA+). - properties() calls the shared predicate functions instead of inline closures — model and runtime checks evaluate identical Rust code. - Three configs: small (fast iteration), multi_lifetime (3 lifetimes, matches MergePipelineShutdown.cfg), deep_chains (level 0→1→2 mature, matches MergePipelineShutdown_chains.cfg). - Restart's next_state debug_asserts MP-11 (restart_re_seeds_all_immature) on its post-state, encoding the action property as a runtime check. All 50 tests pass under `cargo nextest run -p quickwit-dst --features model-checking` (33 invariant unit tests + 4 merge-pipeline model configs + 13 other models). Multi-lifetime checks in 0.1s, deep-chains in 0.6s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: wire merge-pipeline invariant checks into production code Phase C of the verification-gap closure (PR #6369). Production code now evaluates the *same* invariant predicates that the Stateright model and TLA+ spec verify, with results forwarded to the existing DogStatsD recorder (already wired in quickwit-cli/src/logger.rs). Every check emits `pomsky.invariant.checked{invariant=MP-X}` and, on failure, `pomsky.invariant.violated{invariant=MP-X}` — usable directly as Datadog alert criteria. ParquetMergeExecutor (post-merge, pre-uploader): - MP-2 (HasMinimumSplits): merge has >= 2 inputs - MP-1 (LevelHomogeneity): all inputs share num_merge_ops level - MP-3 (ScopeCompatibility): all inputs share sort_fields and window - MP-4 (RowsConserved): sum of input num_rows == sum of output num_rows. Empty-output path checks input rows are all zero (otherwise data was silently dropped). The non-empty path checks the strong row-preservation property — the same MC-1 / RowsConserved invariant from the TLA+ spec. ParquetMergePlanner::new (post-restart re-seed): - MP-11 (RestartReSeedsAllImmature): every split the merge policy classifies as still-immature (and which has a window for compaction) is recorded in scoped_young_splits and known_split_ids. Filters out mature splits, time-expired splits, and pre-Phase-31 splits before comparing — these are intentional drops, not the failure mode MP-11 protects against. Cargo.toml: quickwit-indexing now depends on quickwit-dst for the shared invariant module and check_invariant! macro. Also fixes a pre-existing rustdoc warning: the parquet_merge_planner docs referenced `[`MergePlanner`]` as an intra-doc link, but the type lives in another crate and can't resolve. Changed to plain backticks. All 52 metrics_pipeline tests pass; clippy + nightly fmt + machete + cargo doc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: trace conformance — replay production events through the formal model Phase D of the verification-gap closure (PR #6369). The TLA+ spec and Stateright model are now backed by *production code that actually emits events at every modeled transition*, and a test replays those events through the same predicates the model verifies. A divergence between production and the model surfaces here as a predicate failure on real production state. This closes the loop: TLA+ → Stateright (Phase A+B) → production checks (Phase C) → trace conformance (Phase D). quickwit-dst/src/events/merge_pipeline.rs (new): - MergePipelineEvent enum with one variant per modeled action, using production types (String split IDs, Range<i64> windows). Notably no Crash variant — process death cannot emit anything; crashes are inferred during replay by the absence of events between actions and a subsequent Restart. - Pluggable observer (fn pointer) following the existing set_invariant_recorder pattern. No-op single-atomic-load when no observer is installed; production hot path stays cheap. Production emission sites (always-on, no feature flag): - ParquetMergePipeline::spawn_pipeline → Restart, after fetch_immature_splits - ParquetMergePipeline::FinishPendingMergesAndShutdownPipeline → DisconnectMergePlanner and RunFinalizeAndQuit - ParquetMergePlanner::send_merge_ops → PlanMerge - ParquetUploader::handle (post-upload, pre-publish) → UploadMergeOutput for merge outputs - Publisher::handle (Parquet publish path) → IngestSplit (when no replaced_split_ids) or PublishMergeAndFeedback (when replacing) quickwit-indexing/.../parquet_merge_pipeline_trace_conformance_test.rs (new): - Two scenarios: 1. Normal happy path: 4 splits → 2 merges → 1 mature output. Replay verifies MP-1, MP-4..MP-10 hold at every state. ~0.6s. 2. Crash mid-cascade: publish failure injected on the 2nd merge forces respawn. Trace covers Restart → PlanMerge → Upload → crash → Restart → re-seed → PlanMerge → Upload → Publish. ~1.1s. - StateMirror reconstructs MergePipelineState from events using a production-id → model-u32 interner, applies the same predicate functions used by the Stateright model. - The mock metastore tracks `staged` and `published` separately so list_metrics_splits returns only what real metastores would (not staged-but-failed). Earlier test-side bug here would have masked divergences; the trace test caught it immediately. What this catches that prior tests do not: - The crash test only verified re-seeding *happened*. The trace test verifies that the post-restart state preserves all formal safety invariants (no orphans the planner can never reclaim, no row drift, no level mixing on re-merge, etc.). - A divergence between production order/atomicity and the model's atomic actions surfaces as either a state-mirror error ("event can't be applied") or a predicate failure with the offending event in the panic message. Both tests pass with the current production code: production behavior matches the TLA+ spec for the scenarios exercised. The test infrastructure is now ready to surface real divergences if they exist, and to be extended with adversarial scenarios (multiple back-to-back crashes, finalize-during-crash, concurrency stress). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: add TODO for CS-3 invariant when compaction_start_time lands CS-3 ("splits before compaction_start_time are never compacted") is defined in the TLA+ model and the InvariantId registry, but the production `ParquetMergePolicyConfig` does not yet expose a `compaction_start_time` field. Adding a runtime check now would have nothing to filter against. Records the gap as a planner-level TODO so the check lands alongside the feature when it's implemented (filter splits in `record_splits_if_necessary` + verify via `check_invariant!`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: rename metrics_pipeline → parquet_pipeline, update CODEOWNERS The actors under quickwit-indexing/src/actors/metrics_pipeline/ are the parquet-based ingest+compaction pipeline. They currently process metrics, but logs and traces will move onto the same pipeline in later phases — the metrics-specific name has become misleading. Pure rename: - git mv metrics_pipeline → parquet_pipeline - update mod declaration + import paths (metrics_pipeline:: → parquet_pipeline::) - update CODEOWNERS path Function names that incidentally contain "metrics_pipeline" (spawn_metrics_pipeline, spawn_log_or_metrics_pipeline, test_metrics_pipeline_e2e) are NOT renamed in this commit — they describe pipeline kind ("the metrics-flavored pipeline") rather than the module, and would benefit from their own pass that also addresses the eventual log/trace generalization. CODEOWNERS also adds /docs/internals/ to the byoc-metrics path list: the architecture and verification docs there are domain-aligned with quickwit-parquet-engine + quickwit-dst (already in the list) and should be approvable by the same team. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(sesh-mode): generalize don't-weaken rule to user requirements The "STOP, Don't Weaken" section already covered formal verification specs (TLA+ invariants, Stateright properties, DST assertions). It didn't explicitly cover the spec→plan→implementation translation step, which is where silent weakening tends to slip in for English-language user requirements. Restructure the section under one umbrella ("Specs are Load-Bearing") with two subsections: - "Verification check fails" — original content verbatim - "User requirement seems hard or out-of-scope" — five concrete silent-weakening failure modes (granularity downgrade, constraint dropping, strength reduction, MUST→SHOULD, scope reframing), the protocol that mirrors the verification case, and the plan-approval-is-not-spec-approval meta-trap Lesson surfaced from PR-4: the original plan I wrote substituted column-chunk granularity for the page-level streaming the user had asked for, and got plan-document approval rather than spec approval. The new section flags that failure mode by name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(fmt): re-sort actors/mod.rs entries after parquet_pipeline rename The metrics_pipeline → parquet_pipeline rename moved the module name past `packager` in the alphabetical ordering rustfmt's imports_granularity pass enforces. Swap the two entries (both the `mod` declarations and the `pub use` re-exports) to restore order. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(codeowners): open docs/internals/ and Cargo.lock to any-write-access Last-matching-rule wins. A path line with no team after it removes the codeowner requirement, so any user with write access can approve PRs that only touch those paths. - `/docs/internals/`: shared architecture + verification docs that any team working on the codebase may need to update. Gating on a single team blocks unrelated work. - `/quickwit/Cargo.lock`: churns on routine dependency bumps and doesn't carry domain-specific review value. Per CODEOWNERS semantics, /docs/internals/ no longer falls under byoc-metrics (which the previous commit had assigned it to). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(codeowners): docs/internals and Cargo.lock approvable by either team Per the GitHub docs, multiple owners on the same line means approval from any ONE of them satisfies the codeowner requirement. Listing both `quickwit-core` and `byoc-metrics` on `/docs/internals/` and `/quickwit/Cargo.lock` lets either team approve PRs scoped to those paths. Replaces the previous commit's "no required owner" formulation, which would have allowed any user with write access. The intent is to keep the review gate but distribute it across both teams that work in this repo, not to remove the gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
George Talbot
|
7597552188 |
clean up docs (#6395)
* chore: strip internal-only references from docs and code
Removes references to internal Datadog repos (dd-source, dd-go), internal
Google Docs URLs, internal codenames (Pomsky, Quickhouse, Flurry, Bolt),
and ADR-004 cross-references (the doc itself does not exist in this repo).
Behavior changes:
- DogStatsD: env vars renamed CLOUDPREM_/CP_DOGSTATSD_SERVER_HOST/PORT
-> QW_STATSD_SERVER_HOST/PORT (deprecated CLOUDPREM_ branch was past its
2026-04-01 sunset date). Statsd prefix "cloudprem" -> "quickwit". Tag
keys cloudprem_cluster_id/cloudprem_node_id -> quickwit_cluster_id/
quickwit_node_id. Counter names pomsky.invariant.checked|violated
-> invariant.checked|violated (the prefix supplies the "quickwit." part).
- Datadog log format (DdgFormat) "service" / "ddsource" fields:
"byoc" -> "quickwit".
Doc/comment-only edits otherwise: ADR/gap/locality-compaction prose,
proto comments + regenerated codegen, vendored sortschema proto loses
java_package/go_package options and dd-source / dd-go disclosures.
Husky public references (datadoghq.com blog) and code-level
"Husky-style" / "build_husky_ordered_schema" identifiers retained:
"Husky" is a publicly published name and the conventions are real
technical artifacts (sort schema string format).
* chore: drop personal-name attributions and DD-flavored framing in dst docs
Followup to the internal-references cleanup:
- Replace "Matthew Kim's implementation" / "Done (Matthew Kim)" / "Sort
implementation by Matthew Kim" with neutral phrasing in adr/002 and
gaps/002 (PR authorship is already in git history).
- Replace "at Alexey's scale numbers" with "at the scale numbers cited
in the Scale Considerations section" in locality-compaction/phase-1.
- Reword quickwit-dst/src/invariants/{check,recorder}.rs doc comments
so the InvariantRecorder API is described as a generic metrics-emission
hook rather than a Datadog-specific one (the example still shows a
StatsD-shaped snippet, which is publicly documented).
* chore: route invariant counters through Prometheus, drop DogStatsD shim
The original quickwit-dst PR (#6246) bundled in a DogStatsD metrics
exporter that ran unconditionally on every quickwit-cli startup. That
shim was deployment-specific (CLOUDPREM_/CP_/QW_ env vars, hardcoded
Kubernetes label conventions, fixed-prefix global labels) and parallel
to Quickwit's existing Prometheus stack.
Replace the DogStatsD path with two Prometheus counters that surface on
the existing /metrics endpoint served by quickwit-serve:
- quickwit_verification_invariant_checked_total{invariant="..."}
- quickwit_verification_invariant_violated_total{invariant="..."}
Counters are registered as LazyLock<IntCounterVec<1>> via the existing
quickwit_common::metrics::new_counter_vec factory, matching the rest of
the codebase. setup_metrics() becomes install_invariant_recorder() —
no env vars, no UDP socket, no extra deps.
Drops:
- quickwit-cli deps on metrics + metrics-exporter-dogstatsd
- workspace deps metrics = "0.24" and metrics-exporter-dogstatsd = "0.9"
- 7 transitive crates from LICENSE-3rdparty.csv (metrics, metrics-util,
metrics-exporter-dogstatsd, endian-type, nibble_vec, radix_trie,
rand_xoshiro)
The recorder API in quickwit-dst stays pluggable; the example doc was
updated to reference the OSS Prometheus wiring.
* chore: contain DST/invariants integration in quickwit-dst, no quickwit-cli leakage
Reviewer feedback on the Prometheus invariant recorder: the cleaner answer
is to keep the DST integration entirely contained in quickwit-dst rather
than have quickwit-cli wire up a recorder at all. quickwit-parquet-engine
keeps using quickwit_dst::check_invariant! for in-code invariant checks
(which become silent no-ops in release / panicking debug_assert in debug
when no recorder is installed) — that's the legitimate code-level use.
Reverts quickwit-cli to its pre-#6246 shape with respect to the invariant
recorder hookup:
- Drop install_invariant_recorder() and the Prometheus counters from
logger.rs.
- Drop the call from main_impl().
- Drop the quickwit-dst workspace dep from quickwit-cli/Cargo.toml.
Downstream distributions that want a production recorder (e.g. wired to
DogStatsD) can install one via quickwit_dst::invariants::set_invariant_recorder
in their own binary or as a downstream patch — the recorder API in
quickwit-dst stays pluggable.
Build version tracking is unaffected: register_build_info_metric() in
main.rs continues to register quickwit_build_info{version, build_date,
commit_hash, commit_tags, target} on the existing /metrics endpoint.
|
||
|
George TalbotandClaude Opus 4.6
|
a36f4fec78 |
feat: add configurable ParquetMergePolicyConfig to index settings (#6362)
* feat: add configurable ParquetMergePolicyConfig to index settings Adds `parquet_merge_policy` section to `IndexingSettings`, making the Parquet merge policy configurable per-index via YAML. Parameters: - merge_factor (default 10): min splits to trigger a merge - max_merge_factor (default 12): max splits per merge - max_merge_ops (default 4): bounds write amplification - target_split_size_bytes (default 256 MiB): target output size - maturation_period (default 48h): split maturity timeout - max_finalize_merge_operations (default 3): cold-window shutdown limit Mirrors the existing merge_policy config pattern for logs/traces. Updates index-config.md documentation with the new section. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add ParquetIndexingConfig with sort_fields and window_duration_secs Adds `parquet_indexing` section to `IndexingSettings` for per-index Parquet pipeline configuration: - `sort_fields`: sort schema override (Husky-style pipe-delimited syntax with /V2 suffix). Controls row ordering, query pruning, compression locality, and compaction scope. When omitted, uses the product-type default. - `window_duration_secs`: time window for split partitioning (default 900s / 15 min). Must divide 3600. Updates docs/configuration/index-config.md with: - "Parquet indexing settings" section explaining both parameters - Full sort schema syntax reference (column types, direction overrides, & LSM cutoff marker) - Examples showing minimal, custom, and advanced configurations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update indexing service fingerprint constants and nightly fmt Adding ParquetMergePolicyConfig and ParquetIndexingConfig to IndexingSettings changes the Hash output, which changes the pipeline params fingerprints. Updated the hardcoded test constants. Added a comment explaining how to recompute them when IndexingSettings fields change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
George TalbotandClaude Opus 4.6
|
9f49c8d670 |
fix: SS-2 invariant — nulls always sort last, regardless of direction (#6343)
* fix: SS-2 invariant — nulls always sort last, regardless of direction The SS-2 invariant incorrectly specified that nulls sort before non-null for descending columns. The actual production code uses nulls_first=false everywhere — nulls always sort after non-null values, regardless of direction. This is required for sorted_series key correctness: null columns are omitted from the key, and their keys naturally sort after keys that include those columns. Fixed in all locations: - TLA+ SortSchema.tla: CompareValues and SS2_NullOrdering - Rust invariants/sort.rs: compare_with_null_ordering - Rust models/sort_schema.rs: SS-2 property check and compare_values test - Rust invariants/registry.rs: SS-2 description - ADR-002: SS-2 invariant table - Phase 1 design doc: null handling descriptions Stateright exhaustive_sort_schema BFS passes with corrected model. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify model detects null ordering violations Added two tests to the Stateright sort_schema model: - ss2_detects_null_before_non_null_in_descending: constructs rows with null before non-null in a descending column, verifies both row_leq and is_sorted reject this ordering (nulls always last). - ss2_accepts_non_null_before_null_in_descending: verifies the correct ordering (non-null then null) is accepted. These confirm the model can catch the exact class of bug that the SS-2 invariant fix addresses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
George TalbotandClaude Opus 4.6
|
9763a93815 |
feat: port zonemap package and wire into Parquet pipeline (#6295)
* feat: port zonemap package and wire into Parquet pipeline Port the Go zonemap package (DFA-based prefix-preserving superset regex builder) to Rust and integrate it into the Parquet write pipeline. For each string-valued sort schema column, a compact regex is generated that accepts all column values (and possibly more). These regexes enable query-time split pruning: if a predicate cannot match any string accepted by the regex, the split can be skipped entirely. Zonemap module (quickwit-parquet-engine/src/zonemap/): - automaton.rs: DFA with weighted pruning and deterministic regex generation, including character class collapsing and suffix factoring - regex_builder.rs: PrefixPreservingRegexBuilder with progressive pruning during registration and final prune at build time - minmax.rs: FNV-1a hash-based MinMax builder for future range pruning - mod.rs: extract_zonemap_regexes() public API for Arrow RecordBatches Pipeline integration: - writer.rs: extract zonemap regexes in prepare_write() after row_keys, store as JSON in qh.zonemap_regexes Parquet KV metadata - split_writer.rs: capture zonemap_regexes in MetricsSplitMetadata - WriteMetadata type alias carries both row_keys and zonemap through write_to_bytes() and write_to_file_with_metadata() 34 tests covering automaton (regex, pruning, escaping, character classes, disjunctive clauses, long strings, post-prune behavior), regex builder (basic, pruning, progressive), MinMax (string/int hashing, reset, empty), Arrow extraction (basic, nulls, missing columns, special chars, disabled), and full pipeline integration (KV metadata, split writer, JSON roundtrip, write path consistency). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: gate PARQUET_META_ZONEMAP_REGEXES re-export behind #[cfg(test)] The pub(crate) re-export was unused in non-test builds, causing -D unused-imports to fail in CI. The constant is only needed by zonemap pipeline_tests, so gate it behind #[cfg(test)]. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix import ordering for nightly fmt group_imports Reorder pub use re-exports and use crate:: imports to satisfy cargo +nightly fmt with group_imports = StdExternalCrate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: expect infallible HashMap<String, String> JSON serialization serde_json::to_string on HashMap<String, String> cannot fail — silently swallowing the error with let Ok() would hide a real bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add missing Go test coverage and LSM cutoff support Port remaining Go builder_test.go cases: - TestBuildFragmentZoneMap: exact regex string verification - TestNilSortSchema: empty sort fields → no regexes - TestNonMutatedResult: builder reuse independence - TestInvalidUTF8: Unicode BMP handling (Rust variant) - TestZoneMapForIntColumns: int columns produce no regex - TestResetWithLsmComparisonCutoff: LSM cutoff truncation Also fixes extract_zonemap_regexes to respect lsm_comparison_cutoff from the sort schema — only columns before the cutoff get zonemaps, matching Go FragmentZoneMapBuilder.Reset() behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: use full 584-line integrations benchmark data file Include the complete benchmark_data/integrations file from the Go zonemap package and use it via include_str!() in the benchmark test. Verifies that 584 real integration names with max 64 transitions produces a valid pruned regex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify exact regex strings and add LSM cutoff support Add exact regex string verification for long service name (matches Go TestBuildFragmentZoneMap: "^a_very_very_very_very_long_long_.+$") and metric_name single-value case ("^cpu\.usage$"). Also verifies service and env dict columns produce identical regexes for identical values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: update GAP-002, GAP-004, and ADR-002 with resolution status Mark completed items in GAP-002 (sort schema parser, configurable directions, timeseries_id, schema-driven sort, metadata storage) and GAP-004 (MetricsSplitMetadata fields, RowKeys, zonemap regexes, sorting_columns, KV metadata). Update ADR-002 Implementation Status to reflect the full PR stack (#6287-#6295). Remaining open items: per-index metastore storage (Phase 32), null ordering fix, Parquet column/offset index enabling, PostgreSQL migration for row_keys + zonemap columns. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: nulls always sort last regardless of column direction Change nulls_first from true to false in sort_batch(), sorting_columns() metadata, and the SS-1 verification sort. Nulls now sort after all non-null values for both ascending and descending columns. This simplifies compaction: when a sort column is absent from a split, all rows are treated as null. With nulls-last, these rows cluster at the end and don't interfere with key-range comparisons between splits. Adds test_nulls_sort_last_ascending_and_descending which writes through the full pipeline with ascending and descending service columns, verifying null rows appear last in both cases. Updates ADR-002 and GAP-002 to mark the null ordering item resolved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply nightly fmt to nulls-last test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: use struct init instead of field reassign for TableConfig Fixes clippy::field_reassign_with_default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve merge conflicts after rebase — adapt to ParquetSplitMetadata API - Keep DDSketch sketch validation in prepare_write - Fix ParquetSplitWriter::new to pass ParquetSplitKind - Update MetricsSplitMetadata references to ParquetSplitMetadata - Remove .metadata indirection (write_split returns metadata directly) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add timeseries_id to remaining inline test batches after mandatory enforcement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add .unwrap() to constructor calls and fix WriteMetadata destructuring in zonemap tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: replace raw pointers with arena indices in automaton pruning The pruning algorithm used raw *mut State pointers in a BinaryHeap to enable mutable access to tree nodes during traversal. Replace with an arena (Vec<State>) indexed by usize, eliminating the unsafe block entirely. States are now allocated via alloc_state() and referenced by index throughout add(), prune(), and regex generation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle LargeUtf8 in register_string_values to match is_string_dict is_string_dict accepted Dictionary(_, LargeUtf8) and plain LargeUtf8, but register_string_values only handled Dictionary(_, Utf8) and Utf8. A LargeUtf8 column would pass the filter but produce an empty regex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use debug_assert for max_num_transitions invariant instead of panic This is an internal invariant (not user input), so debug_assert is appropriate — it documents the contract for reviewers and catches violations in debug builds without crashing production. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify all string column types produce zonemaps Exercises extract_zonemap_regexes with Utf8, LargeUtf8, Dictionary(Int32, Utf8), and Dictionary(Int32, LargeUtf8) columns. All four produce the same regex for the same input values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: merge duplicate arrow::datatypes imports for nightly fmt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle all Arrow dictionary key types in zonemap register_string_values Previously only DictionaryArray<Int32Type> was handled. A dictionary with any other key type (Int8, UInt8, etc.) would pass is_string_column but silently produce an empty regex, breaking the superset guarantee. Uses a macro to expand all 8 key types (Int8..UInt64) without boilerplate. Test covers all 18 dictionary variants (8 key types × Utf8/LargeUtf8) plus plain Utf8 and LargeUtf8, asserting each produces the exact correct regex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use [\s\S] instead of . in pruned regex wildcards to match newlines The '.' wildcard excludes \n in standard regex semantics. If a tag value contains a newline, a pruned regex using .+ would fail to match, breaking the superset guarantee and causing incorrect split pruning. [\s\S] matches any character including newlines without requiring dot-all mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: verify pruned regex matches values containing newlines Adds a test that registers values with embedded \n, prunes, compiles the generated regex, and asserts it matches all original values. This would fail with the old '.' wildcard since '.' doesn't match newlines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Adrien Guillo
|
11587c000b |
Document disable_checksums storage config parameter (#6325)
|
||
|
trinity-1686a
|
62a4756639 | fix wrong keyword in doc (#6321) | ||
|
Nicolas Coiffier
|
44c439f7f3 | Fix typos in metrics.md (#6304) | ||
|
|
a92e744750 |
docs: add CLAUDE.md, architecture docs, and /sesh-mode skill (#6247)
* feat: replace fixed MetricDataPoint fields with dynamic tag HashMap * feat: replace ParquetField enum with constants and dynamic validation * feat: derive sort order and bloom filters from batch schema * feat: union schema accumulation and schema-agnostic ingest validation * feat: dynamic column lookup in split writer * feat: remove ParquetSchema dependency from indexing actors * refactor: deduplicate test batch helpers * lint * feat(31): sort schema foundation — proto, parser, display, validation, window, TableConfig Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: rustdoc link errors — use backticks for private items * feat(31): compaction metadata types — extend split metadata, postgres model, field lookup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(31): wire TableConfig into sort path, add compaction KV metadata Wire TableConfig-driven sort order into ParquetWriter and add self-describing Parquet file metadata for compaction: - ParquetWriter::new() takes &TableConfig, resolves sort fields at construction via parse_sort_fields() + ParquetField::from_name() - sort_batch() uses resolved fields with per-column direction (ASC/DESC) - SS-1 debug_assert verification: re-sort and check identity permutation - build_compaction_key_value_metadata(): embeds sort_fields, window_start, window_duration, num_merge_ops, row_keys (base64) in Parquet kv_metadata - SS-5 verify_ss5_kv_consistency(): kv_metadata matches source struct - write_to_file_with_metadata() replaces write_to_file() - prepare_write() shared method for bytes and file paths - ParquetWriterConfig gains to_writer_properties_with_metadata() - ParquetSplitWriter passes TableConfig through - All callers in quickwit-indexing updated with TableConfig::default() - 23 storage tests pass including META-07 self-describing roundtrip Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(31): PostgreSQL migration 27 + compaction columns in stage/list/publish Add compaction metadata to the PostgreSQL metastore: Migration 27: - 6 new columns: window_start, window_duration_secs, sort_fields, num_merge_ops, row_keys, zonemap_regexes - Partial index idx_metrics_splits_compaction_scope on (index_uid, sort_fields, window_start) WHERE split_state = 'Published' stage_metrics_splits: - INSERT extended from 15 to 21 bind parameters for compaction columns - ON CONFLICT SET updates all compaction columns list_metrics_splits: - PgMetricsSplit construction includes compaction fields (defaults from JSON) Also fixes pre-existing compilation errors on upstream-10b-parquet-actors: - Missing StageMetricsSplitsRequestExt import - index_id vs index_uid type mismatches in publish/mark/delete - IndexUid binding (to_string() for sqlx) - ListMetricsSplitsResponseExt trait disambiguation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(31): close port gaps — split_writer metadata, compaction scope, publish validation Close critical gaps identified during port review: split_writer.rs: - Store table_config on ParquetSplitWriter (not just pass-through) - Compute window_start from batch time range using table_config.window_duration_secs - Populate sort_fields, window_duration_secs, parquet_files on metadata before write - Call write_to_file_with_metadata(Some(&metadata)) to embed KV metadata in Parquet - Update size_bytes after write completes metastore/mod.rs: - Add window_start and sort_fields fields to ListMetricsSplitsQuery - Add with_compaction_scope() builder method metastore/postgres/metastore.rs: - Add compaction scope filters (AND window_start = $N, AND sort_fields = $N) to list query - Add replaced_split_ids count verification in publish_metrics_splits - Bind compaction scope query parameters ingest/config.rs: - Add table_config: TableConfig field to ParquetIngestConfig Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(31): final gap fixes — file-backed scope filter, META-07 test, dead code removal - file_backed_index/mod.rs: Add window_start and sort_fields filtering to metrics_split_matches_query() for compaction scope queries - writer.rs: Add test_meta07_self_describing_parquet_roundtrip test (writes compaction metadata to Parquet, reads back from cold file, verifies all fields roundtrip correctly) - fields.rs: Remove dead sort_order() method (replaced by TableConfig) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(31): correct postgres types for window_duration_secs and zonemap_regexes Gap 1: Change window_duration_secs from i32 to Option<i32> in both PgMetricsSplit and InsertableMetricsSplit. Pre-Phase-31 splits now correctly map 0 → NULL in PostgreSQL, enabling Phase 32 compaction queries to use `WHERE window_duration_secs IS NOT NULL` instead of the fragile `WHERE window_duration_secs > 0`. Gap 2: Change zonemap_regexes from String to serde_json::Value in both structs. This maps directly to JSONB in sqlx, avoiding ambiguity when PostgreSQL JSONB operators are used in Phase 34/35 zonemap pruning. Gap 3: Add two missing tests: - test_insertable_from_metadata_with_compaction_fields: verifies all 6 compaction fields round-trip through InsertableMetricsSplit - test_insertable_from_metadata_pre_phase31_defaults: verifies pre-Phase-31 metadata produces window_duration_secs: None, zonemap_regexes: json!({}) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: rustfmt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(31): add metrics split test suite to shared metastore_test_suite! macro 11 tests covering the full metrics split lifecycle: - stage (happy path + non-existent index error) - stage upsert (ON CONFLICT update) - list by state, time range, metric name, compaction scope - publish (happy path + non-existent split error) - mark for deletion - delete (happy path + idempotent non-existent) Tests are generic and run against both file-backed and PostgreSQL backends. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(31): read compaction columns in list_metrics_splits, fix cleanup_index FK * fix(31): correct error types for non-existent metrics splits - publish_metrics_splits: return NotFound (not FailedPrecondition) when staged splits don't exist - delete_metrics_splits: succeed silently (idempotent) for non-existent splits instead of returning FailedPrecondition - Tests now assert the correct error types on both backends Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: rustfmt metastore tests and postgres * fix(31): address PR review — align metrics_splits with splits table - Migration 27: add maturity_timestamp, delete_opstamp, node_id columns and publish_timestamp trigger to match the splits table (Paul's review) - ListMetricsSplitsQuery: adopt FilterRange<i64> for time_range (matching log-side pattern), single time_range field for both read and compaction paths, add node_id/delete_opstamp/update_timestamp/create_timestamp/ mature filters to close gaps with ListSplitsQuery - Use SplitState enum instead of stringly-typed Vec<String> for split_states - StoredMetricsSplit: add create_timestamp, node_id, delete_opstamp, maturity_timestamp so file-backed metastore can filter on them locally - File-backed filter: use FilterRange::overlaps_with() for time range and window intersection, apply all new filters matching log-side predicate - Postgres: intersection semantics for window queries, FilterRange-based SQL generation for all range filters - Fix InsertableMetricsSplit.window_duration_secs from Option<i32> to i32 - Rename two-letter variables (ws, sf, dt) throughout Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix rustfmt nightly formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(31): add shared invariants module to quickwit-dst Extract duplicated invariant logic into a shared `invariants/` module within `quickwit-dst`. This is the "single source of truth" layer in the verification pyramid — used by stateright models, production debug_assert checks, and (future) Datadog metrics emission. Key changes: - `invariants/registry.rs`: InvariantId enum (20 variants) with Display - `invariants/window.rs`: shared window_start_secs(), is_valid_window_duration() - `invariants/sort.rs`: generic compare_with_null_ordering() for SS-2 - `invariants/check.rs`: check_invariant! macro wrapping debug_assert - stateright gated behind `model-checking` feature (optional dep) - quickwit-parquet-engine uses shared functions and check_invariant! Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(31): check invariants in release builds, add pluggable recorder The check_invariant! macro now always evaluates the condition — not just in debug builds. This implements Layer 4 (Production) of the verification stack: invariant checks run in release, with results forwarded to a pluggable InvariantRecorder for Datadog metrics emission. - Debug builds: panic on violation (debug_assert, Layer 3) - All builds: evaluate condition, call recorder (Layer 4) - set_invariant_recorder() wires up statsd at process startup - No recorder registered = no-op (single OnceLock load) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(31): wire invariant recorder to DogStatsD metrics Emit cloudprem.pomsky.invariant.checked and .violated counters with invariant label via the metrics crate / DogStatsD exporter at process startup, completing Layer 4 of the verification stack. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: license headers + cfg(not(test)) for quickwit-dst and quickwit-cli * chore: regenerate third-party license file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix rustfmt nightly formatting for quickwit-dst and quickwit-parquet-engine Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add CLAUDE.md and docs/internals architecture documentation Ports CLAUDE.md (development guide, coding standards, known pitfalls) and the full docs/internals tree including ADRs, gap analyses, TLA+ specs, verification guides, style references, and compaction architecture. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: split CLAUDE.md into repo context + opt-in /sesh-mode skill - Move verification-first workflow (TLA+, DST, formal specs) to /sesh-mode skill - Keep repo knowledge in CLAUDE.md (pitfalls, reliability rules, testing, docker, commands) - Remove Crate Map (derivable from filesystem) - Remove Coding Style bullet summary (CODE_STYLE.md is linked) - Fix relative links in SKILL.md for .claude/skills/sesh-mode/ path Co-Authored-By: Claude <noreply@anthropic.com> * docs: add machete, cargo doc, and fmt details to CI checklist in CLAUDE.md * review: parquet_file singular, proto doc link, fix metastore accessor * style: fix rustfmt nightly comment wrapping in split metadata Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use plain code span for proto reference to avoid broken rustdoc link Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update quickwit/quickwit-parquet-engine/src/table_config.rs Co-authored-by: Matthew Kim <matthew.kim@datadoghq.com> * Update quickwit/quickwit-parquet-engine/src/table_config.rs Co-authored-by: Matthew Kim <matthew.kim@datadoghq.com> * style: rustfmt long match arm in default_sort_fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: make parquet_file field backward-compatible in MetricsSplitMetadata Pre-existing splits were serialized before the parquet_file field was added, so their JSON doesn't contain it. Adding #[serde(default)] makes deserialization fall back to empty string for old splits. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle empty-column batches in accumulator flush When the commit timeout fires and the accumulator contains only zero-column batches, union_fields is empty and concat_batches fails with "must either specify a row count or at least one column". Now flush_internal treats empty union_fields the same as empty pending_batches — resets state and returns None. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: rustfmt check_invariant macro argument Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: move code changes to separate PR Reverts |
||
|
Oleksii Syniakov
|
8954ddec4b | Improve cluster sizing doc on control plane/metastore/janitor. Closes #5872 (#6202) | ||
|
|
1ff255cdf7 |
feat(es-compat): add index_filter support for field capabilities API (#6102)
* feat(es-compat): add index_filter support for field capabilities API Implements index_filter parameter support for the ES-compatible _field_caps endpoint, allowing users to filter field capabilities based on document queries. Changes: - Add query_ast field to ListFieldsRequest and LeafListFieldsRequest protos - Parse index_filter from ES Query DSL and convert to QueryAst - Pass query_ast through to leaf nodes for future filtering support - Add unit tests for index_filter parsing - Add REST API integration tests Note: This implementation accepts and parses the index_filter parameter for API compatibility. Full split-level document filtering will be added as a follow-up enhancement. Closes #5693 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: ruo <ruoliu.dev@gmail.com> * feat(es-compat): implement split-level filtering for field_caps index_filter Address PR review comments for index_filter support in _field_caps API: - Extract `parse_index_filter_to_query_ast()` function with clean prototype - Implement split-level filtering via `split_matches_query()` using lightweight `query.count()` execution (no document materialization) - Add proper async handling with ByteRangeCache, warmup(), and run_cpu_intensive() for Quickwit's async-only storage - Add metastore-level pruning: - Tag extraction via `extract_tags_from_query()` - Time range extraction via `refine_start_end_timestamp_from_ast()` - Build DocMapper only when query_ast is provided (no overhead for common path without index_filter) - Fix REST API tests: use `json:` key (not `json_body:`), use lowercase term values to match tokenizer behavior - Update tests to run against both quickwit and elasticsearch engines Two-level filtering now implemented: 1. Metastore level: tags + time range from query AST 2. Split level: lightweight query execution for accurate filtering Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: ruo <ruoliu.dev@gmail.com> * refactor(es-compat): use best-effort metadata filtering for index_filter Remove heavy split-level query execution for field_caps index_filter. The implementation now aligns with ES's "best-effort" approach that uses metadata-level filtering only (time range, tags) instead of opening splits and executing queries. Changes: - Remove split_matches_query function (no longer opens splits) - Remove query_ast and doc_mapper from LeafListFieldsRequest proto - Keep metadata-level filtering in root_list_fields: - Time range extraction from query AST - Tag-based split pruning - Simplify leaf_list_fields to just return fields from all splits This matches ES semantics: "filtering is done on a best-effort basis... this API may return an index even if the provided filter matches no document." Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: ruo <ruoliu.dev@gmail.com> * fix(es-compat): reject empty index_filter to match ES behavior - Remove empty object {} handling in parse_index_filter_to_query_ast - ES rejects empty index_filter with 400, now QW does too - Add tag_fields config and tag-based index_filter test - Update unit and integration tests accordingly Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: ruo <ruoliu.dev@gmail.com> * Added ref doc for the new functionality. * cargo fmt fix --------- Signed-off-by: ruo <ruoliu.dev@gmail.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: fulmicoton <paul.masurel@datadoghq.com> |
||
|
Nicolas Coiffier
|
137621a507 | Fix typo in metrics name in metrics.md (#6141) | ||
|
Paul Masurel
|
8412e6d24e |
Offload leaf search work to AWS Lambda functions (#6157)
* Offload leaf search work to AWS Lambda functions
The goal is to handle traffic spikes gracefully without provisioning additional searcher nodes: when the local search queue is saturated, overflow splits are transparently routed to Lambda for processing.
The offloading decision happens **on the leaf side**, inside the `SearchPermitProvider`. The permit provider already manages a bounded queue of pending split search tasks (gated by memory budget and download slots). When a leaf search request arrives, the provider checks the current queue depth against a configurable `offload_threshold`. If granting permits for all requested splits would exceed this threshold, only enough splits to fill up to the threshold are processed locally — the rest are marked for offloading.
The offloaded splits are batched (up to `max_splits_per_invocation` splits per batch, balanced by document count) and sent to Lambda in parallel. Each Lambda invocation runs the same leaf search code path and **returns per-split results individually**. This is important: the per-split responses are fed back into the `IncrementalCollector` and populate the **partial result cache**, so subsequent queries hitting the same splits benefit from cached results regardless of whether the split was searched locally or on Lambda.
Depending on the configuration, the Lambda function code can be **deployed automatically** at startup. The `quickwit-lambda-client` crate embeds a compressed Lambda binary at compile time. When `auto_deploy` is configured, Quickwit will:
1. Check if a published Lambda version matching the current binary already exists (identified by a description tag `quickwit:{version}-{hash}`)
2. Create or update the function and publish a new version if needed
3. Garbage-collect old versions (keeping the current one + 5 most recent)
This ensures the Lambda function always matches the running Quickwit version without any external deployment tooling. Manual deployment is also supported for users who prefer to manage Lambda functions through Terraform or other IaC tools.
Lambda offloading is opt-in. Add a `lambda` section under `searcher` in the node configuration:
```yaml
searcher:
lambda:
offload_threshold: 100 # queue depth before offloading kicks in (0 = always offload)
max_splits_per_invocation: 10
auto_deploy:
execution_role_arn: arn:aws:iam::123456789012:role/quickwit-lambda-role
memory_size: 5 GiB
invocation_timeout_secs: 15
```
- **`quickwit-lambda-client`**: Handles Lambda invocation (with metrics) and auto-deployment logic. Embeds the Lambda binary at build time.
- **`quickwit-lambda-server`**: The Lambda function handler itself — receives a `LeafSearchRequest`, runs `multi_index_leaf_search`, and returns per-split `LeafSearchResponse`s.
- **`quickwit-search`**: New `LambdaLeafSearchInvoker` trait; `SearchPermitProvider` gains `get_permits_with_offload` to split work between local and offloaded; `leaf.rs` orchestrates local and Lambda tasks in parallel.
- **`quickwit-config`**: New `LambdaConfig` and `LambdaDeployConfig` structs under `SearcherConfig`.
- **`quickwit-serve`**: Initializes the Lambda invoker at startup when configured.
- **`quickwit-proto`**: New `LeafSearchResponses` wrapper message for batched per-split responses.
|
||
|
Remi
|
bdcdd7d993 | Improve documentation regarding json fields and concatenated json fields (#6047) | ||
|
Raphaël Cohen
|
b6f853fbd2 |
feat: Add case_insensitive param to some Elastic DSL queries (#6005)
* feat: Add case_insensitive param to some Elastic DSL queries Signed-off-by: Darkheir <raphael.cohen@sekoia.io> * Apply PR review sugestion Signed-off-by: Darkheir <raphael.cohen@sekoia.io> --------- Signed-off-by: Darkheir <raphael.cohen@sekoia.io> |
||
|
Raphaël Cohen
|
bfed5c65d1 |
feat: Prefix and Wildcard in Elastic DSL (#6000)
* feat: Prefix and Wildcard in Elastic DSL Signed-off-by: Darkheir <raphael.cohen@sekoia.io> * chore: Add licence header Signed-off-by: Darkheir <raphael.cohen@sekoia.io> * feat: Add integration tests Signed-off-by: Darkheir <raphael.cohen@sekoia.io> * feat: Add support for short request format Signed-off-by: Darkheir <raphael.cohen@sekoia.io> * Apply PR review sugestion Signed-off-by: Darkheir <raphael.cohen@sekoia.io> --------- Signed-off-by: Darkheir <raphael.cohen@sekoia.io> |
||
|
Remi
|
03ed8cfb9a | Remove mentions to stream api (#5958) | ||
|
Adrien Guillo
|
4c6a293fe7 | Remove search stream endpoint (#5886) |