Add Block checksum validation and "influx_inspect verify" tool #5582

seiflotfy · 2016-02-08T20:23:03Z

Output is in the form of:

[Broken Block] ShardID: 3    File: /Users/seif/.influxdb/data/stress/default/3/000000001-000000001.tsm   Key: cpu,host=server-23136,location=us-west#!~#value    Block: min=2006-01-02 00:00:10 +0000 UTC max=2006-01-02 00:04:10 +0000 UTC ofs=1220899 siz=82
[Broken Block] ShardID: 3    File: /Users/seif/.influxdb/data/stress/default/3/000000001-000000001.tsm   Key: cpu,host=server-48876,location=us-west#!~#value    Block: min=2006-01-02 00:00:10 +0000 UTC max=2006-01-02 00:04:10 +0000 UTC ofs=3612460 siz=82
Broken Blocks: 2 / 100000

jwilder · 2016-02-08T20:31:47Z

cmd/influx_inspect/verify.go

+	tstore.EngineOptions.Config.Dir = filepath.Join(path, "data")
+	tstore.EngineOptions.Config.WALLoggingEnabled = false
+	tstore.EngineOptions.Config.WALDir = filepath.Join(path, "wal")
+	if err := tstore.Open(); err != nil {


This tool really shouldn't be loading the store and shards. It's meant to be a separate and independent implementation of lower level TSM functions. Calling Open has some side-effects like kicking off compactions, cleanup, etc.. which makes it unsafe to run outside of the server.

Note taken, I will just open the file manually and parse the bytes in based on the TSM structure. Is that what you mean?

Yes. Checksum is the first 4 bytes of each block.

jwilder · 2016-02-08T20:40:24Z

@seiflotfy #5502 was about validating the value we have here: https://github.com/influxdata/influxdb/blob/master/cmd/influx_inspect/tsm.go#L547 since we just print it.

Would you be able to re-work this so that it does not use the tsdb.Store and tsdb.Shard? I think having a separate verify command is a good idea as opposed to tacking on another column into the dumptsm1dev command.

seiflotfy · 2016-02-08T20:50:43Z

I am on it 👍

seiflotfy · 2016-02-08T21:18:54Z

So I updated the code to now "walk" through the data directory getting all the .tsm files. It hurts a bit to do that but I found no other way around it.

The new output looks as follows:

[Broken Block] in File: /Users/seif/.influxdb/data/stress/default/3/000000001-000000001.tsm  Key: cpu,host=server-61547,location=us-west#!~#value   Block: min=2006-01-02 00:00:10 +0000 UTC max=2006-01-02 00:04:10 +0000 UTC ofs=4790017 siz=84
[Broken Block] in File: /Users/seif/.influxdb/data/stress/default/3/000000001-000000001.tsm  Key: cpu,host=server-77729,location=us-west#!~#value   Block: min=2006-01-02 00:00:10 +0000 UTC max=2006-01-02 00:04:00 +0000 UTC ofs=6288168 siz=79
Broken Blocks: 2 / 100003

e-dard · 2016-02-09T15:33:45Z

tsdb/engine/tsm1/reader.go

+func (t *TSMReader) ValidateChecksum() map[string]map[string]bool {
+	checksumMap := make(map[string]map[string]bool)
+	for _, key := range t.Keys() {
+		checksums, _ := t.accessor.getChecksums(key)


I think you need to get a RLock on t.accessor here?

e-dard · 2016-02-09T18:38:16Z

LGTM 👍

seiflotfy · 2016-02-11T20:28:34Z

@jwilder is this good to go then?

jwilder · 2016-02-11T21:06:22Z

tsdb/engine/tsm1/reader.go

@@ -99,6 +100,7 @@ type blockAccessor interface {
 	readBytes(entry *IndexEntry, buf []byte) ([]byte, error)
 	path() string
 	close() error
+	getChecksums(key string) (map[string]bool, error)


Can you rename this to just checksums(...)?

seiflotfy · 2016-02-11T21:27:33Z

Ok, going to purge this and start all over I guess :)

jwilder · 2016-02-11T21:31:38Z

You should be able to just call:

TSMReader.BlockIterator() which will return an iterator that will allow you to walk over every block in a file sequentially. The Read call strips the checksum out currently so the underlying bits would need to be modified to return that instead of stripping it. fileAccessor and mmapAccessor should only need a small change to readBytes

seiflotfy · 2016-02-11T21:37:50Z

that is what I am doing

On Thu, Feb 11, 2016 at 3:32 PM Jason Wilder notifications@github.com
wrote:

You should be able to just call:

TSMReader.BlockIterator() which will return an iterator that will allow
you to walk over every block in a file sequentially. The Read call strips
the checksum out currently so the underlying bits would need to be modified
to return that instead of stripping it. fileAccessor and mmapAccessor
should only need a small change to readBytes

—
Reply to this email directly or view it on GitHub
#5582 (comment).

toddboom · 2016-04-11T22:52:10Z

@seiflotfy Sorry for the delays on this. If you could give this a final rebase, I'll try to get it merged in. A quick glance says it should be pretty quick. Thanks!

seiflotfy · 2016-04-11T23:33:27Z

I pushed by mistake (its late here). Don't merge yet since i need to test it out properly

toddboom · 2016-04-11T23:39:58Z

@seiflotfy no problem! i'll check back in later.

seiflotfy · 2016-04-11T23:51:34Z

it works but its SLOW. Do not merge, I will try to speed it up

seiflotfy · 2016-04-16T00:23:44Z

@toddboom @benbjohnson I added a cache to speed things up. Please have a look at the changes to writer.go and tell me if it makes sense?

jwilder · 2016-04-19T05:59:02Z

tsdb/engine/tsm1/writer.go

+	mu               sync.RWMutex
+	size             uint32
+	blocks           map[string]*indexEntries
+	cachedSortedKeys []string


This will double the size of the index in memory when writing TSM files during compactions. Can you revert this?

sure

On Tue, Apr 19, 2016 at 7:59 AM Jason Wilder notifications@github.com
wrote:

In tsdb/engine/tsm1/writer.go
#5582 (comment):

@@ -264,9 +264,10 @@ func NewDirectIndex() TSMIndex {
// directIndex is a simple in-memory index implementation for a TSM file. The full index
// must fit in memory.
type directIndex struct {

mu sync.RWMutex

size uint32

blocks map[string]*indexEntries

mu sync.RWMutex

size uint32

blocks map[string]*indexEntries

cachedSortedKeys []string

This will double the size of the index in memory when writing TSM files
during compactions. Can you revert this?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/influxdata/influxdb/pull/5582/files/ff72a3debddbae9be9a92d59f66cc3bc6394e66c#r60176562

I think there needs to be a way to not have to resort the keys when calling Keys() every time. what do you think?

seiflotfy · 2016-04-19T06:16:13Z

@jwilder done

seiflotfy · 2016-04-19T06:32:43Z

@toddboom @jwilder its much faster now... thanks. Don't know what the appveyor test is though

jwilder · 2016-04-19T20:18:13Z

👍 Looks like another rebase is needed.

seiflotfy · 2016-04-19T20:32:30Z

done

On Tue, Apr 19, 2016 at 10:18 PM Jason Wilder notifications@github.com
wrote:

👍 Looks like another rebase is needed.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5582 (comment)

Fixes #5502

jwilder · 2016-04-19T20:52:38Z

Thanks @seiflotfy!

jwilder reviewed Feb 8, 2016
View reviewed changes

e-dard reviewed Feb 9, 2016
View reviewed changes

jwilder reviewed Feb 11, 2016
View reviewed changes

jwilder reviewed Apr 19, 2016
View reviewed changes

seiflotfy added 2 commits April 19, 2016 22:33

Add Block checksum validation and "influx_inspect verify" tool

c6e3c87

Fixes #5502

Use TSMReaderOptions MMAP in verify cmd

467bc6b

jwilder merged commit 7ca9992 into influxdata:master Apr 19, 2016

jwilder added this to the 0.13.0 milestone Apr 19, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Block checksum validation and "influx_inspect verify" tool #5582

Add Block checksum validation and "influx_inspect verify" tool #5582

seiflotfy commented Feb 8, 2016

jwilder Feb 8, 2016

seiflotfy Feb 8, 2016

jwilder Feb 8, 2016

jwilder commented Feb 8, 2016

seiflotfy commented Feb 8, 2016

seiflotfy commented Feb 8, 2016

e-dard Feb 9, 2016

e-dard commented Feb 9, 2016

seiflotfy commented Feb 11, 2016

jwilder Feb 11, 2016

seiflotfy commented Feb 11, 2016

jwilder commented Feb 11, 2016

seiflotfy commented Feb 11, 2016

toddboom commented Apr 11, 2016

seiflotfy commented Apr 11, 2016

toddboom commented Apr 11, 2016

seiflotfy commented Apr 11, 2016

seiflotfy commented Apr 16, 2016

jwilder Apr 19, 2016

seiflotfy Apr 19, 2016

seiflotfy Apr 19, 2016

seiflotfy commented Apr 19, 2016

seiflotfy commented Apr 19, 2016

jwilder commented Apr 19, 2016

seiflotfy commented Apr 19, 2016

jwilder commented Apr 19, 2016

Add Block checksum validation and "influx_inspect verify" tool #5582

Add Block checksum validation and "influx_inspect verify" tool #5582

Conversation

seiflotfy commented Feb 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwilder commented Feb 8, 2016

seiflotfy commented Feb 8, 2016

seiflotfy commented Feb 8, 2016

Choose a reason for hiding this comment

e-dard commented Feb 9, 2016

seiflotfy commented Feb 11, 2016

Choose a reason for hiding this comment

seiflotfy commented Feb 11, 2016

jwilder commented Feb 11, 2016

seiflotfy commented Feb 11, 2016

toddboom commented Apr 11, 2016

seiflotfy commented Apr 11, 2016

toddboom commented Apr 11, 2016

seiflotfy commented Apr 11, 2016

seiflotfy commented Apr 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seiflotfy commented Apr 19, 2016

seiflotfy commented Apr 19, 2016

jwilder commented Apr 19, 2016

seiflotfy commented Apr 19, 2016

jwilder commented Apr 19, 2016