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

Use prometheus/tsdb in ingester. #1427

Closed
wants to merge 2 commits into from
Closed

Conversation

tomwilkie
Copy link
Contributor

This PR is the first attempt at using prometheus/tsdb in the ingester.

There is lots missing/TODO:

  • Add series limits to TSDB to match existing Cortex limits.
  • Upstream TSDB changes for exposing chunks on series interface.
  • Figure out how to do per user metrics for TSDB (number of series, ingestion rate etc).
  • Ensure TSDB errors for things like out-of-order etc are being handled correctly.
  • Figure out how to handle transfers for TSDB.
  • Figure out how to do upgrades.
  • Implement flushing TSDB to object store
  • Allow index entries to point to sub-objects
  • Benchmark Bigtable vs in-object-store index.

@codesome
Copy link
Contributor

I can pick TSDB changes to be made 1 by 1, starting with series limit.

@codesome
Copy link
Contributor

Limit series in TSDB: prometheus-junkyard/tsdb#617

@cstyan
Copy link
Contributor

cstyan commented Jun 3, 2019

Figure out how to do per user metrics for TSDB (number of series, ingestion rate etc). Could we achieve this by just having a separate TSDB data dir per user ID?

Edit: I think we could then extend that to just make it easy to get the # of active series in TSDB (looks like right now we just have the gauge metric) rather than having to add a limit to TSDB itself, and then Cortex can ask TSDB for that value when checking if there's room for more series for a given user?

@codesome
Copy link
Contributor

codesome commented Jun 4, 2019

@cstyan Yes, after I opened the PR in TSDB to have limits on number of series, it looks unnecessary to have it in TSDB. We should be able to handle that in cortex (with or without TSDB per user ID).
Also, wouldn't it be too many database instances to handle if we have TSDB per user ID?

@@ -53,6 +53,9 @@ type Series interface {

// Iterator returns a new iterator of the data of the series.
Iterator() SeriesIterator

// Chunks returns a copy of the compressed chunks that make up this series.
Chunks() []chunks.Meta
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this coming from? I cannot see this on tip of TSDB 🤔 nor 0.8.0 which go mod vendors here.

Is this manually modifed TSDB code in vendor? Cannot see any replace directive on go.mod as well so you don't use fork as well...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added it here; opening a TSDB PR now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bboreham bboreham mentioned this pull request Oct 2, 2019
@tomwilkie
Copy link
Contributor Author

Closing in favour of #1695.

@tomwilkie tomwilkie closed this Oct 3, 2019
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.

4 participants