Always check if shard is local when accessing its stats #2449
Conversation
|
Fixes issue #2452 |
| @@ -3355,7 +3359,7 @@ func (s *Server) DiagnosticsAsRows() []*influxql.Row { | |||
| nodes = append(nodes, strconv.FormatUint(n, 10)) | |||
| } | |||
| var path string | |||
| if sh.store != nil { | |||
There was a problem hiding this comment.
Not strictly necessary, but standardizes on using HasDataNodeID to check if a shard is local.
otoolep
changed the title
| @@ -234,7 +234,9 @@ func (s *Shard) close() error { | |||
| if s.store != nil { | |||
| _ = s.store.Close() | |||
| } | |||
| s.stats.Inc("close") | |||
There was a problem hiding this comment.
Also caused panic. We need to continue to rework the shard type in our code. @corylanou started some of this work.
There was a problem hiding this comment.
I'm not sure how stats can ever be nil. We instantiate it for both creating or opening a shard. It feels like somewhere else in our code we are creating a shard and not instantiating it properly?
There was a problem hiding this comment.
Yes, this is why we need to continue to rework this code. Here:
https://github.com/influxdb/influxdb/blob/master/server.go#L318
we add shards, but never open them. But in this function nothing prevents close being called on a shard object that was never opened.
There was a problem hiding this comment.
https://github.com/influxdb/influxdb/blob/master/server.go#L275 is where shards can be closed that were never opened.
There was a problem hiding this comment.
The other problem it would appear is something is calling code when the server is shut down, which doesn't seem right either.
|
+1 |
|
Thanks @corylanou -- I have opened ticket #2455 to work on refactoring. |
Fixes issue #2452
Always check if shard is local when accessing its stats
No description provided.