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

Add Chunk.Utilization() methods #2055

Merged
merged 1 commit into from
Oct 6, 2016
Merged

Add Chunk.Utilization() methods #2055

merged 1 commit into from
Oct 6, 2016

Conversation

juliusv
Copy link
Member

@juliusv juliusv commented Oct 5, 2016

When using the chunking code in other projects (both Weave Prism and
ChronixDB ingester), you sometimes want to know how well you are
utilizing your chunks when closing/storing them.

@juliusv
Copy link
Member Author

juliusv commented Oct 5, 2016

@beorn7

@@ -322,6 +322,11 @@ func (c varbitChunk) UnmarshalFromBuf(buf []byte) error {
// encoding implements chunk.
func (c varbitChunk) Encoding() Encoding { return Varbit }

// Utilization implements chunk.
func (c varbitChunk) Utilization() float64 {
return float64(c.nextSampleOffset()) / float64(cap(c))
Copy link
Member

Choose a reason for hiding this comment

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

You might want to count the footer part at the end of the chunk as used, too. Not sure about the exact math, but with the current implementation, your utilization can never reach 100% (which might or might not be what you want).

@beorn7
Copy link
Member

beorn7 commented Oct 6, 2016

Just a math nit. Decide yourself what you need for your use case. Otherwise 👍

When using the chunking code in other projects (both Weave Prism and
ChronixDB ingester), you sometimes want to know how well you are
utilizing your chunks when closing/storing them.
@juliusv
Copy link
Member Author

juliusv commented Oct 6, 2016

Updated varbit utilization function together with @beorn7.

@juliusv
Copy link
Member Author

juliusv commented Oct 6, 2016

👍 from @beorn7 in person

@juliusv juliusv merged commit 2844a8c into master Oct 6, 2016
@juliusv juliusv deleted the utilization branch October 6, 2016 14:36
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