Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5117 disable per index metrics #5125

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Conversation

fulmicoton
Copy link
Contributor

No description provided.

@fulmicoton fulmicoton force-pushed the 5117-disable-per-index-metrics branch 2 times, most recently from 670e7cd to 365f706 Compare June 14, 2024 08:09
@fulmicoton fulmicoton force-pushed the 5117-disable-per-index-metrics branch 2 times, most recently from c65adee to ab4e806 Compare June 14, 2024 08:14
Copy link

github-actions bot commented Jun 14, 2024

On SSD:

Average search latency is 1.0x that of the reference (lower is better).
Ref run id: 2141, ref commit: 1c2ec26
Link

On GCS:

Average search latency is 0.961x that of the reference (lower is better).
Ref run id: 2142, ref commit: 1c2ec26
Link

@fulmicoton fulmicoton force-pushed the 5117-disable-per-index-metrics branch 2 times, most recently from fb373f4 to cd6594b Compare June 14, 2024 10:32
variable.

By default they are enabled.
If the `PER_INDEX_METRICS_ENABLED` environment variable is set to false,
then all of the index metrics are grouped under the index_id `__any__`.

In addition, this PR refactors a little bit the way we handled the
docprocessor metrics. We now cache the docprocessor counters, hence
preventing 1 hash lookup per document.
@fulmicoton fulmicoton force-pushed the 5117-disable-per-index-metrics branch from cd6594b to 1e0e2fa Compare June 14, 2024 10:35
Copy link
Member

@guilload guilload left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice little refactor.

quickwit/quickwit-common/src/metrics.rs Outdated Show resolved Hide resolved
quickwit/quickwit-common/src/metrics.rs Outdated Show resolved Hide resolved
@@ -395,7 +432,7 @@ impl DocProcessor {
if cfg!(not(feature = "vrl")) && transform_config_opt.is_some() {
bail!("VRL is not enabled: please recompile with the `vrl` feature")
}
let doc_processor = Self {
Ok(DocProcessor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want me to edit the contribution guidelines and prohibit Self? :D

@fulmicoton fulmicoton merged commit 0555978 into main Jun 17, 2024
5 checks passed
@fulmicoton fulmicoton deleted the 5117-disable-per-index-metrics branch June 17, 2024 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants