X Tutup
The Wayback Machine - https://web.archive.org/web/20201223002217/https://github.com/prometheus/prometheus/pull/6088
Skip to content

/ prometheus

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

Jump to bottom

Add tsdb commands as promtool tsdb subcommand #6088

Merged
merged 1 commit into from Jul 23, 2020
Merged

Add tsdb commands as promtool tsdb subcommand #6088

merged 1 commit into from Jul 23, 2020

Conversation

@yeya24
Copy link
Contributor

@yeya24 yeya24 commented Oct 2, 2019 •
edited

Signed-off-by: yeya24 yb532204897@gmail.com

fixes #6087
@yeya24 yeya24 force-pushed the yeya24:extend-promtool branch Oct 2, 2019
@krasi-georgiev krasi-georgiev self-assigned this Oct 2, 2019
@simonpasquier
simonpasquier reviewed Oct 3, 2019
View changes
cmd/promtool/tsdb.go Outdated
@@ -1,4 +1,4 @@
// Copyright 2017 The Prometheus Authors
// Copyright 2019 The Prometheus Authors

This comment has been minimized.

Sign in to view
@simonpasquier

simonpasquier Oct 3, 2019
Member

no need to change the copyright date.
cmd/promtool/main.go Show resolved Hide resolved
cmd/promtool/main.go Show resolved Hide resolved
cmd/promtool/main.go Show resolved Hide resolved
@krasi-georgiev
krasi-georgiev reviewed Oct 3, 2019
View changes
cmd/promtool/main.go Outdated
@@ -103,6 +103,29 @@ func main() {
"The unit test file.",
).Required().ExistingFiles()

defaultDBPath := filepath.Join("benchout", "storage")

This comment has been minimized.

Sign in to view
@krasi-georgiev

krasi-georgiev Oct 3, 2019
Contributor

now that tsdb has been moved recently all default need to be revised. Could you test these and update accordingly.

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Oct 3, 2019
Author Contributor

Is there any other path I need to update besides the tsdb path?

This comment has been minimized.

Sign in to view
@krasi-georgiev

krasi-georgiev Oct 3, 2019
Contributor

just run the command locally and you will find out if any returns an error.

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Oct 3, 2019
Author Contributor

I have already tested in my local env and it works well. Can you also check it?
@yeya24
yeya24 reviewed Oct 3, 2019
View changes
cmd/promtool/main.go
tsdbBenchWriteCmd := tsdbBenchCmd.Command("write", "Run a write performance benchmark.")
benchWriteOutPath := tsdbBenchWriteCmd.Flag("out", "Set the output path.").Default("benchout").String()
benchWriteNumMetrics := tsdbBenchWriteCmd.Flag("metrics", "Number of metrics to read.").Default("10000").Int()
benchSamplesFile := tsdbBenchWriteCmd.Arg("file", "Input file with samples data, default is ("+filepath.Join("..", "..", "tsdb", "testdata", "20kseries.json")+").").Default(filepath.Join("..", "..", "tsdb", "testdata", "20kseries.json")).String()

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Oct 3, 2019
Author Contributor

I am not sure about this benchmark series file. Should we copy it in the same folder with promtool?

This comment has been minimized.

Sign in to view
@krasi-georgiev

krasi-georgiev Oct 3, 2019
Contributor

it should already be in a subfolder
@yeya24 yeya24 force-pushed the yeya24:extend-promtool branch Oct 3, 2019
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Oct 3, 2019

All green now. @krasi-georgiev
@yeya24 yeya24 force-pushed the yeya24:extend-promtool branch Oct 4, 2019
@simonpasquier
simonpasquier reviewed Oct 7, 2019
View changes
Makefile Outdated
TSDB_CLI_DIR="$(TSDB_PROJECT_DIR)/cmd/tsdb"
TSDB_BIN = "$(TSDB_CLI_DIR)/tsdb"
TSDB_CLI_DIR = "./cmd/promtool"
TSDB_BIN = "$(TSDB_CLI_DIR)/promtool"

