Open db in Read only mode #588

krasi-georgiev · 2019-04-24T14:58:26Z

a better approach than #541 (Thanks to Bartek's review.)

fixes #516

TODO:

open an issue for the TODO comments refactoring.

Signed-off-by: Krasi Georgiev [email protected]

Signed-off-by: Krasi Georgiev <[email protected]>

db.go

Signed-off-by: Krasi Georgiev <[email protected]>

db.go

bwplotka · 2019-04-24T17:42:51Z

Also we need to think about our use case. In current design we have only snaphot reader. There is no one syncing blocks if some writer changes it

bwplotka · 2019-04-24T18:04:14Z

Sorry for another PR: #589

Signed-off-by: Krasi Georgiev <[email protected]>

db.go

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka

So there was some version that I liked: no reference to db and purerly read all by DBReadOnly on Block and Querier, However you changed it back to the version with db. Why? IMO it's wrong as you read only struct has access to wrtie methods in db so it's easy to "write" something by accident.

I closed my PR as I like your idea more to sync in Querier and Block on demand, but with that thing is not longer View it's rather DBReadOnly name more as you had before. Can we revert the naming?

Also there are lots of side fixes, can those go in separate PR for clarity?

db.go

Signed-off-by: Krasi Georgiev <[email protected]>

…tive Signed-off-by: Krasi Georgiev <[email protected]>

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev · 2019-05-30T12:23:28Z

@bwplotka , @brancz

I did a bit more work on this and I think it is ready for a final review.

codesome

Are we planning to guarantee "read only" here? Because it is still possible to call Delete() and CleanTombstones() from a Block. If we want to assure read-only at all levels, we may need a BlockReadOnly.

cmd/tsdb/main.go

db.go

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev · 2019-07-02T08:16:53Z

ping @bwplotka

codesome

Few comments, didn't have time to look at the tests and wal.go.

cmd/tsdb/main.go

db.go

head.go

testutil/directory.go

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev · 2019-07-09T08:24:46Z

ping @codesome @bwplotka for a review.

codesome

LGTM. Just 1 comment.

testutil/directory.go

Signed-off-by: Krasi Georgiev <[email protected]>

Signed-off-by: Ganesh Vernekar <[email protected]>

NumSeries in Meta() of Head

gouthamve · 2019-07-17T06:05:25Z

Can you hold off one more day? I plan to review this tomorrow but if I don't do anything tomorrow, feel free to merge, I don't forsee any issues :)

krasi-georgiev · 2019-07-17T09:31:41Z

@gouthamve thanks! waiting for the review.

bwplotka · 2019-07-17T09:39:57Z

Reviewing now (:

bwplotka

Nice one! Looking good, I think it's super close to be done, but I think we can still improve it bit:

for defers when doing exitWithError
I am quite confused with thread safety for DBReadOnly. I think we can improve that part. Let me know if I understood the comment corectly.

Plus some other nits.

Let me know what's your thoughts @krasi-georgiev

block.go

cmd/tsdb/main.go

db.go

head.go

Signed-off-by: Krasi Georgiev <[email protected]>

…b into read-only-alternative Signed-off-by: Krasi Georgiev <[email protected]>

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka

Thanks!

Further thoughts.

CHANGELOG.md

cmd/tsdb/main.go

bwplotka · 2019-07-18T11:51:08Z

cmd/tsdb/main.go

 		}
