Conversation
fulmicoton-dd
changed the title
fulmicoton-dd
force-pushed
the
isolating-metrics2
branch
2 times, most recently
from
6ff57fb to
6436347
Compare
fulmicoton-dd
force-pushed
the
isolating-metrics2
branch
from
e17efd3 to
e8171e5
Compare
fulmicoton-dd
force-pushed
the
isolating-metrics2
branch
from
8ab18ee to
ad69cad
Compare
|
@codex review |
![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: ad69cadaa0
ℹ️ 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".
| self.spawn_log_pipeline( | ||
| ctx, | ||
| indexing_pipeline_id.clone(), | ||
| index_config, | ||
| source_config, |
There was a problem hiding this comment.
In non-metrics builds this branch unconditionally calls spawn_log_pipeline, so a metrics index task is silently started on the logs pipeline instead of being rejected. In practice that means Arrow IPC metric payloads go through DocProcessor, get counted as invalid docs, and checkpoints can still advance, which drops data rather than surfacing an unsupported-feature error. Please add an explicit metrics-index guard in this path and fail pipeline creation when metrics support is disabled.
Useful? React with 👍 / 👎.
| indexing_setting: self.params.indexing_settings.clone(), | ||
| }; | ||
| let source = ctx | ||
| .protect_future(quickwit_supported_sources().load_source(source_runtime)) |
There was a problem hiding this comment.
The metrics pipeline now loads sources from quickwit_supported_sources(), which includes source types that are not metrics-compatible (e.g. non-Arrow producers). That changes prior fail-fast behavior into runtime processing where ParquetDocProcessor records format errors while checkpoints may still progress, creating a silent data-drop path for misconfigured metrics indexes. This should keep the metrics-specific source loader behavior (or perform equivalent validation before spawning).
Useful? React with 👍 / 👎.
2 participants
This PR attempts to isolate the metrics pipeline code from the rest of quickwit.
It only does the work on the indexing crate for the moment. The target is :
The main trick used is in this PR have been to remove the proliferation of generics introduced in #6244.
This was done by introducing an abstract notion of a source sink and an abstract notion of pipeline.