mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 16:09:05 +00:00
Hengfei Yang
41ff3cd9b6
Design at: #13822 ## Summary `distinct_values_*` metadata streams existed to serve one reader: the `_values` API. They are a redundant — and lossier — materialization of what the tantivy secondary index already holds, and the index has the collectors needed to replace them outright (`SimpleTopN` / `SimpleDistinct`). This PR removes the distinct streams from the write and read paths and lets the `_values` API be served by the tantivy index optimizer. ### 1. Treat distinct value fields as secondary index fields (read-time union) - `get_stream_setting_index_fields()` — the single accessor used by both the index build side (ingester parquet/pack jobs, compactor merge) and the query side (leader/follower index optimizers) — now unions `distinct_value_fields` into the effective index field set, gated on `enable_distinct_fields`. Fields that cannot be secondary-indexed (full text search keys, partition keys, reserved columns) are skipped; the values API falls back to a normal scan for those. - The distinct list is deliberately **not merged into `settings.index_fields`**: the two stay independent, so removing a distinct value field (or disabling `enable_distinct_fields`) cleanly stops its indexing with no residue, and the user-managed `index_fields` list is never mutated behind their back. - Correctness gate: a distinct-only field is considered indexed since its `added_ts` (`get_stream_setting_index_updated_at_for_fields`), so parquet files older than that are served by DataFusion scan instead of the index (`split_file_list_by_time_range`). `added_ts` is set on add and reset when `enable_distinct_fields` is re-enabled (existing behavior). For streams configured before this release the fields only start being indexed at upgrade time, so `added_ts` is additionally clamped by a release-wide cutoff `/tantivy/distinct_fields/updated_at` — a lazy marker initialized to the first access after upgrade, following the existing `ttv_secondary_index_updated_at` pattern. No data migration and no `DB_SCHEMA_VERSION` bump needed. ### 2. `_values` API always queries the original stream - Removed `can_use_distinct_stream()` and the `distinct_values_*` / `SUM(count)` / `StreamType::Metadata` rewrite from `values_inner` (GET `/_values`, formerly `values_v1`) and `build_search_request_per_field` (POST `/_values_stream`). - The generated SQL shapes are exactly what the index optimizer detects: - `SELECT f AS zo_sql_key, COUNT(*) AS zo_sql_num FROM s GROUP BY zo_sql_key ORDER BY zo_sql_num DESC, zo_sql_key ASC` + limit → `SimpleTopN` - `no_count`: `SELECT f AS zo_sql_key FROM s GROUP BY zo_sql_key ORDER BY zo_sql_key ASC` + limit → `SimpleDistinct` - The `keyword` filter (`str_match_ignore_case`) is a valid index condition, so keyword-filtered values queries stay on the index path too. ### 3. Remove distinct values generation - Dropped the per-record distinct value collection in logs and traces ingestion and the whole `core::metadata` module (channel, mem-table, flush job, shutdown hook). - Removed the now-unused `DISTINCT_FIELDS` static and the `ZO_FEATURE_DISTINCT_EXTRA_FIELDS` / `ZO_DISTINCT_VALUES_INTERVAL` / `ZO_DISTINCT_VALUES_HOURLY` config knobs, including the `SUM(count)` merge rollup the last one gated — legacy `distinct_values_*` streams now compact as ordinary metadata streams. The default fields `DISTINCT_FIELDS` carried (`service_name`, `operation_name`) are already default secondary index fields via `SQL_SECONDARY_INDEX_SEARCH_FIELDS`, so traces `_values` queries on them remain index-served. - Renamed `values_v1` to `values_inner`: the `v2` variant was removed long ago (#5087) and the distinct stream switch inside `v1` is gone now too, so the version suffix was pure history. ### 4. Default fields after this change - `service_name`, `operation_name`: default secondary index fields (unchanged, via `SQL_SECONDARY_INDEX_SEARCH_FIELDS`). - `trace_id`: now also a built-in default **bloom filter** field (`BLOOM_FILTER_DEFAULT_FIELDS`, gated on `ZO_FEATURE_DEFAULT_INDEX_FIELDS_ENABLED`), in addition to being a default index field — bloom sits on top of the secondary index and prunes files for high-cardinality equality lookups. - There is no "default distinct fields" concept anymore. ### 5. Fix surfaced by e2e: SimpleDistinct returned null values as `""` The tantivy index encodes null/missing field values as empty strings (`index_builder` adds `""` for null docs), so the `SimpleDistinct` term-dictionary walk returned `""` as a distinct value — sorted first in ascending order, it surfaced as a blank first option in dashboard variable dropdowns, which the DataFusion `GROUP BY` path never returns (its null group sorts last). This was exposed once distinct fields became index-served; `handle_simple_distinct` now skips empty terms (with a unit test). ### 6. Drive-by: fix the failing `api_fuzz_tests` CI job `CreateAlertRequestSchema` was an untagged documentation union with two identical variants, producing `oneOf: [CreateAlertRequestBody, CreateAlertRequestBody]` in the OpenAPI spec. Under `oneOf` semantics (exactly one branch must match) no instance can ever validate, so schemathesis filtered out every generated example and `api_fuzz_tests` failed on `POST /api/v2/{org_id}/alerts` on every CI run since that schema landed. The handler deserializes `CreateAlertRequestBody` directly, so the annotation now documents exactly that type; runtime behavior is unchanged. ## What stays (intentionally) - `distinct_value_fields` / `enable_distinct_fields` in stream settings: still the user-facing way to declare values-API fields; the list now drives the read-time index-field union described above. - The `distinct_value_fields` DB table that tracks dashboard/report usage of these fields (still gates removal in the settings API). - `DISTINCT_STREAM_PREFIX` helpers, retention sync and stream-deletion cleanup for existing `distinct_values_*` streams — legacy stream data keeps compacting (as ordinary metadata streams) and ages out via retention; no new records are written to them. ## Testing - `cargo check --workspace --all-targets` and `cargo clippy --workspace --all-targets` clean. - Enterprise build verified with `Cargo.toml.openobserve` (`cargo check --all-targets`). - New unit tests for the index-field union (skip rules for FTS/partition/reserved fields, `enable_distinct_fields` gating) and the `added_ts` / release-cutoff indexed-since resolution; schema, infra, config and api-search unit tests pass. A few pre-existing failures on a clean tree (config `ssrf_guard`, one flaky sqlite-backed destinations test, one `.env`-dependent index-fields test) are local-environment issues and unrelated. Closes #13822