This comment has been minimized.

Sign in to view
@simonpasquier

simonpasquier Oct 7, 2019
Member

Why not use ./promtool instead of rebuilding the binary?

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Oct 7, 2019
Author Contributor

Done
@yeya24 yeya24 force-pushed the yeya24:extend-promtool branch Oct 7, 2019
@krasi-georgiev
krasi-georgiev reviewed Oct 9, 2019
View changes
Makefile Outdated
@@ -15,8 +15,8 @@
DOCKER_ARCHS ?= amd64 armv7 arm64

TSDB_PROJECT_DIR = "./tsdb"
TSDB_CLI_DIR="$(TSDB_PROJECT_DIR)/cmd/tsdb"
TSDB_BIN = "$(TSDB_CLI_DIR)/tsdb"
TSDB_CLI_DIR = "./cmd/promtool"

This comment has been minimized.

Sign in to view
@krasi-georgiev

krasi-georgiev Oct 9, 2019
Contributor

should rename the var

Suggested change
TSDB_CLI_DIR = "./cmd/promtool"
PROMTOOL_DIR = "./cmd/promtool"
Makefile Outdated
@@ -15,11 +15,10 @@
DOCKER_ARCHS ?= amd64 armv7 arm64

TSDB_PROJECT_DIR = "./tsdb"
TSDB_CLI_DIR="$(TSDB_PROJECT_DIR)/cmd/tsdb"
TSDB_BIN = "$(TSDB_CLI_DIR)/tsdb"
TSDB_BIN = "./promtool"

This comment has been minimized.

Sign in to view
@krasi-georgiev

krasi-georgiev Oct 9, 2019
Contributor

Suggested change
TSDB_BIN = "./promtool"
PROMTOOL = "./promtool"

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Oct 9, 2019
Author Contributor

Done
cmd/promtool/tsdb.go Outdated Show resolved Hide resolved
cmd/promtool/tsdb.go Outdated
@@ -617,8 +571,20 @@ func analyzeBlock(b tsdb.BlockReader, limit int) error {
return nil
}

