Skip to content

Breakdown tsdb/head.go into multiple files#9147

Merged
bwplotka merged 1 commit intoprometheus:mainfrom
codesome:head-refactor
Aug 3, 2021
Merged

Breakdown tsdb/head.go into multiple files#9147
bwplotka merged 1 commit intoprometheus:mainfrom
codesome:head-refactor

Conversation

@codesome
Copy link
Member

@codesome codesome commented Aug 3, 2021

head.go has grown very big and lots of things have spread around randomly, making it hard to find stuff and read the code. In this PR I am proposing to break this down into the following files:

  1. head_append.go - all the code for appending stuff (samples, exemplars, future histograms, etc)
  2. head_read.go - all the code for reading the series/chunks from the Head
  3. head_wal.go - current WAL replay code, and soon, the snapshot code would go into this.
  4. head.go - remaining stuff, i.e., structs (for head, series, stripe series, chunk etc), GC/truncation of head, helper functions for the structs

This PR merely moves code around and is a no-op. So if the contents in the new files makes sense, then that should be enough of a check.

NOTE: this will require a rebase of existing open TSDB PRs and future PRs that are in progress, but that will be required anytime we break head.go down (or refactor), and the breakdown would be required sooner or later.

@roidelapluie
Copy link
Member

Would we benefit from a head package instead?

@codesome
Copy link
Member Author

codesome commented Aug 3, 2021

Would we benefit from a head package instead?

I thought about it, but in many places (including non Head tests) we use the non exported method and fields of the Head. While it makes sense, it would be a bigger refactor touching other files, and I wont not be able to commit to it myself at the moment. So this should be a good first step before we have a head package.

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Copy link
Member

@roidelapluie roidelapluie left a comment

Choose a reason for hiding this comment

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

Approved, but let's try to get Bartek's approval as well

@codesome
Copy link
Member Author

codesome commented Aug 3, 2021

I will wait till EOD to merge and address any concerns later if Bartek cannot come to it today. Because any other TSDB PR merges will make updating this PR tricky :)

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice. I don't mind - it really depends on what IDEs ppl are using. No harm either. Thanks!

@bwplotka bwplotka merged commit 8002a3a into prometheus:main Aug 3, 2021
austince pushed a commit to austince/prometheus that referenced this pull request Aug 4, 2021
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
@codesome codesome deleted the head-refactor branch August 6, 2021 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants