TSDB data import tool for OpenMetrics format. #5887

dipack95 · 2019-08-13T15:51:05Z

Created a tool to import data formatted according to the Prometheus exposition format. The tool can be accessed via the TSDB CLI.

closes #535

Signed-off-by: Dipack P Panjabi dipack.panjabi@gmail.com

(Port of prometheus-junkyard/tsdb#671)

dipack95 · 2019-08-13T17:35:11Z

(Copying my response to the last comment on the earlier PR for clarity: prometheus-junkyard/tsdb#671 (comment))

@codesome If I understand correctly, you're looking for a block structure as follows:

1. Overlapping time range

Existing data: |---------|-----------|----------|
New data:      |----     |-----      |--------  |

2. Non-overlapping time range

Existing data: 				|---------|-----------|----------|
New data: 		|----|----| (in 2h blocks)

One road-block I see is that we would have to take down the running TSDB instance, in case the data being imported overlaps with the current data, and then force a compaction. The default mode of operation would only work if the data does not overlap, but we could certainly add a force compaction stage for overlapping data using a flag or some such.

krasi-georgiev

Sorry for the long delay.

Enough time has passed and noone has commented anything about the format so lets leave it as expfmt and can add another one if/when needed.

tsdb/importer/import.go

tsdb/importer/import-trial.go

tsdb/importer/util.go

tsdb/importer/import_test.go

tsdb/importer/import.go

codesome · 2019-09-19T12:53:56Z

@dipack95 sorry for the delay too.
Replying to your comment: Yes. More like

Instead of this
Existing data: |---------|-----------|----------|
New data:      |----     |-----      |--------  |

We will have this
Existing data: |---------|-----------|----------|
New data:      |----|    |-----|     |--------|

And there is already a flag to allow overlapping blocks which takes care of merging the blocks. So if that flag is not enabled, then yes you have to restart Prometheus with that flag enabled.

dipack95 · 2019-09-19T23:10:20Z

Thanks for the review @krasi-georgiev, @codesome, I should have some free time this weekend to implement your suggestions. Hopefully, I'll have a refreshed PR by next week.

dipack95 · 2019-09-22T23:28:00Z

I've made quite a few changes in the last few commits, to bring them in line with your suggestions,

Inferring the block ranges from an existing database, and lining up overlapping imported data with them, to match their time ranges best, as discussed with @codesome
Instead of pulling all the data to be imported into program memory, I flush them periodically to disk, inline with Brian's (@brian-brazil) suggestion on the earlier version of this PR
Changed the testing strategy to test just the public API of the importer, instead of the internal implementation, according to @krasi-georgiev

There are a few other minor changes that are yet to be implemented, like shuffling around a couple of util functions to be part of prometheus/pkg, and also exposing the createHead, and createBlock functions, and I will implement those once the rest of the code has passed review!

krasi-georgiev · 2019-09-23T10:56:56Z

Inferring the block ranges from an existing database, and lining up overlapping imported data with them, to match their time ranges best, as discussed with @codesome

I still think that this is unnecessary. Passing ranges leaves the responsibility to the user and can just give some examples for best practices. Different users will have different requirements. Imagine in the case of Thanos which I think only uploads 2h blocks. In this case you would want to import all blocks to be 2h.

tsdb/importer/import_old.go

tsdb/importer/import.go

krasi-georgiev · 2019-09-23T11:48:04Z

I still think that this is unnecessary. Passing ranges leaves the responsibility to the user and can just give some examples for best practices. Different users will have different requirements. Imagine in the case of Thanos which I think only uploads 2h blocks. In this case you would want to import all blocks to be 2h.

after a chat with @codesome I do agree that passing block ranges would be too much hassle for the user so lets leave it as is.

@bwplotka In the case of Thanos do you have any idea/suggestion how should backfiling/importing data should work?
I remember that the sidecar would only upload the latest 2h block so need to also think about that use case as well. Or maybe this importing/backfilling should be implemented with the compactor.

dipack95 · 2019-09-23T14:04:21Z

In any case, if the importer cannot find any existing blocks to try and align the imported data with, it cuts the imported data into 2h blocks, so that should help with the Thanos use case.

dipack95 · 2019-09-23T14:33:13Z

Odd, one of the tests, in a completely unrelated part of the code, has failed.

krasi-georgiev · 2019-09-23T14:37:40Z

yes this is a flaky test and I think Simon is working on a fix

dipack95 · 2019-09-23T16:31:13Z

Alright, looks like the builds + tests went through fine this time!

dipack95 · 2019-09-27T20:46:30Z

@krasi-georgiev @codesome Does the PR look to be in good shape now?

pkg/value/value.go

tsdb/importer/import_test.go

tsdb/importer/import.go

dipack95 · 2019-10-09T14:06:11Z

Regarding exporting the createHead and createBlock functions, I've found that I get the go list related errors from golangci-lint only when I make those functions part of tsbdutil, it works fine when those same functions are part of a different package testutil or something.
I haven't managed to understand why that happens, though.

krasi-georgiev · 2019-10-10T09:18:02Z

could you add the changes that trigger the error when part of tsdbutil and I can also have a look.

dipack95 · 2019-10-10T15:43:22Z

@krasi-georgiev I've added the breaking changes.

The CircleCI test breaks with the error I receive on my machine, but the Travis build goes through that bit fine, but breaks at the test stage complaining about circular dependencies.

krasi-georgiev · 2019-10-14T12:23:37Z

I looked into it and the only way to avoid the circular import is to put CreateHead and CreateBlock in block.go in the tsdb root dir.

The problem is that the tsdb tests use tsdbutil can't import tsdb.
I think the proper way to fix this is to extract some components from tsdb root and place them in their own packages. I have opened a new issue for that. #6136

dipack95 · 2019-10-14T15:30:56Z

Okay, I'll revert my most recent commit then, to allow the builds to go through + merge

krasi-georgiev · 2019-10-15T08:18:17Z

I still think we should reuse these funcs to please put them in a file tsdbutil.go in the tsdb root

dipack95 · 2019-10-15T14:58:00Z

Oh, I thought that the new issue you've opened would take care of that as well.

krasi-georgiev · 2019-10-15T15:04:09Z

my idea is to reuse the func anyway and in the issue just refactor the code to allow better package structure.

dipack95 · 2019-10-15T15:12:09Z

my idea is to reuse the func anyway and in the issue just refactor the code to allow better package structure.

Makes sense, I've moved the functions to tsdbblockutil.go in tsdb/.

brian-brazil

I'm not sure this is capable of dealing with real world data.

tsdb/importer/import.go

krasi-georgiev · 2019-10-22T08:16:22Z

ping @dipack95

bwplotka

Nice!

It's looking very good. I plan to extend it a bit with CSV support, in next PR basing on yours 🤗

The only major comment is related to input. I would suggest just specifying output dir nothing else. What do you think? Explained below.

cc @gouthamve

bwplotka · 2020-07-12T08:41:32Z

docs/storage.md

+An example import command is: `tsdb import rrd_exported_data.txt /var/lib/prometheus/ --max-samples-in-mem=10000`
+
+You can increase `max-sample-in-mem` to speed up the process, but the value 10000 seems a good balance.
+This tool will create all Prometheus blocks (see [On-disk layout][On-disk layout] above), in a temporary workspace. By default temp workspace is /tmp/ according to the $TMPDIR env var, so you can change it if you have disk space issues (`TMPDIR=/new/path tsdb import [...]`) !


I need to agree here.

I would simplify configuration here. What about just following flow:

Create new blocks in given directory. It's up to user if this dir is tmp, Prometheus Dir or White House Google Drive. Does not matter. We just write blocks as <ID>.tmp and use os.Rename to avoid partial reads. Done.

What do you think? (:

bwplotka · 2020-07-12T08:43:35Z

tsdb/importer/import_test.go

+	"strings"
+	"testing"
+
+	labels2 "github.com/prometheus/prometheus/pkg/labels"


No need for this being label2 anymore (:

bwplotka · 2020-07-12T08:46:00Z

tsdb/importer/import_test.go

+		},
+		{
+			ToParse: `# HELP http_requests_total The total number of HTTP requests.
+# TYPE http_requests_total counter


Nice, but I am missing test that actually test import of OpenMetircs/Prom exposition with multiple samples 🤔
Is this supported?

Yes, the test named TestImportIntoExistingDB is for exactly that case, where we import multiple samples into the DB.

Why not having this in the same TableTest? Why complicating? (:

Also I checked that test

// genOpenMetricsText formats the given series data into OpenMetrics compliant format. func genOpenMetricsText(metricName, metricType string, series []tsdb.MetricSample) string { str := fmt.Sprintf("# HELP %s This is a metric\n# TYPE %s %s", metricName, metricName, metricType) for _, s := range series { str += fmt.Sprintf("\n%s%s %f %d", metricName, labelsToStr(s.Labels), s.Value, s.Timestamp) } str += fmt.Sprintf("\n# EOF") return str }

This is not allowing me to inject many samples for single series, just single sample, no?

So how do user import series with let's say 100 samples? (:

bwplotka · 2020-07-12T08:48:11Z

tsdb/importer/importer.go

+// It iterates through the given durations, creating progressive blocks, and once it reaches
+// the last given duration, it divvies all the remaining samples into blocks of that duration.
+// Input blocks are assumed sorted by time.
+func binBlocks(blocks []*newBlockMeta, durations []int64) [][]*newBlockMeta {


What bin means here? Can we have more verbose function name? (:

Yeah, that function basically creates bins, or tranches, of the given samples corresponding to the provided durations. I thought bin was an accurate term for it

bin like... garbage bin? 🤔

bwplotka · 2020-07-12T08:49:01Z

tsdb/importer/importer.go

+		return err
+	}
+
+	level.Debug(logger).Log("msg", "copying newly created blocks from temp location to actual DB location")


What if location is remote?

I haven't tested that specific case, but that seems like something the OS / golang libs itself should handle right?

How? The key is to not overcomplicate the program. I would suggest simple thing. Just output dir and nothing more. (:

bwplotka · 2020-07-12T08:49:44Z

tsdb/importer/import_test.go

+	testutil.Assert(t, r.Len() > 0, "import text has been completely read")
+}
+
+// func benchVirtual(vThresh int, b *testing.B) {


Avoid commented, dead code in the PR. It makes this PR looks like it's unfinished 🤗

Yeah I think I meant to remove this before merge.

bwplotka · 2020-07-12T08:54:42Z

tsdb/importer/importer.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package importer


Suggested change

package importer

package import

Usually packages are noun, then internal structs can be -er

Fair enough

bwplotka · 2020-07-12T08:55:13Z

Also cc @JessicaGreben as there is much to reuse from this PR

bwplotka · 2020-07-12T08:59:02Z

Also I would be curious if we can find better name for this command. import might be misleading. It does not import anything TBH just produces TSDB blocks ^^ cc @krasi-georgiev @roidelapluie @brian-brazil @dipack95

bwplotka · 2020-07-12T09:08:19Z

tsdb/cmd/tsdb/main.go

+		dumpMinTime              = dumpCmd.Flag("min-time", "minimum timestamp to dump").Default(strconv.FormatInt(math.MinInt64, 10)).Int64()
+		dumpMaxTime              = dumpCmd.Flag("max-time", "maximum timestamp to dump").Default(strconv.FormatInt(math.MaxInt64, 10)).Int64()
+		importCmd                = cli.Command("import", "import samples from file containing information formatted in the Open Metrics format. Please refer to the storage docs for more details.")
+		importFilePath           = importCmd.Arg("file path", "file to import samples from (must be in Open Metrics format)").Required().String()


I would just ask for input (: so we can do

cat X | promtool import /path/here (:

I have almost ready PR with this improvement if you want tot take a look

bwplotka · 2020-07-12T09:08:46Z

tsdb/cmd/tsdb/main.go

+		dumpPath                 = dumpCmd.Arg("db path", "database path (default is "+defaultDBPath+")").Default(defaultDBPath).String()
+		dumpMinTime              = dumpCmd.Flag("min-time", "minimum timestamp to dump").Default(strconv.FormatInt(math.MinInt64, 10)).Int64()
+		dumpMaxTime              = dumpCmd.Flag("max-time", "maximum timestamp to dump").Default(strconv.FormatInt(math.MaxInt64, 10)).Int64()
+		importCmd                = cli.Command("import", "import samples from file containing information formatted in the Open Metrics format. Please refer to the storage docs for more details.")


probably it should be rather gen-block instead of import 🤔

I think import makes sense because you're generating blocks, and placing them in the db after the fact, which is an import operation

So we cannot place them into DB, that's something I would see in this tool to separate. Separation of concern essentially.

bwplotka · 2020-07-12T09:12:58Z

tsdb/cmd/tsdb/main.go

+		importFilePath           = importCmd.Arg("file path", "file to import samples from (must be in Open Metrics format)").Required().String()
+		importDbPath             = importCmd.Arg("db path", "database path").Required().String()
+		importMaxSamplesInMemory = importCmd.Flag("max-samples-in-mem", "maximum number of samples to process in a cycle").Default("10000").Int()
+		importMaxBlockChildren   = importCmd.Flag("max-block-children", "maximum number of children a block can have at a given time").Default("20").Int()


Why we care to align blocks? 🤔 Might make sense but requires really a good documentation explanation

We need to align blocks to avoid compaction issues down the line with the newly created blocks, as explained by @codesome here

Cool, let's document it more 🤗

dipack95 · 2020-07-12T16:00:45Z

FYI: @dipack95 as per out DevSummit yesterday we agreed we want to support both OpenMetrics like format and CSV, so let's finish this PR hugs
Are you available?

Awesome @bwplotka! Yes I am available. Will rebase this PR and apply your suggestions in the next few days.

bwplotka · 2020-07-13T17:42:51Z

@dipack95 are you on CNCF slack (https://slack.cncf.io/) maybe? So we can talk offline as well? 🤗 I am @bwplotka there

bwplotka · 2020-07-13T18:24:54Z

tsdb/cmd/tsdb/main.go

-		dumpPath             = dumpCmd.Arg("db path", "database path (default is "+defaultDBPath+")").Default(defaultDBPath).String()
-		dumpMinTime          = dumpCmd.Flag("min-time", "minimum timestamp to dump").Default(strconv.FormatInt(math.MinInt64, 10)).Int64()
-		dumpMaxTime          = dumpCmd.Flag("max-time", "maximum timestamp to dump").Default(strconv.FormatInt(math.MaxInt64, 10)).Int64()
+		cli                      = kingpin.New(filepath.Base(os.Args[0]), "CLI tool for tsdb")


Just curious why this tool is in TSDB tool?

Is this really what we want? 🤔 Just double checking, I think we all talked about promtool.

Looks to me overall that it would be nice to move tsdb tool to subcommand of promtool tsdb... but we can do that as separate piece of work (:

bwplotka · 2020-07-14T09:03:44Z

tsdb/importer/importer.go

+		decBuf := bytes.NewBuffer(encSample)
+		sample := tsdb.MetricSample{}
+		if err := gob.NewDecoder(decBuf).Decode(&sample); err != nil {
+			level.Error(logger).Log("msg", "failed to decode current entry returned by file scanner", "err", err)


Let's avoid things like this, either log error or return, let's not handle errors twice (:

bwplotka · 2020-07-14T09:07:32Z

tsdb/importer/importer.go

+
+// sampleStreamer returns a function that can be used to parse an OpenMetrics compliant
+// byte array, and return each token in the user-provided byte buffer.
+func sampleStreamer(buf *bytes.Buffer) func([]byte, bool) (int, []byte, error) {


I am not fan of this function. Why we encode twice? We parse, then encode in buffer with enc := gob.NewEncoder(buf). Why we do that?

bwplotka · 2020-07-14T12:42:51Z

tsdb/importer/importer.go

+	}
+	defer db.Close()
+
+	blocks, err := db.Blocks()


I don't think we can rely on being in Prometheus DB path. Think about cases when you run Promethues on remote machines

@dipack95

Based on #5887 Thanks for your work so far @dipack95, it helped a lot! Changes on top of @dipack95: * Addressed all reviews components * Use subcommands for different formats * Simplifed block creation, no need to be such complex for first iteration. * Simpliefied and separate concerns. No need to have access to DB. Block * writting is separated as well for ease of benchmarking and test. This will be also needed by @JessicaGreben * Added import support for different formats. * Removed all tests - those had to be pulled over and adjusted ): Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

@dipack95

Based on #5887 Thanks for your work so far @dipack95, it helped a lot! Changes on top of @dipack95: * Addressed all reviews components * Use subcommands for different formats * Simplifed block creation, no need to be such complex for first iteration. * Simpliefied and separate concerns. No need to have access to DB. Block * writting is separated as well for ease of benchmarking and test. This will be also needed by @JessicaGreben * Added import support for different formats. * Removed all tests - those had to be pulled over and adjusted ): Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

@dipack95

Based on #5887 Thanks for your work so far @dipack95, it helped a lot! Changes on top of @dipack95: * Addressed all reviews components * Use subcommands for different formats * Simplifed block creation, no need to be such complex for first iteration. * Simpliefied and separate concerns. No need to have access to DB. Block * writting is separated as well for ease of benchmarking and test. This will be also needed by @JessicaGreben * Added import support for different formats. * Removed all tests - those had to be pulled over and adjusted ): Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

bwplotka · 2020-07-16T09:20:11Z

Hey, since you seems busy @dipack95 and we all would love to have that in, I created separate PR with your work, plus CSV format. I had to simplify a lot allow different format. Also applied all review comments from here: #7586

Please let me know if you want to still help, I would love to see us together moving this forward. Feel free to propose changes to tsdb-import branch! (:

@dipack95

Based on #5887 Thanks for your work so far @dipack95, it helped a lot! Changes on top of @dipack95: * Addressed all reviews components * Use subcommands for different formats * Simplifed block creation, no need to be such complex for first iteration. * Simpliefied and separate concerns. No need to have access to DB. Block * writting is separated as well for ease of benchmarking and test. This will be also needed by @JessicaGreben * Added import support for different formats. * Removed all tests - those had to be pulled over and adjusted ): Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

@dipack95

Based on #5887 Thanks for your work so far @dipack95, it helped a lot! Changes on top of @dipack95: * Addressed all reviews components * Use subcommands for different formats * Simplifed block creation, no need to be such complex for first iteration. * Simpliefied and separate concerns. No need to have access to DB. Block * writting is separated as well for ease of benchmarking and test. This will be also needed by @JessicaGreben * Added import support for different formats. * Removed all tests - those had to be pulled over and adjusted ): Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

dipack95 · 2020-07-16T15:08:53Z

Thank you @bwplotka! I was planning to work on this this weekend, but it's great that you're using my work as a base.

beorn7 · 2020-08-17T14:33:03Z

This has been superseded (twice) by #7586 and #7675 . Closing.

Davidrjx · 2021-09-18T06:45:39Z

now, which is the stable import tool? tsdb or promtool
however tsdb cannot found in latest branch, and both have no import from CSV

beorn7 · 2021-09-20T16:58:36Z

The tsdb features have been fully moved into the tsdb subcommand of promtool.

CSV import never really worked. It's actually tricky to define a reasonable structure of a CSV that would nicely fit the Prometheus data model. Most relevant open issue is probably #7119.

Davidrjx · 2021-09-22T09:23:00Z

thanks, very great, currently switch to promtool already

obitech mentioned this pull request Aug 21, 2019

Add tests for tsdb CLI commands #5927

Closed

krasi-georgiev self-assigned this Aug 22, 2019

krasi-georgiev reviewed Sep 19, 2019

View reviewed changes

krasi-georgiev reviewed Sep 23, 2019

View reviewed changes

tsdb/importer/import_old.go Outdated Show resolved Hide resolved

tsdb/importer/import.go Outdated Show resolved Hide resolved

krasi-georgiev reviewed Sep 30, 2019

View reviewed changes

This was referenced Oct 15, 2019

Add mechanism to perform bulk imports #535

Closed

Add tsdb commands as promtool tsdb subcommand #6088

Merged

brian-brazil reviewed Oct 15, 2019

View reviewed changes

dipack95 mentioned this pull request Oct 22, 2019

Compute WAL size and use it during retention size checks #5886

Merged

stale bot removed the stale label Jul 12, 2020

bwplotka reviewed Jul 12, 2020

View reviewed changes

bwplotka changed the title ~~TSDB data import tool~~ TSDB data import tool for OpenMetrics format. Jul 12, 2020

bwplotka reviewed Jul 12, 2020

View reviewed changes

bwplotka reviewed Jul 14, 2020

View reviewed changes

bwplotka mentioned this pull request Jul 16, 2020

Added TSDB import with OpenMetrics and CSV file support. #7586

Closed

5 tasks

beorn7 closed this Aug 17, 2020

piroux mentioned this pull request Oct 6, 2020

Tracking backfill/bulk-import feature progress #8016

Closed

roidelapluie added this to Abandoned in backfilling Oct 18, 2020

grafanabot mentioned this pull request Aug 10, 2021

Blocks storage unable to ingest samples older than 1h after an outage grafana/mimir#116

Closed

TSDB data import tool for OpenMetrics format. #5887

TSDB data import tool for OpenMetrics format. #5887

Conversation

dipack95 commented Aug 13, 2019 • edited Loading

dipack95 commented Aug 13, 2019

krasi-georgiev left a comment

Choose a reason for hiding this comment

codesome commented Sep 19, 2019

dipack95 commented Sep 19, 2019

dipack95 commented Sep 22, 2019

krasi-georgiev commented Sep 23, 2019

krasi-georgiev commented Sep 23, 2019

dipack95 commented Sep 23, 2019

dipack95 commented Sep 23, 2019

krasi-georgiev commented Sep 23, 2019

dipack95 commented Sep 23, 2019

dipack95 commented Sep 27, 2019

dipack95 commented Oct 9, 2019 • edited Loading

krasi-georgiev commented Oct 10, 2019

dipack95 commented Oct 10, 2019 • edited Loading

krasi-georgiev commented Oct 14, 2019 • edited Loading

dipack95 commented Oct 14, 2019

krasi-georgiev commented Oct 15, 2019

dipack95 commented Oct 15, 2019

krasi-georgiev commented Oct 15, 2019

dipack95 commented Oct 15, 2019

brian-brazil left a comment

Choose a reason for hiding this comment

krasi-georgiev commented Oct 22, 2019

bwplotka left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Jul 12, 2020

bwplotka commented Jul 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dipack95 commented Jul 12, 2020

bwplotka commented Jul 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka Jul 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Jul 16, 2020

dipack95 commented Jul 16, 2020

beorn7 commented Aug 17, 2020

Davidrjx commented Sep 18, 2021

beorn7 commented Sep 20, 2021

Davidrjx commented Sep 22, 2021

dipack95 commented Aug 13, 2019 •

edited

Loading

dipack95 commented Oct 9, 2019 •

edited

Loading

dipack95 commented Oct 10, 2019 •

edited

Loading

krasi-georgiev commented Oct 14, 2019 •

edited

Loading

bwplotka left a comment •

edited

Loading

bwplotka Jul 13, 2020 •

edited

Loading

bwplotka commented Jul 12, 2020 •

edited

Loading

bwplotka Jul 14, 2020 •

edited

Loading