Fix panic: interface conversion: tsm1.Value is *tsm1.FloatValue, not *tsm1.StringValue #7533
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e-dard
approved these changes
Oct 28, 2016
| @@ -219,7 +240,10 @@ func (c *Cache) Write(key string, values []Value) error { | |||
| return ErrCacheMemoryExceeded | |||
| } | |||
|
|
|||
| c.write(key, values) | |||
| if err := c.write(key, values); err != nil { | |||
| c.mu.Unlock() | |||
There was a problem hiding this comment.
Should we update c.stats.WriteErr or maybe add a new stat for this unusual condition?
There was a problem hiding this comment.
Done. Just discovered that the existing OK and Err stats are tracked, but not actually recorded.
e-dard
reviewed
Oct 28, 2016
| @@ -261,7 +261,7 @@ func (f *FileStore) Add(files ...TSMFile) { | |||
| for _, file := range files { | |||
| atomic.AddInt64(&f.stats.DiskBytes, int64(file.Size())) | |||
| } | |||
| f.lastFileStats = f.lastFileStats[:0] // Will need to be recalculated on next call to Stats. | |||
| f.lastFileStats = nil | |||
There was a problem hiding this comment.
Heh, I guess we were trying to be too clever for our own good making that change 😄
The file store stats slice is re-used which causes the race below:
WARNING: DATA RACE
Write at 0x00c42007e140 by goroutine 43:
github.com/influxdata/influxdb/tsdb/engine/tsm1.(*FileStore).Stats()
/Users/jason/go/src/github.com/influxdata/influxdb/tsdb/engine/tsm1/file_store.go:511 +0x22e
github.com/influxdata/influxdb/tsdb/engine/tsm1.(*DefaultPlanner).findGenerations()
/Users/jason/go/src/github.com/influxdata/influxdb/tsdb/engine/tsm1/compact.go:461 +0x6f
github.com/influxdata/influxdb/tsdb/engine/tsm1.(*DefaultPlanner).PlanLevel()
Previous read at 0x00c42007e140 by goroutine 40:
github.com/influxdata/influxdb/tsdb/engine/tsm1.(*DefaultPlanner).findGenerations()
/Users/jason/go/src/github.com/influxdata/influxdb/tsdb/engine/tsm1/compact.go:463 +0x13d
github.com/influxdata/influxdb/tsdb/engine/tsm1.(*DefaultPlanner).PlanOptimize()
…*tsm1.StringValue If concurrent writes to the same shard occur, it's possible for different types to be added to the cache for the same series. The way the measurementFields map on the shard is updated is racy in this scenario which would normally prevent this from occurring. When this occurs, the snapshot compaction panics because it can't encode different types in the same series. To prevent this, we have the cache return an error a different type is added to existing values in the cache. Fixes #7498
Adds a new dropped stat as well as fixes OK and error stats not actually get collected and stored.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Required for all non-trivial PRs
This fixes a panic when snapshotting the cache. If concurrent writes to the same shard occur, it's possible for different types to be added to the cache for the same series. The way the measurementFields map on the shard is updated is racy in this scenario which would normally prevent this from occurring. When this occurs, the snapshot compaction panics because it can't encode different types in the same series.
To prevent this, we have the cache return an error a different type is added to existing values in the cache.
This also fixes a race that was introduced in #7480 where the
FileStore.Statsslice is read by the compactor without a lock when theFileStoreis updating it.Fixes #7498
cc @e-dard