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

Adds bytes and entries to chunk metadata in tsdb #5414

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Feb 16, 2022

This naively adds the number of bytes and entries for each chunk as part of it's metadata in tsdb. We may store more optimized (smaller) forms in the future, such as KB instead of bytes and/or use uint32 instead of uint64 to store them, but I decided to take the naive path first.

@owen-d owen-d requested a review from a team as a code owner February 16, 2022 20:53
@@ -4,7 +4,15 @@ package index
type ChunkMeta struct {
Checksum uint32

// Time range the data covers.
// When MaxTime == math.MaxInt64 the chunk is still open and being appended to.
// Nanosecond precision
Copy link
Contributor

Choose a reason for hiding this comment

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

Milliseconds?

Copy link
Member Author

@owen-d owen-d Feb 16, 2022

Choose a reason for hiding this comment

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

I'm intending to store nanoseconds here, but the code we have so far doesn't yet use time.Time anywhere to populate this field -- it's just an int64 so I added the comment to give more clarity.

pkg/storage/tsdb/index/index.go Show resolved Hide resolved
// should have little effect as long as there is enough memory available
Bytes uint64

Entries uint32
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: I assume this is number of log entries that are part of the particular chunk correct?. I'm trying to understand why uint32 here and how it correlates with max size of each chunk?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, exactly. I used uint64 for bytes (rationale in the comment) but uint32 for the entries count for space reduction purposes. That make sense?

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

@owen-d owen-d merged commit 137a28d into grafana:main Feb 17, 2022
@owen-d
Copy link
Member Author

owen-d commented Feb 18, 2022

ref #5428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants