From 56d98325dac15b7cbdba5cb554ee83cdc316f77c Mon Sep 17 00:00:00 2001 From: Gustav Westling Date: Thu, 29 Dec 2016 20:13:48 +0100 Subject: [PATCH 1/5] Removed ineffective assignments, and added checks for errors that previsouly was not checked --- cmd/influx_inspect/dumptsm/dumptsm.go | 7 ++---- cmd/influx_tsm/tsdb/database.go | 4 ++++ cmd/influxd/restore/restore.go | 1 - coordinator/statement_executor_test.go | 2 +- influxql/functions.go | 2 +- influxql/query_executor_test.go | 4 ++-- node.go | 5 +++++ services/graphite/parser_test.go | 3 +++ services/snapshotter/client.go | 1 - services/subscriber/http.go | 4 ++++ stress/v2/stressql/statement/parser.go | 6 +++--- tsdb/engine/tsm1/bit_reader_test.go | 3 +++ tsdb/engine/tsm1/file_store.go | 4 ++++ tsdb/engine/tsm1/file_store_test.go | 30 ++++++++++++++++++++++++++ tsdb/engine/tsm1/reader_test.go | 3 +++ tsdb/engine/tsm1/tombstone_test.go | 8 +++---- 16 files changed, 69 insertions(+), 18 deletions(-) diff --git a/cmd/influx_inspect/dumptsm/dumptsm.go b/cmd/influx_inspect/dumptsm/dumptsm.go index 424df2a637..c766e314af 100644 --- a/cmd/influx_inspect/dumptsm/dumptsm.go +++ b/cmd/influx_inspect/dumptsm/dumptsm.go @@ -113,12 +113,9 @@ func (cmd *Command) dump() error { pos++ split := strings.Split(string(key), "#!~#") - // We dont' know know if we have fields so use an informative default - var measurement, field string = "UNKNOWN", "UNKNOWN" - // Possible corruption? Try to read as much as we can and point to the problem. - measurement = split[0] - field = split[1] + measurement := split[0] + field := split[1] if cmd.filterKey != "" && !strings.Contains(string(key), cmd.filterKey) { continue diff --git a/cmd/influx_tsm/tsdb/database.go b/cmd/influx_tsm/tsdb/database.go index c276db08a7..f85322e765 100644 --- a/cmd/influx_tsm/tsdb/database.go +++ b/cmd/influx_tsm/tsdb/database.go @@ -172,6 +172,10 @@ func (d *Database) Shards() ([]*ShardInfo, error) { // Process each shard shards, err := rpfd.Readdirnames(-1) + if err != nil { + return nil, err + } + for _, sh := range shards { fmt, sz, err := shardFormat(filepath.Join(d.path, rp, sh)) if err != nil { diff --git a/cmd/influxd/restore/restore.go b/cmd/influxd/restore/restore.go index 9f1bb62812..1504a692aa 100644 --- a/cmd/influxd/restore/restore.go +++ b/cmd/influxd/restore/restore.go @@ -157,7 +157,6 @@ func (cmd *Command) unpackMeta() error { i += int(length) // Size of the node.json bytes - length = int(binary.BigEndian.Uint64(b[i : i+8])) i += 8 nodeBytes := b[i:] diff --git a/coordinator/statement_executor_test.go b/coordinator/statement_executor_test.go index f41cdee559..640caafac5 100644 --- a/coordinator/statement_executor_test.go +++ b/coordinator/statement_executor_test.go @@ -198,7 +198,7 @@ func NewQueryExecutor() *QueryExecutor { var out io.Writer = &e.LogOutput if testing.Verbose() { - out = io.MultiWriter(out, os.Stderr) + io.MultiWriter(out, os.Stderr) } e.QueryExecutor.WithLogger(zap.New( zap.NewTextEncoder(), diff --git a/influxql/functions.go b/influxql/functions.go index f48a400ab2..a05c2d3c8b 100644 --- a/influxql/functions.go +++ b/influxql/functions.go @@ -657,7 +657,6 @@ func (r *FloatHoltWintersReducer) forecast(h int, params []float64) []float64 { lT := params[4] bT := params[5] - sT := 0.0 // seasonals is a ring buffer of past sT values var seasonals []float64 @@ -683,6 +682,7 @@ func (r *FloatHoltWintersReducer) forecast(h int, params []float64) []float64 { stm = seasonals[(t-m+so)%m] stmh = seasonals[(t-m+hm+so)%m] } + var sT float64 yT, lT, bT, sT = r.next( params[0], // alpha params[1], // beta diff --git a/influxql/query_executor_test.go b/influxql/query_executor_test.go index 3cc643f11d..8e90ef8cc1 100644 --- a/influxql/query_executor_test.go +++ b/influxql/query_executor_test.go @@ -143,7 +143,7 @@ func TestQueryExecutor_Abort(t *testing.T) { } func TestQueryExecutor_ShowQueries(t *testing.T) { - q, err := influxql.ParseQuery(`SELECT count(value) FROM cpu`) + _, err := influxql.ParseQuery(`SELECT count(value) FROM cpu`) if err != nil { t.Fatal(err) } @@ -161,7 +161,7 @@ func TestQueryExecutor_ShowQueries(t *testing.T) { }, } - q, err = influxql.ParseQuery(`SHOW QUERIES`) + q, err := influxql.ParseQuery(`SHOW QUERIES`) if err != nil { t.Fatal(err) } diff --git a/node.go b/node.go index 2d8e1b77d7..68709edc30 100644 --- a/node.go +++ b/node.go @@ -95,7 +95,12 @@ func upgradeNodeFile(path string) error { } return err } + err = json.Unmarshal(pb, &peers) + if err != nil { + return err + } + if len(peers) > 1 { return fmt.Errorf("to upgrade a cluster, please contact support at influxdata") } diff --git a/services/graphite/parser_test.go b/services/graphite/parser_test.go index 8e5e52a702..1c1603d7c1 100644 --- a/services/graphite/parser_test.go +++ b/services/graphite/parser_test.go @@ -679,6 +679,9 @@ func TestApplyTemplateField(t *testing.T) { } measurement, _, field, err := p.ApplyTemplate("current.users.logged_in") + if err != nil { + t.Fatalf(err) + } if measurement != "current_users" { t.Errorf("Parser.ApplyTemplate unexpected result. got %s, exp %s", diff --git a/services/snapshotter/client.go b/services/snapshotter/client.go index 4a5d07c75a..ac6c87411b 100644 --- a/services/snapshotter/client.go +++ b/services/snapshotter/client.go @@ -44,7 +44,6 @@ func (c *Client) MetastoreBackup() (*meta.Data, error) { length := int(binary.BigEndian.Uint64(b[i : i+8])) i += 8 metaBytes := b[i : i+length] - i += int(length) // Unpack meta data. var data meta.Data diff --git a/services/subscriber/http.go b/services/subscriber/http.go index a407ea672a..64ca649ada 100644 --- a/services/subscriber/http.go +++ b/services/subscriber/http.go @@ -24,6 +24,10 @@ func NewHTTP(addr string, timeout time.Duration) (*HTTP, error) { func NewHTTPS(addr string, timeout time.Duration, unsafeSsl bool, caCerts string) (*HTTP, error) { tlsConfig, err := createTlsConfig(caCerts) + if err != nil { + return nil, err + } + conf := client.HTTPConfig{ Addr: addr, Timeout: timeout, diff --git a/stress/v2/stressql/statement/parser.go b/stress/v2/stressql/statement/parser.go index 72211b2fc4..8326352986 100644 --- a/stress/v2/stressql/statement/parser.go +++ b/stress/v2/stressql/statement/parser.go @@ -608,13 +608,13 @@ func (p *Parser) ParseFunction() (*statement.Function, error) { fn := &statement.Function{} - tok, lit := p.scanIgnoreWhitespace() + _, lit := p.scanIgnoreWhitespace() fn.Type = lit - tok, lit = p.scanIgnoreWhitespace() + _, lit = p.scanIgnoreWhitespace() fn.Fn = lit - tok, lit = p.scanIgnoreWhitespace() + tok, lit := p.scanIgnoreWhitespace() if tok != LPAREN { return nil, fmt.Errorf("Error parsing Insert template function\n Expected: LPAREN\n Found: %v\n", lit) } diff --git a/tsdb/engine/tsm1/bit_reader_test.go b/tsdb/engine/tsm1/bit_reader_test.go index a3fac0dd70..a540f6d9b3 100644 --- a/tsdb/engine/tsm1/bit_reader_test.go +++ b/tsdb/engine/tsm1/bit_reader_test.go @@ -17,6 +17,9 @@ func TestBitStreamEOF(t *testing.T) { br := tsm1.NewBitReader([]byte("0")) b, err := br.ReadBits(8) + if err != nil { + t.Fatal(err) + } if b != '0' { t.Error("ReadBits(8) didn't return first byte") } diff --git a/tsdb/engine/tsm1/file_store.go b/tsdb/engine/tsm1/file_store.go index 18c41c55d0..a178df5f0b 100644 --- a/tsdb/engine/tsm1/file_store.go +++ b/tsdb/engine/tsm1/file_store.go @@ -845,6 +845,10 @@ func ParseTSMFileName(name string) (int, int, error) { } generation, err := strconv.ParseUint(id[:idx], 10, 32) + if err != nil { + return 0, 0, err + } + sequence, err := strconv.ParseUint(id[idx+1:], 10, 32) return int(generation), int(sequence), err diff --git a/tsdb/engine/tsm1/file_store_test.go b/tsdb/engine/tsm1/file_store_test.go index d6cbb3b5e9..47a7b7ca96 100644 --- a/tsdb/engine/tsm1/file_store_test.go +++ b/tsdb/engine/tsm1/file_store_test.go @@ -153,6 +153,10 @@ func TestFileStore_SeekToAsc_Duplicate(t *testing.T) { c.Next() values, err = c.ReadFloatBlock(&buf) + if err != nil { + t.Fatal(err) + } + exp = nil if got, exp := len(values), len(exp); got != exp { t.Fatalf("value length mismatch: got %v, exp %v", got, exp) @@ -533,6 +537,10 @@ func TestFileStore_SeekToAsc_OverlapMinFloat(t *testing.T) { c.Next() values, err = c.ReadFloatBlock(&buf) + if err != nil { + t.Fatal(err) + } + exp = nil if got, exp := len(values), len(exp); got != exp { t.Fatalf("value length mismatch: got %v, exp %v", got, exp) @@ -608,6 +616,10 @@ func TestFileStore_SeekToAsc_OverlapMinInteger(t *testing.T) { c.Next() values, err = c.ReadIntegerBlock(&buf) + if err != nil { + t.Fatal(err) + } + exp = nil if got, exp := len(values), len(exp); got != exp { t.Fatalf("value length mismatch: got %v, exp %v", got, exp) @@ -683,6 +695,10 @@ func TestFileStore_SeekToAsc_OverlapMinBoolean(t *testing.T) { c.Next() values, err = c.ReadBooleanBlock(&buf) + if err != nil { + t.Fatal(err) + } + exp = nil if got, exp := len(values), len(exp); got != exp { t.Fatalf("value length mismatch: got %v, exp %v", got, exp) @@ -758,6 +774,10 @@ func TestFileStore_SeekToAsc_OverlapMinString(t *testing.T) { c.Next() values, err = c.ReadStringBlock(&buf) + if err != nil { + t.Fatal(err) + } + exp = nil if got, exp := len(values), len(exp); got != exp { t.Fatalf("value length mismatch: got %v, exp %v", got, exp) @@ -2070,11 +2090,18 @@ func TestFileStore_Replace(t *testing.T) { cur.Next() buf := make([]tsm1.FloatValue, 10) values, err := cur.ReadFloatBlock(&buf) + if err != nil { + t.Fatal(err) + } if got, exp := len(values), 1; got != exp { t.Fatalf("value len mismatch: got %v, exp %v", got, exp) } + cur.Next() values, err = cur.ReadFloatBlock(&buf) + if err != nil { + t.Fatal(err) + } if got, exp := len(values), 1; got != exp { t.Fatalf("value len mismatch: got %v, exp %v", got, exp) } @@ -2082,6 +2109,9 @@ func TestFileStore_Replace(t *testing.T) { // No more blocks for this cursor cur.Next() values, err = cur.ReadFloatBlock(&buf) + if err != nil { + t.Fatal(err) + } if got, exp := len(values), 0; got != exp { t.Fatalf("value len mismatch: got %v, exp %v", got, exp) } diff --git a/tsdb/engine/tsm1/reader_test.go b/tsdb/engine/tsm1/reader_test.go index 242badee11..45a771eb51 100644 --- a/tsdb/engine/tsm1/reader_test.go +++ b/tsdb/engine/tsm1/reader_test.go @@ -763,6 +763,9 @@ func TestIndirectIndex_Type(t *testing.T) { index.Add("cpu", tsm1.BlockInteger, 0, 1, 10, 20) b, err := index.MarshalBinary() + if err != nil { + t.Fatal(err) + } ind := tsm1.NewIndirectIndex() if err := ind.UnmarshalBinary(b); err != nil { diff --git a/tsdb/engine/tsm1/tombstone_test.go b/tsdb/engine/tsm1/tombstone_test.go index e910ba9ff5..64d0ecddfd 100644 --- a/tsdb/engine/tsm1/tombstone_test.go +++ b/tsdb/engine/tsm1/tombstone_test.go @@ -173,12 +173,12 @@ func TestTombstoner_ReadV1(t *testing.T) { ts := &tsm1.Tombstoner{Path: f.Name()} - entries, err := ts.ReadAll() + _, err := ts.ReadAll() if err != nil { fatal(t, "ReadAll", err) } - entries, err = ts.ReadAll() + entries, err := ts.ReadAll() if err != nil { fatal(t, "ReadAll", err) } @@ -220,12 +220,12 @@ func TestTombstoner_ReadEmptyV1(t *testing.T) { ts := &tsm1.Tombstoner{Path: f.Name()} - entries, err := ts.ReadAll() + _, err := ts.ReadAll() if err != nil { fatal(t, "ReadAll", err) } - entries, err = ts.ReadAll() + entries, err := ts.ReadAll() if err != nil { fatal(t, "ReadAll", err) } From a8343dee9946d6064330b068a46f5c15b178fe2f Mon Sep 17 00:00:00 2001 From: Gustav Westling Date: Thu, 29 Dec 2016 20:41:00 +0100 Subject: [PATCH 2/5] Fix broken test. Change Fatalf to Fatal. --- services/graphite/parser_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/graphite/parser_test.go b/services/graphite/parser_test.go index 1c1603d7c1..4ee8c34af3 100644 --- a/services/graphite/parser_test.go +++ b/services/graphite/parser_test.go @@ -680,7 +680,7 @@ func TestApplyTemplateField(t *testing.T) { measurement, _, field, err := p.ApplyTemplate("current.users.logged_in") if err != nil { - t.Fatalf(err) + t.Fatal(err) } if measurement != "current_users" { From 26b33307aea507bef2c81152c5843413187dd58c Mon Sep 17 00:00:00 2001 From: Gustav Westling Date: Fri, 30 Dec 2016 11:42:38 +0100 Subject: [PATCH 3/5] Resolved PR comments on test files --- coordinator/statement_executor_test.go | 4 ++-- influxql/query_executor_test.go | 5 ----- tsdb/engine/tsm1/file_store.go | 7 +++++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/coordinator/statement_executor_test.go b/coordinator/statement_executor_test.go index 640caafac5..a58fa4438d 100644 --- a/coordinator/statement_executor_test.go +++ b/coordinator/statement_executor_test.go @@ -198,11 +198,11 @@ func NewQueryExecutor() *QueryExecutor { var out io.Writer = &e.LogOutput if testing.Verbose() { - io.MultiWriter(out, os.Stderr) + out = io.MultiWriter(out, os.Stderr) } e.QueryExecutor.WithLogger(zap.New( zap.NewTextEncoder(), - zap.Output(os.Stderr), + zap.Output(zap.AddSync(out)), )) return e diff --git a/influxql/query_executor_test.go b/influxql/query_executor_test.go index 8e90ef8cc1..8b93f4c8f3 100644 --- a/influxql/query_executor_test.go +++ b/influxql/query_executor_test.go @@ -143,11 +143,6 @@ func TestQueryExecutor_Abort(t *testing.T) { } func TestQueryExecutor_ShowQueries(t *testing.T) { - _, err := influxql.ParseQuery(`SELECT count(value) FROM cpu`) - if err != nil { - t.Fatal(err) - } - e := NewQueryExecutor() e.StatementExecutor = &StatementExecutor{ ExecuteStatementFn: func(stmt influxql.Statement, ctx influxql.ExecutionContext) error { diff --git a/tsdb/engine/tsm1/file_store.go b/tsdb/engine/tsm1/file_store.go index a178df5f0b..73f4a36201 100644 --- a/tsdb/engine/tsm1/file_store.go +++ b/tsdb/engine/tsm1/file_store.go @@ -846,12 +846,15 @@ func ParseTSMFileName(name string) (int, int, error) { generation, err := strconv.ParseUint(id[:idx], 10, 32) if err != nil { - return 0, 0, err + return 0, 0, fmt.Errorf("file %s is named incorrectly", name) } sequence, err := strconv.ParseUint(id[idx+1:], 10, 32) + if err != nil { + return 0, 0, fmt.Errorf("file %s is named incorrectly", name) + } - return int(generation), int(sequence), err + return int(generation), int(sequence), nil } type KeyCursor struct { From 13efbc6ce0730adecd6982f7db988eef9bd77cd3 Mon Sep 17 00:00:00 2001 From: Gustav Westling Date: Fri, 30 Dec 2016 12:22:09 +0100 Subject: [PATCH 4/5] Removed empty line --- services/subscriber/http.go | 1 - 1 file changed, 1 deletion(-) diff --git a/services/subscriber/http.go b/services/subscriber/http.go index 64ca649ada..43e7cc56e9 100644 --- a/services/subscriber/http.go +++ b/services/subscriber/http.go @@ -23,7 +23,6 @@ func NewHTTP(addr string, timeout time.Duration) (*HTTP, error) { // NewHTTPS returns a new HTTPS points writer with default options and HTTPS configured func NewHTTPS(addr string, timeout time.Duration, unsafeSsl bool, caCerts string) (*HTTP, error) { tlsConfig, err := createTlsConfig(caCerts) - if err != nil { return nil, err } From 69c5354d984b6e482addce95d784b819236a4fd6 Mon Sep 17 00:00:00 2001 From: Gustav Westling Date: Fri, 30 Dec 2016 12:23:40 +0100 Subject: [PATCH 5/5] Use length instead of removing it --- cmd/influxd/restore/restore.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/influxd/restore/restore.go b/cmd/influxd/restore/restore.go index 1504a692aa..dda2a42a45 100644 --- a/cmd/influxd/restore/restore.go +++ b/cmd/influxd/restore/restore.go @@ -158,7 +158,8 @@ func (cmd *Command) unpackMeta() error { // Size of the node.json bytes i += 8 - nodeBytes := b[i:] + length = int(binary.BigEndian.Uint64(b[i : i+8])) + nodeBytes := b[i : i+length] // Unpack into metadata. var data meta.Data