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

Headblock interop #3995

Merged
merged 3 commits into from
Jul 19, 2021
Merged

Headblock interop #3995

merged 3 commits into from
Jul 19, 2021

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Jul 13, 2021

This PR is the next one working towards enabling out of order writes in Loki. Notably, it

  1. Introduces a HeadBlock interface type and ensures both ordered & unordered variants implement it.
  2. Separates Chunk vs HeadBlock format in WAL checkpoints and enables recovering from them in an interoperable fashion.

ref #1544

@owen-d owen-d requested a review from cyriltovena July 13, 2021 20:41
@owen-d owen-d mentioned this pull request Jul 15, 2021
Copy link
Contributor

@sandeepsukhani sandeepsukhani left a comment

Choose a reason for hiding this comment

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

I just added a minor suggestion irrelevant to your code.
The changes look good to me!

@@ -74,7 +88,7 @@ type MemChunk struct {
cutBlockSize int

// Current in-mem block being appended to.
head *headBlock
head HeadBlock

// the chunk format default to v2
Copy link
Contributor

Choose a reason for hiding this comment

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

While we are at it, If I am not wrong, we default to v3 now, and I think we should update this comment too?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its not yet active since NewMemChunk still creates an ordered head.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found the DefaultChunkFormat being set to v3. Am I looking at the wrong place?

Copy link
Contributor

Choose a reason for hiding this comment

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

No I missed it

Copy link
Member Author

Choose a reason for hiding this comment

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

This is covered in the followup PR #4001 :)

}

func (hb *unorderedHeadBlock) Reset() {
x := newUnorderedHeadBlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be interesting instead to use https://github.com/golang-collections/go-datastructures/blob/59788d5eb259/rangetree/entries.go#L33 which will attemps to reuse entries.

Copy link
Member Author

Choose a reason for hiding this comment

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

ooh great catch

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 a4db1a4 into grafana:main Jul 19, 2021
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.

None yet

3 participants