Remove Parquet code - #6809
Remove Parquet code#6809nadav-govari wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b243001f0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[serde(default)] | ||
| pub merge_policy: MergePolicyConfig, | ||
| /// Merge policy for Parquet (metrics/sketches) splits. | ||
| #[cfg(feature = "metrics")] | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub parquet_merge_policy: Option<ParquetMergePolicyConfig>, | ||
| /// Parquet-specific indexing settings (sort schema, window duration). | ||
| #[cfg(feature = "metrics")] | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub parquet_indexing: Option<ParquetIndexingConfig>, | ||
| #[serde(default)] | ||
| pub resources: IndexingResources, |
There was a problem hiding this comment.
When upgrading a metastore containing an index configured with parquet_merge_policy or parquet_indexing, persisted IndexMetadata is deserialized through this #[serde(deny_unknown_fields)] struct, so those former fields now make the entire index metadata unreadable—even for deleting the obsolete index—and bulk index-list consumers may fail because of one such entry. Preserve a versioned deserialization/migration path for stored metadata and document the required upgrade behavior before rejecting these fields in new configurations.
AGENTS.md reference: AGENTS.md:L23-L24
Useful? React with 👍 / 👎.
75695e2 to
647de6a
Compare
647de6a to
de81562
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de8156274a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let pipeline_handle: BoxedPipelineHandle = self | ||
| .spawn_log_or_metrics_pipeline( | ||
| let pipeline_handle: IndexingPipelineHandle = self | ||
| .spawn_indexing_pipeline( |
There was a problem hiding this comment.
When upgrading with pre-existing OTLP metrics or sketch records still queued in the ingest-v2 WAL, this unconditional spawn_indexing_pipeline replaces the former ID-based MetricsPipeline dispatch. Those records contain Arrow IPC payloads, so the standard DocProcessor interprets them as JSON, rejects every document, and publishes an EmptySplit checkpoint specifically to avoid retrying faulty documents; the queued metrics are therefore irreversibly skipped. Detect and reject these legacy indexes or provide a drain/migration path before routing them through the standard pipeline.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
| @@ -148,13 +129,11 @@ impl From<FileBackedIndexV0_8> for FileBackedIndex { | |||
| per_source_shards.insert(source_id.clone(), Shards::empty(index_uid, source_id)); | |||
| } | |||
| } | |||
| Self::new_with_metrics_splits( | |||
| Self::new( | |||
There was a problem hiding this comment.
When a file-backed metastore contains nonempty metrics_splits or sketch_splits, the revised v0.9 serde struct silently ignores those now-unknown fields and this conversion rebuilds the index without them. The next metadata mutation rewrites the file and permanently loses the only paths and sizes for those objects; both GC and index deletion now enumerate only regular splits, so the Parquet files remain orphaned in storage. Retain the legacy fields long enough to delete their files, or perform an explicit migration before dropping them.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
|
Hey, does metrics support in Quickwit is abandonned? Or is it still planned but everything is removed and will be add back later with a complete feature? |
The current approach is abandonned. Will probably come back later. |
3 participants
Description
Removes the unused Parquet code. Commit can be reverted if we want it back.
How was this PR tested?
Unit tests all pass.