Fix panic when reporting shard diagnostics #2430
Conversation
panic when reporting shards. I also changed the TestSingleServerDiags test to reproduce the error Fixes #2421
| @@ -3345,9 +3346,14 @@ func (s *Server) DiagnosticsAsRows() []*influxql.Row { | |||
|
|
|||
| // Shard may not be local to this node. | |||
| if sh.store != nil { | |||
| shardsRow.Columns = append(shardsRow.Columns, "path") | |||
| shardsRow.Values[0] = append(shardsRow.Values[0], sh.store.Path()) | |||
| if !hasPath { | |||
There was a problem hiding this comment.
I don't think this is necessary. If a sh.store is non-nil, then it must have a path.
There was a problem hiding this comment.
@otoolep but you iterate through each shard. The hasPath variable controls if if the path column has been added before or we need to add it as it's the first time it appears. Otherwise the path column get's appended multiple times unnecessarily
|
You can ignore the test failure for now, it's a known issue we need to look into. |
|
OK, thanks @marcosnils -- I'll take a closer look at this diff shortly. |
This code is clearer -- simply append an empty path if the shard is not local. Fixes issue #2430
This code is clearer -- simply append an empty path if the shard is not local. Fixes issue #2430
|
OK, thanks very much @marcosnils -- I see where you are going. In fact this code has caused multiple issues, so I've actually simplified it at #2431. Your code showed me what was wrong, but I'd like to solve it slightly differently. Please take a look at PR 2431. |
This code is clearer -- simply append an empty path if the shard is not local. Fixes issue #2430
Shard diagnostics wasn't been populated correctly and it made influx panic when reporting shards.
I also changed the TestSingleServerDiags test to reproduce the error
Fixes #2421