-
Notifications
You must be signed in to change notification settings - Fork 561
Reduce log volume #6568
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
base: main
Are you sure you want to change the base?
Reduce log volume #6568
Changes from all commits
1bb7502
ae41e11
45f9832
02d3099
29d1527
33717b6
7e7a208
0501791
697c223
4a23e3c
0e5c3c3
c6b8750
78c91c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,15 +189,13 @@ fn compute_load_per_shard(shard_entries: &[&ShardEntry]) -> NonZeroU32 { | |
| } | ||
|
|
||
| fn get_default_load_per_shard() -> NonZeroU32 { | ||
| static DEFAULT_LOAD_PER_SHARD: LazyLock<NonZeroU32> = LazyLock::new(|| { | ||
| let default_load_per_shard = quickwit_common::get_from_env( | ||
| "QW_DEFAULT_LOAD_PER_SHARD", | ||
| PIPELINE_FULL_CAPACITY.cpu_millis() / 4, | ||
| false, | ||
| ); | ||
| NonZeroU32::new(default_load_per_shard).unwrap() | ||
| }); | ||
| *DEFAULT_LOAD_PER_SHARD | ||
| let default_load_per_shard = quickwit_common::get_from_env_cached!( | ||
| u32, | ||
| "QW_DEFAULT_LOAD_PER_SHARD", | ||
| PIPELINE_FULL_CAPACITY.cpu_millis() / 4, | ||
| false | ||
| ); | ||
| NonZeroU32::new(default_load_per_shard).unwrap() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: I don't see the benefit of a macro here. Are we trying to same one LOC? Less nitpick: if introducing a macro is something we want to do, it should be a separate PR.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main benefit is to avoid this issue in the future by adding a central way that includes caching. I can put it in a different PR
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I liked what we did before because this is macro, but this nitpick/ personal taste. Macro is ok, but it should be in a separate file, and cover ALL places where we get env. This is NOT the case today. |
||
| } | ||
|
|
||
| fn get_sources_to_schedule(model: &ControlPlaneModel) -> Vec<SourceToSchedule> { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, we should have a env.rs file in quickwit-common to gather all of these helpers.
Also not by implementing your macro only for a two calls, you creating a fracture in the way we deal with env variable.
Some stuff use a macro, some not.