-		printBlocks(db.Blocks(), listCmdHumanReadable)
+		defer func() {
+			err = db.Close()


This will mask actual error, so we need some kind of multierror here or something like Thanos have: https://github.com/improbable-eng/thanos/blob/master/pkg/runutil/runutil.go#L113

I would really either import this runutil package from Thanos OR move to TSDB (someday Prometheus), so Thanos can import TSDB I guess (:

Also we are extending this package with: thanos-io/thanos#1302

db.go

bwplotka · 2019-07-18T11:56:58Z

db.go

+		db.mtx.Unlock()
+	}
+
+	dbWritable := &DB{


// TODO(Github-issue): Refactor DB to have writer implementation using pure reader implementation not the opposite.

Potentially with github issue for that.

db.go

head.go

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka

LGTM 👍 Thanks (:

Cannot see this Meta comment we were talking about - for special head implementation of BlockReader behaviour. Otherwise LGTM (:

head.go

bwplotka · 2019-07-22T16:24:46Z

Can see comment https://github.com/prometheus/tsdb/pull/588/files#diff-0ced3454d447442a96a77b33ebe87300R1131 👍

Thanks

Krasi Georgiev added 2 commits April 24, 2019 17:22

Added db read only open mode and use it for the tsdb cli.

0997a15

Signed-off-by: Krasi Georgiev <[email protected]>

add the DBReadOnly struct

cd08753

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev commented Apr 24, 2019

View reviewed changes

db.go Outdated Show resolved Hide resolved

brian-brazil reviewed Apr 24, 2019

View reviewed changes

db.go Outdated Show resolved Hide resolved

fix wal closing

8128c53

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka reviewed Apr 24, 2019

View reviewed changes

db.go Outdated Show resolved Hide resolved

bwplotka reviewed Apr 24, 2019

View reviewed changes

db.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

bwplotka mentioned this pull request Apr 24, 2019

Proposal for read only TSDB for CLI (debug/repair) purposes. #589

Closed

refactored to update blocks on domand

83d3683

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka reviewed Apr 25, 2019

View reviewed changes

db.go Outdated Show resolved Hide resolved

rename to DBView

a704541

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka suggested changes Apr 25, 2019

View reviewed changes

db.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

revert renaming

a16cd17

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev force-pushed the read-only-alternative branch from 3fadbf9 to 6fded52 Compare April 25, 2019 08:36

remove metrics

ce25efc

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev force-pushed the read-only-alternative branch from 6fded52 to ce25efc Compare April 25, 2019 09:58

Krasi Georgiev added 3 commits May 6, 2019 12:12

detach db from openBlocks()

825d6c0

Signed-off-by: Krasi Georgiev <[email protected]>

Merge remote-tracking branch 'upstream/master' into read-only-alterna…

ca72a4c

…tive Signed-off-by: Krasi Georgiev <[email protected]>

Merge remote-tracking branch 'upstream/master' into read-only-alterna…

93af048

…tive Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev force-pushed the read-only-alternative branch from 50ef318 to cb67884 Compare May 30, 2019 10:07

refactor and add tests

c2d809b

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev force-pushed the read-only-alternative branch from cb67884 to c2d809b Compare May 30, 2019 10:14

added db.Close to close all open blocks for windows

1b68f0d

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev mentioned this pull request May 30, 2019

ReadOnly option #541

Closed

krasi-georgiev changed the title ~~Read only alternative~~ Open db in Read only mode May 30, 2019

codesome reviewed May 31, 2019

View reviewed changes

cmd/tsdb/main.go Outdated Show resolved Hide resolved

cmd/tsdb/main.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

add a read only interface for a block.

4778d73

Signed-off-by: Krasi Georgiev <[email protected]>

nit

106d0e9

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev force-pushed the read-only-alternative branch from c798dbf to 106d0e9 Compare June 25, 2019 15:16

codesome reviewed Jul 2, 2019

View reviewed changes

cmd/tsdb/main.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

db.go Outdated Show resolved Hide resolved

db.go Show resolved Hide resolved

head.go Show resolved Hide resolved

testutil/directory.go Outdated Show resolved Hide resolved

nits

2b4ddbc

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev force-pushed the read-only-alternative branch from 1405b56 to 2b4ddbc Compare July 2, 2019 12:32

Merge branch 'master' into read-only-alternative

32ae42e

codesome reviewed Jul 9, 2019

View reviewed changes

testutil/directory.go Outdated Show resolved Hide resolved

refactor error handling for DirHash and DirSize

fb75682

Signed-off-by: Krasi Georgiev <[email protected]>

krasi-georgiev mentioned this pull request Jul 15, 2019

NumSeries() method for BlockReader interface #656

Closed

codesome and others added 2 commits July 16, 2019 15:09

NumSeries in Meta() of Head

1cf409f

Signed-off-by: Ganesh Vernekar <[email protected]>

Merge pull request #5 from codesome/num-series-read-only

0a645ab

NumSeries in Meta() of Head

bwplotka suggested changes Jul 17, 2019

View reviewed changes

krasi-georgiev added 4 commits July 17, 2019 18:26

nits

af10877

Signed-off-by: Krasi Georgiev <[email protected]>

Merge branch 'read-only-alternative' of github.com:krasi-georgiev/tsd…

b8c4781

…b into read-only-alternative Signed-off-by: Krasi Georgiev <[email protected]>

use channel for the db closing and remove mutex

df4a374

Signed-off-by: Krasi Georgiev <[email protected]>

add ErrClosed and add a test for it.

b3f7774

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka reviewed Jul 18, 2019

View reviewed changes

krasi-georgiev added 3 commits July 19, 2019 13:22

handle mutli errors.

1a97569

Signed-off-by: Krasi Georgiev <[email protected]>

nits

03b33b1

Signed-off-by: Krasi Georgiev <[email protected]>

head meta comment

beeaefa

Signed-off-by: Krasi Georgiev <[email protected]>

bwplotka approved these changes Jul 22, 2019

View reviewed changes

head.go Show resolved Hide resolved

krasi-georgiev merged commit 6f9bbc7 into prometheus-junkyard:master Jul 23, 2019

krasi-georgiev deleted the read-only-alternative branch July 23, 2019 08:04

codesome mentioned this pull request Jul 23, 2019

Vendor tsdb v0.10.0 prometheus/prometheus#5793

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Open db in Read only mode #588

Open db in Read only mode #588

krasi-georgiev commented Apr 24, 2019 •

edited

Loading

bwplotka commented Apr 24, 2019

bwplotka commented Apr 24, 2019

bwplotka left a comment

krasi-georgiev commented May 30, 2019

codesome left a comment

krasi-georgiev commented Jul 2, 2019

codesome left a comment

krasi-georgiev commented Jul 9, 2019

codesome left a comment

gouthamve commented Jul 17, 2019

krasi-georgiev commented Jul 17, 2019

bwplotka commented Jul 17, 2019

bwplotka left a comment

bwplotka left a comment •

edited

Loading

bwplotka Jul 18, 2019

krasi-georgiev Jul 22, 2019

bwplotka Jul 18, 2019

bwplotka left a comment

bwplotka commented Jul 22, 2019

Open db in Read only mode #588

Open db in Read only mode #588

Conversation

krasi-georgiev commented Apr 24, 2019 • edited Loading

bwplotka commented Apr 24, 2019

bwplotka commented Apr 24, 2019

bwplotka left a comment

Choose a reason for hiding this comment

krasi-georgiev commented May 30, 2019

codesome left a comment

Choose a reason for hiding this comment

krasi-georgiev commented Jul 2, 2019

codesome left a comment

Choose a reason for hiding this comment

krasi-georgiev commented Jul 9, 2019

codesome left a comment

Choose a reason for hiding this comment

gouthamve commented Jul 17, 2019

krasi-georgiev commented Jul 17, 2019

bwplotka commented Jul 17, 2019

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka left a comment • edited Loading

Choose a reason for hiding this comment

bwplotka Jul 18, 2019

Choose a reason for hiding this comment

krasi-georgiev Jul 22, 2019

Choose a reason for hiding this comment

bwplotka Jul 18, 2019

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka commented Jul 22, 2019

krasi-georgiev commented Apr 24, 2019 •

edited

Loading

bwplotka left a comment •

edited

Loading