Skip to content

Make max chunk size 1hr #118

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

Merged
merged 2 commits into from
Nov 7, 2016
Merged

Make max chunk size 1hr #118

merged 2 commits into from
Nov 7, 2016

Conversation

tomwilkie
Copy link
Contributor

Also, schedule series for flushing if it has >1 chunk.

@tomwilkie
Copy link
Contributor Author

Will need post-merge review #97

@tomwilkie tomwilkie merged commit 357588f into master Nov 7, 2016
@tomwilkie tomwilkie deleted the chunk-size branch November 7, 2016 21:34
This was referenced Nov 7, 2016
@@ -515,7 +515,7 @@ func (i *Ingester) flushSeries(u *userState, fp model.Fingerprint, series *memor
}

firstTime := series.chunkDescs[0].FirstTime()
flush := immediate || model.Now().Sub(firstTime) > i.cfg.MaxChunkAge
flush := immediate || len(series.chunkDescs) > 1 || model.Now().Sub(firstTime) > i.cfg.MaxChunkAge
Copy link
Contributor

Choose a reason for hiding this comment

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

Why strictly greater than one? I could understand > 0, but > 1 seems a bit odd. I realise at this point we know that it's non-empty, but why do we not want to flush if it has only 1 thing?

Somewhat out of scope for this PR, but it's difficult for me to reason locally about fpLocker. Any reader of this method needs to know about its call-site to understand why locking is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why strictly greater than one?

We need to flush something in the series if:

  • we are existing (immedidate)
  • a chunk has be filled by the incoming data and a new one created (len > 1)
  • the series (and by implication, the one and only chunk) is older than the threshold

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. The logic is that we always flush closed chunks, or open old chunks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More of less. Series.Add will 'close' chunks when they get full:

https://github.com/weaveworks/cortex/blob/master/ingester/series.go#L141

(chunks don't actually have a closed concept, we just track it for the head chunk specially)

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