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

allow configuring a target chunk size in compressed bytes #1406

Merged
merged 2 commits into from
Dec 12, 2019

Conversation

slim-bean
Copy link
Collaborator

Need to figure out still what to do with the error if you fail to cut a chunk...

Signed-off-by: Edward Welch edward.welch@grafana.com

Signed-off-by: Edward Welch <edward.welch@grafana.com>
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Nice! This is just a quick first pass.

pkg/chunkenc/gzip.go Outdated Show resolved Hide resolved
pkg/ingester/stream.go Show resolved Hide resolved
@@ -267,8 +267,16 @@ The `ingester_config` block configures Ingesters.
# period as long as they receieve no further activity.
[chunk_idle_period: <duration> | default = 30m]

# The maximum size in bytes a chunk can be before it should be flushed.
# The maximum _uncompressed_ size in bytes of a chunk block,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is 100% accurate: almost all blocks are going to end up being greater than this size. This is more like the cuttoff point for when an uncompressed block should be compressed.

Do you think it's worth explaining that in the docs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah agree, will reword this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rfratto reworded, let me know what you think

Copy link
Member

Choose a reason for hiding this comment

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

Nice, this is more clear. Thanks!

pkg/ingester/stream.go Outdated Show resolved Hide resolved
pkg/chunkenc/gzip.go Outdated Show resolved Hide resolved
Signed-off-by: Edward Welch <edward.welch@grafana.com>
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

LGTM!

@slim-bean slim-bean merged commit 5aa82ba into master Dec 12, 2019
@slim-bean slim-bean deleted the target-chunk-size branch December 12, 2019 19:17
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
Limiting chunks query start time with config
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.

3 participants