func dumpSamples(db *tsdb.DBReadOnly, mint, maxt int64) (err error) {
func DumpSamples(path string, mint, maxt int64) error {

This comment has been minimized.

Sign in to view
@krasi-georgiev

krasi-georgiev Oct 9, 2019
Contributor

Why do you think it needs this extra func?

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Oct 9, 2019
Author Contributor

Done
@krasi-georgiev
krasi-georgiev reviewed Oct 14, 2019
View changes
Copy link
Contributor

@krasi-georgiev krasi-georgiev left a comment

LGTM after that small change
Makefile Outdated Show resolved Hide resolved
@krasi-georgiev
Copy link
Contributor

@krasi-georgiev krasi-georgiev commented Oct 15, 2019

please resolve the makefile conflict the will merge
@yeya24 yeya24 force-pushed the yeya24:extend-promtool branch to ff7c0e1 Oct 15, 2019
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Oct 15, 2019

please resolve the makefile conflict the will merge

Updated, PTAL
@krasi-georgiev
Copy link
Contributor

@krasi-georgiev krasi-georgiev commented Oct 15, 2019

yep, I think it is all good now.

#5887
Unfortunately there is another PR which started before this one and will also be ready for merging very soon(adding an import command), which is open against the tsdb tools as well and will cause a conflict. Would you mind adding the changes from that PR after it is merged here as well?
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Oct 15, 2019

yep, I think it is all good now.

#5887
Unfortunately there is another PR which started before this one and will also be ready for merging very soon(adding an import command), which is open against the tsdb tools as well and will cause a conflict. Would you mind adding the changes from that PR after it is merged here as well?

No. Please merge that PR first and then I will resolve the conflicts here.
@simonpasquier simonpasquier mentioned this pull request Oct 17, 2019
Closed
@simonpasquier
Copy link
Member

@simonpasquier simonpasquier commented Oct 23, 2019

Since a few PRs are piling up to fix issues with the current master, I've updated this pull request to resolve the conflict. I've also simplified a bit the changes to Makefile.
This was referenced Oct 23, 2019
Closed
Closed
@simonpasquier simonpasquier mentioned this pull request Nov 19, 2019
Merged
@stale stale bot added the stale label Feb 19, 2020
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Feb 25, 2020

Still valid
@krasi-georgiev
Copy link
Contributor

@krasi-georgiev krasi-georgiev commented Feb 25, 2020

yeah that other import cmd is taking way longer than I expected. Just too many variables to consider there.
@stale stale bot removed the stale label Feb 25, 2020
@krasi-georgiev krasi-georgiev removed their assignment Mar 6, 2020
@stale stale bot added the stale label May 5, 2020
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Jul 16, 2020

Not stale.
@stale stale bot removed the stale label Jul 16, 2020
@bwplotka
Copy link
Member

@bwplotka bwplotka commented Jul 16, 2020

@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Jul 16, 2020 •
edited

Will we merge #5887 recently? If so I can wait until that merged and fix conflicts here.
@roidelapluie
Copy link
Member

@roidelapluie roidelapluie commented Jul 16, 2020

I would merge this one first to make sure that the new PR is forced to use promtool.
@yeya24
add tsdb cmds into promtool
Loading status checks…
41d61b6 
Signed-off-by: yeya24 <yb532204897@gmail.com>

update tsdb cli in makefile and promu

Signed-off-by: yeya24 <yb532204897@gmail.com>

remove building tsdb bin

Signed-off-by: yeya24 <yb532204897@gmail.com>

remove useless func

Signed-off-by: yeya24 <yb532204897@gmail.com>

refactor analyzeBlock

Signed-off-by: yeya24 <yb532204897@gmail.com>

Fix Makefile

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@yeya24 yeya24 force-pushed the yeya24:extend-promtool branch from ecbcf6b to 41d61b6 Jul 17, 2020
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Jul 17, 2020

I rebased master and tested this pr. PTAL @bwplotka @roidelapluie
@yeya24
yeya24 reviewed Jul 17, 2020
View changes
cmd/promtool/tsdb.go
@@ -184,24 +91,24 @@ func (b *writeBenchmark) run() error {
if err != nil {
return err
}
st.DisableCompactions()

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Jul 17, 2020
Author Contributor

I am not sure about this. When I tested tsdb bench write, I saw one compaction failure in the stopStorage stage. I think this is caused by https://github.com/prometheus/prometheus/pull/6088/files#diff-587d0d6cc9fcdcf5edcc7dbe2c79cf96R134-R137, we close the tsdb here so it causes a race for auto compaction.

So I disable the auto compaction to prevent the race, does it make sense?

>> running benchmark, writing result to ./benchout
level=info ts=2020-07-17T04:04:42.711295339Z caller=head.go:641 msg="Replaying on-disk memory mappable chunks if any"
level=info ts=2020-07-17T04:04:42.71132615Z caller=head.go:655 msg="On-disk memory mappable chunks replay completed" duration=1.697µs
level=info ts=2020-07-17T04:04:42.7113348Z caller=head.go:661 msg="Replaying WAL, this may take a while"
level=info ts=2020-07-17T04:04:42.711515635Z caller=head.go:713 msg="WAL segment loaded" segment=0 maxSegment=0
level=info ts=2020-07-17T04:04:42.71152925Z caller=head.go:716 msg="WAL replay completed" checkpoint_replay_duration=15.97µs wal_replay_duration=171.071µs total_replay_duration=204.467µs
>> start stage=readData
>> completed stage=readData duration=9.010239ms
>> start stage=ingestScrapes
level=info ts=2020-07-17T04:04:42.911077592Z caller=compact.go:495 msg="write block" mint=30000 maxt=7200000 ulid=01EDDF46P7MCDN4Q1EJ5X3ZWPG duration=87.424018ms
level=info ts=2020-07-17T04:04:42.916771849Z caller=head.go:804 msg="Head GC completed" duration=1.796868ms
level=info ts=2020-07-17T04:04:42.980048205Z caller=compact.go:495 msg="write block" mint=7200000 maxt=14400000 ulid=01EDDF46SC2ZG9GQQYYKWZ2VQY duration=55.081893ms
level=info ts=2020-07-17T04:04:42.985088429Z caller=head.go:804 msg="Head GC completed" duration=1.785562ms
level=info ts=2020-07-17T04:04:43.081263835Z caller=compact.go:495 msg="write block" mint=14400000 maxt=21600000 ulid=01EDDF46VFGNKW589YA66EYBTP duration=89.558609ms
level=info ts=2020-07-17T04:04:43.105916664Z caller=head.go:804 msg="Head GC completed" duration=8.574277ms
level=info ts=2020-07-17T04:04:43.192507556Z caller=compact.go:495 msg="write block" mint=21600000 maxt=28800000 ulid=01EDDF46ZJQ8CH6ZW4227QWP3X duration=70.337554ms
level=info ts=2020-07-17T04:04:43.197806232Z caller=head.go:804 msg="Head GC completed" duration=1.697093ms
level=info ts=2020-07-17T04:04:43.201031161Z caller=checkpoint.go:96 msg="Creating checkpoint" from_segment=0 to_segment=1 mint=28800000
level=info ts=2020-07-17T04:04:43.2503322Z caller=head.go:884 msg="WAL checkpoint complete" first=0 last=1 duration=49.352097ms
level=info ts=2020-07-17T04:04:43.308328754Z caller=compact.go:495 msg="write block" mint=28800000 maxt=36000000 ulid=01EDDF473MXBJW38VEQFY7VMTC duration=55.554547ms
level=info ts=2020-07-17T04:04:43.313698535Z caller=head.go:804 msg="Head GC completed" duration=1.967459ms
level=info ts=2020-07-17T04:04:43.369380643Z caller=compact.go:495 msg="write block" mint=36000000 maxt=43200000 ulid=01EDDF475S6XFJBPWQ58VQC3FF duration=47.424709ms
level=info ts=2020-07-17T04:04:43.375949209Z caller=head.go:804 msg="Head GC completed" duration=1.867668ms
level=info ts=2020-07-17T04:04:43.380923622Z caller=checkpoint.go:96 msg="Creating checkpoint" from_segment=2 to_segment=3 mint=43200000
level=info ts=2020-07-17T04:04:43.410941297Z caller=head.go:884 msg="WAL checkpoint complete" first=2 last=3 duration=30.100943ms
level=info ts=2020-07-17T04:04:43.46156506Z caller=compact.go:495 msg="write block" mint=43200000 maxt=50400000 ulid=01EDDF478PSTASAHX6PSFVY49S duration=47.192342ms
level=info ts=2020-07-17T04:04:43.467009835Z caller=head.go:804 msg="Head GC completed" duration=1.715044ms
level=info ts=2020-07-17T04:04:43.517852121Z caller=compact.go:495 msg="write block" mint=50400000 maxt=57600000 ulid=01EDDF47AHZAGK0DC74Z0F89TK duration=43.879272ms
level=info ts=2020-07-17T04:04:43.522848234Z caller=head.go:804 msg="Head GC completed" duration=1.889164ms
level=info ts=2020-07-17T04:04:43.526273196Z caller=checkpoint.go:96 msg="Creating checkpoint" from_segment=4 to_segment=5 mint=57600000
level=info ts=2020-07-17T04:04:43.575024041Z caller=head.go:884 msg="WAL checkpoint complete" first=4 last=5 duration=48.821878ms
ingestion completed
>> completed stage=ingestScrapes duration=860.843044ms
 > total samples: 3000000
 > samples/sec: 3.484920271389978e+06
>> start stage=stopStorage
level=error ts=2020-07-17T04:04:43.583874046Z caller=db.go:685 msg="compaction failed" err="persist head block: 2 errors: write compaction: add symbol: context canceled; context canceled"
>> completed stage=stopStorage duration=161.018512ms

This comment has been minimized.

Sign in to view
@bwplotka

bwplotka Jul 23, 2020
Member

Let's have some issue to track this. I think this is overall fine for this piece of work, but we should investigate anyway 🤔
@bwplotka bwplotka changed the title add tsdb cmds into promtool Add tsdb commands as promtool tsdb subcommand Jul 17, 2020
@bwplotka
bwplotka approved these changes Jul 17, 2020
View changes
Copy link
Member

@bwplotka bwplotka left a comment

LGTM 💪

👍 👍 👍 Thanks!
cmd/promtool/tsdb.go
@@ -184,24 +91,24 @@ func (b *writeBenchmark) run() error {
if err != nil {
return err
}
st.DisableCompactions()

This comment has been minimized.

Sign in to view
@bwplotka

bwplotka Jul 17, 2020
Member

Let's look on the race, if it's happening on prod code as well. Otherwise disable compaction sounds good here as this benchmarks ingestion only (:

This comment has been minimized.

Sign in to view
@yeya24

yeya24 Jul 23, 2020
Author Contributor

There is a race, also mentioned in #7643. But seems disabling compaction is not the correct way to solve this. I will update this pr after this problem is fixed.
@yeya24
Copy link
Contributor Author

@yeya24 yeya24 commented Jul 23, 2020 •
edited

After rebasing master, I still get this error.

ingestion completed
>> completed stage=ingestScrapes duration=974.863976ms
 > total samples: 3000000
 > samples/sec: 3.0773225260118665e+06
>> start stage=stopStorage
level=error ts=2020-07-23T14:04:24.856156231Z caller=db.go:697 msg="compaction failed" err="persist head block: context canceled"
>> completed stage=stopStorage duration=148.546395ms

So disabling compaction is still needed IMO.
@bwplotka
bwplotka approved these changes Jul 23, 2020
View changes
Copy link
Member

@bwplotka bwplotka left a comment

LGTM!
cmd/promtool/tsdb.go
@@ -184,24 +91,24 @@ func (b *writeBenchmark) run() error {
if err != nil {
return err
}
st.DisableCompactions()

This comment has been minimized.

Sign in to view
@bwplotka

bwplotka Jul 23, 2020
Member

Let's have some issue to track this. I think this is overall fine for this piece of work, but we should investigate anyway 🤔
@bwplotka bwplotka merged commit 50c2615 into prometheus:master Jul 23, 2020
6 checks passed
6 checks passed
DCO DCO
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: fuzzit_regression Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: test_windows Your tests passed on CircleCI!
Details
netlify/prometheus-react/deploy-preview Deploy preview canceled.
Details
@yeya24 yeya24 deleted the yeya24:extend-promtool branch Jul 23, 2020
kangwoo added a commit to kangwoo/prometheus that referenced this pull request Jul 31, 2020
@yeya24 @kangwoo
add tsdb cmds into promtool (prometheus#6088)
47a16ad 
Signed-off-by: yeya24 <yb532204897@gmail.com>

update tsdb cli in makefile and promu

Signed-off-by: yeya24 <yb532204897@gmail.com>

remove building tsdb bin

Signed-off-by: yeya24 <yb532204897@gmail.com>

remove useless func

Signed-off-by: yeya24 <yb532204897@gmail.com>

refactor analyzeBlock

Signed-off-by: yeya24 <yb532204897@gmail.com>

Fix Makefile

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
Signed-off-by: kangwoo <kangwoo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonpasquier simonpasquier

@krasi-georgiev krasi-georgiev

@bwplotka bwplotka

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Extend promtool with all commands from the tsdb tool and remove the tsdb tool
5 participants
@yeya24 @krasi-georgiev @simonpasquier @bwplotka @roidelapluie
You can’t perform that action at this time.
X Tutup