From 77d49ded6494999f207e4ed90e12f4374f9bef1b Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 29 Mar 2019 14:40:21 -0500 Subject: [PATCH] so much lint So with the switch to a new linter, we get a lot of new warnings, and the majority of them are harmless probably, but a few might be real. Variously just use _ to suppress warnings, or report errors. There's probably things here that deserve better fixes, but we can always revisit it. --- cluster.go | 11 +- cluster_internal_test.go | 68 ++++++++--- cmd/root.go | 3 +- cmd/root_test.go | 6 +- ctl/check.go | 15 ++- ctl/check_test.go | 16 ++- ctl/config_test.go | 13 +- ctl/export_test.go | 12 +- ctl/generate_config_test.go | 12 +- ctl/import_test.go | 194 +++++++++++++++++++++++------- ctl/inspect.go | 8 +- ctl/inspect_test.go | 13 +- diagnostics_internal_test.go | 21 +++- enterprise/b/btree.go | 4 +- enterprise/b/containers_btree.go | 3 +- field.go | 4 +- field_internal_test.go | 10 +- field_test.go | 15 ++- fragment.go | 4 +- fragment_internal_test.go | 89 +++++++++++--- gopsutil/systeminfo.go | 4 +- holder.go | 8 +- holder_internal_test.go | 46 +++++-- holder_test.go | 14 ++- http/client.go | 5 +- http/client_test.go | 15 ++- http/handler.go | 45 +++++-- logger/logger.go | 23 ++++ lru/lru.go | 4 +- pql/pql.peg.go | 2 +- roaring/btree.go | 10 +- roaring/containers_btree.go | 3 +- roaring/roaring.go | 13 +- roaring/roaring_internal_test.go | 55 +++++++-- roaring/roaring_test.go | 199 +++++++++++++++++++------------ server.go | 7 +- server/handler_test.go | 10 +- server/server_test.go | 7 +- utils_internal_test.go | 6 +- view.go | 2 +- 40 files changed, 733 insertions(+), 266 deletions(-) diff --git a/cluster.go b/cluster.go index 204a1016b..024c426a1 100644 --- a/cluster.go +++ b/cluster.go @@ -844,8 +844,8 @@ func (c *cluster) partition(index string, shard uint64) int { // Hash the bytes and mod by partition count. h := fnv.New64a() - h.Write([]byte(index)) - h.Write(buf[:]) + _, _ = h.Write([]byte(index)) + _, _ = h.Write(buf[:]) return int(h.Sum64() % uint64(c.partitionN)) } @@ -1892,7 +1892,12 @@ func (c *cluster) mergeClusterStatus(cs *ClusterStatus) error { for _, node := range officialNodes { if node.ID == c.Node.ID && node.State != c.Node.State { c.logger.Printf("mismatched state in mergeClusterStatus got %v have %v", node.State, c.Node.State) - go c.setNodeState(c.Node.State) + go func() { + err := c.setNodeState(c.Node.State) + if err != nil { + c.logger.Printf("error setting node state from %v to %v: %v", node.State, c.Node.State, err) + } + }() } if err := c.addNode(node); err != nil { return errors.Wrap(err, "adding node") diff --git a/cluster_internal_test.go b/cluster_internal_test.go index fa40c48b7..84392ffd9 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -607,7 +607,9 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Single node, in topology", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode() + if err := tc.addNode(); err != nil { + t.Fatalf("adding node: %v", err) + } node := tc.Clusters[0] @@ -615,7 +617,9 @@ func TestCluster_ResizeStates(t *testing.T) { top := &Topology{ nodeIDs: []string{node.Node.ID}, } - tc.WriteTopology(node.Path, top) + if err := tc.WriteTopology(node.Path, top); err != nil { + t.Fatalf("writing topology: %v", err) + } // Open TestCluster. if err := tc.Open(); err != nil { @@ -635,7 +639,9 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Single node, not in topology", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode() + if err := tc.addNode(); err != nil { + t.Fatalf("adding node: %v", err) + } node := tc.Clusters[0] @@ -643,7 +649,9 @@ func TestCluster_ResizeStates(t *testing.T) { top := &Topology{ nodeIDs: []string{"some-other-host"}, } - tc.WriteTopology(node.Path, top) + if err := tc.WriteTopology(node.Path, top); err != nil { + t.Fatalf("writing topology: %v", err) + } // Open TestCluster. expected := "coordinator node0 is not in topology: [some-other-host]" @@ -660,14 +668,18 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Multiple nodes, no data", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode() + if err := tc.addNode(); err != nil { + t.Fatalf("adding node: %v", err) + } // Open TestCluster. if err := tc.Open(); err != nil { - t.Fatal(err) + t.Fatalf("opening cluster: %v", err) } - tc.addNode() + if err := tc.addNode(); err != nil { + t.Fatalf("adding node: %v", err) + } node0 := tc.Clusters[0] node1 := tc.Clusters[1] @@ -698,18 +710,23 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Multiple nodes, in/not in topology", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode() + err := tc.addNode() + if err != nil { + t.Fatalf("adding node: %v", err) + } node0 := tc.Clusters[0] // write topology to data file top := &Topology{ nodeIDs: []string{"node0", "node2"}, } - tc.WriteTopology(node0.Path, top) + if err := tc.WriteTopology(node0.Path, top); err != nil { + t.Fatalf("writing topology: %v", err) + } // Open TestCluster. if err := tc.Open(); err != nil { - t.Fatal(err) + t.Fatalf("opening cluster: %v", err) } // Ensure that node is in state STARTING before the other node joins. @@ -719,19 +736,22 @@ func TestCluster_ResizeStates(t *testing.T) { // Expect an error by adding a node not in the topology. expectedError := "host is not in topology: node1" - err := tc.addNode() + err = tc.addNode() if err == nil || err.Error() != expectedError { t.Errorf("did not receive expected error: %s", expectedError) } - tc.addNode() + err = tc.addNode() + if err != nil { + t.Fatalf("adding node: %v", err) + } node2 := tc.Clusters[2] // Ensure that node comes up in state NORMAL. if node0.State() != ClusterStateNormal { t.Errorf("expected node0 state: %v, but got: %v", ClusterStateNormal, node0.State()) } else if node2.State() != ClusterStateNormal { - t.Errorf("expected node1 state: %v, but got: %v", ClusterStateNormal, node2.State()) + t.Errorf("expected node2 state: %v, but got: %v", ClusterStateNormal, node2.State()) } // Close TestCluster. @@ -742,20 +762,27 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Multiple nodes, with data", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode() + err := tc.addNode() + if err != nil { + t.Fatalf("adding node: %v", err) + } node0 := tc.Clusters[0] // Open TestCluster. - if err := tc.Open(); err != nil { + if err = tc.Open(); err != nil { t.Fatal(err) } // Add Bit Data to node0. if err := tc.CreateField("i", "f", OptFieldTypeDefault()); err != nil { - t.Fatal(err) + t.Fatalf("creating field: %v", err) + } + if err := tc.SetBit("i", "f", 1, 101, nil); err != nil { + t.Fatalf("setting bit: %v", err) + } + if err := tc.SetBit("i", "f", 1, ShardWidth+1, nil); err != nil { + t.Fatalf("setting bit: %v", err) } - tc.SetBit("i", "f", 1, 101, nil) - tc.SetBit("i", "f", 1, ShardWidth+1, nil) // Before starting the resize, get the CheckSum to use for // comparison later. @@ -765,7 +792,10 @@ func TestCluster_ResizeStates(t *testing.T) { node0Checksum := node0Fragment.Checksum() // addNode needs to block until the resize process has completed. - tc.addNode() + err = tc.addNode() + if err != nil { + t.Fatalf("adding node: %v", err) + } node1 := tc.Clusters[1] // Ensure that nodes come up in state NORMAL. diff --git a/cmd/root.go b/cmd/root.go index 64b8b1ca9..a8f8e6ddc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -54,9 +54,8 @@ Build Time: ` + pilosa.BuildTime + "\n", if ret, err := cmd.Flags().GetBool("dry-run"); ret && err == nil { if cmd.Parent() != nil { return fmt.Errorf("dry run") - } else if err != nil { - return fmt.Errorf("problem getting dry-run flag: %v", err) } + return fmt.Errorf("problem getting dry-run flag: %v", err) } return nil diff --git a/cmd/root_test.go b/cmd/root_test.go index 28bbea20d..6807aaf3c 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -187,10 +187,12 @@ bind = "127.0.0.1:10101" "127.0.0.1:10101", "127.0.0.1:10111", ]` - file.Write([]byte(config)) + if _, err := file.Write([]byte(config)); err != nil { + t.Fatalf("writing config file: %v", err) + } file.Close() _, err = ExecNewRootCommand(t, "server", "--config", file.Name()) - if err.Error() != "invalid option in configuration file: cluster.partitions" { + if err == nil || err.Error() != "invalid option in configuration file: cluster.partitions" { t.Fatalf("Expected invalid option in configuration file, but err: '%v'", err) } } diff --git a/ctl/check.go b/ctl/check.go index b1389b6ee..0beee5bc5 100644 --- a/ctl/check.go +++ b/ctl/check.go @@ -68,7 +68,7 @@ func (cmd *CheckCommand) Run(_ context.Context) error { } // checkBitmapFile performs a consistency check on path for a roaring bitmap file. -func (cmd *CheckCommand) checkBitmapFile(path string) error { +func (cmd *CheckCommand) checkBitmapFile(path string) (err error) { // Open file handle. f, err := os.Open(path) if err != nil { @@ -86,8 +86,17 @@ func (cmd *CheckCommand) checkBitmapFile(path string) error { if err != nil { return errors.Wrap(err, "mmapping") } - defer syscall.Munmap(data) - + defer func() { + e := syscall.Munmap(data) + if e != nil { + fmt.Fprintf(cmd.Stderr, "WARNING: munmap failed: %v", e) + } + // don't overwrite another error with this, but also indicate + // this error. + if err == nil { + err = e + } + }() // Attach the mmap file to the bitmap. bm := roaring.NewBitmap() if err := bm.UnmarshalBinary(data); err != nil { diff --git a/ctl/check_test.go b/ctl/check_test.go index 1b2ebd6f6..e37d99358 100644 --- a/ctl/check_test.go +++ b/ctl/check_test.go @@ -40,7 +40,9 @@ func TestCheckCommand_RunCacheFile(t *testing.T) { err := cm.Run(context.Background()) w.Close() var buf bytes.Buffer - io.Copy(&buf, r) + if _, err := io.Copy(&buf, r); err != nil { + t.Fatalf("copy: %v", err) + } if !strings.Contains(buf.String(), "ignoring cache file") { t.Fatalf("expect: ignoring cache file, actual: '%s'", err) @@ -59,7 +61,9 @@ func TestCheckCommand_RunSnapshot(t *testing.T) { err := cm.Run(context.Background()) w.Close() var buf bytes.Buffer - io.Copy(&buf, r) + if _, err := io.Copy(&buf, r); err != nil { + t.Fatalf("copy: %v", err) + } if !strings.Contains(buf.String(), "ignoring snapshot file") { t.Fatalf("expect: ignoring snapshot file, actual: '%s'", err) @@ -71,7 +75,9 @@ func TestCheckCommand_Run(t *testing.T) { if err != nil { t.Fatal(err) } - file.Write([]byte("1234,1223")) + if _, err := file.Write([]byte("1234,1223")); err != nil { + t.Fatalf("writing to temp file: %v", err) + } file.Close() rder := []byte{} @@ -83,7 +89,9 @@ func TestCheckCommand_Run(t *testing.T) { err = cm.Run(context.Background()) w.Close() var buf bytes.Buffer - io.Copy(&buf, r) + if _, err := io.Copy(&buf, r); err != nil { + t.Fatalf("copy: %v", err) + } if !strings.HasPrefix(err.Error(), "checking bitmap: unmarshalling: reading roaring header:") { t.Fatalf("expect error: invalid roaring file, actual: '%s'", err) diff --git a/ctl/config_test.go b/ctl/config_test.go index ca08c273b..b9a5dc0b5 100644 --- a/ctl/config_test.go +++ b/ctl/config_test.go @@ -33,13 +33,16 @@ func TestConfigCommand_Run(t *testing.T) { cm.Config = server.NewConfig() err := cm.Run(context.Background()) - w.Close() - var buf bytes.Buffer - io.Copy(&buf, r) - if err != nil { t.Fatalf("Config Run doesn't work: %s", err) - } else if !strings.Contains(buf.String(), ":10101") { + } + w.Close() + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(buf.String(), ":10101") { t.Fatalf("Unexpected config: \n%s", buf.String()) } } diff --git a/ctl/export_test.go b/ctl/export_test.go index e3189efe3..6cb611460 100644 --- a/ctl/export_test.go +++ b/ctl/export_test.go @@ -52,8 +52,16 @@ func TestExportCommand_Run(t *testing.T) { hostport := cmd.API.Node().URI.HostPort() cm.Host = hostport - http.DefaultClient.Do(test.MustNewHTTPRequest("POST", "http://"+hostport+"/index/i", strings.NewReader(""))) - http.DefaultClient.Do(test.MustNewHTTPRequest("POST", "http://"+hostport+"/index/i/field/f", strings.NewReader(""))) + resp, err := http.DefaultClient.Do(test.MustNewHTTPRequest("POST", "http://"+hostport+"/index/i", strings.NewReader(""))) + if err != nil { + t.Fatalf("making http request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(test.MustNewHTTPRequest("POST", "http://"+hostport+"/index/i/field/f", strings.NewReader(""))) + if err != nil { + t.Fatalf("making http request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" diff --git a/ctl/generate_config_test.go b/ctl/generate_config_test.go index 26b531e8f..a431cd468 100644 --- a/ctl/generate_config_test.go +++ b/ctl/generate_config_test.go @@ -29,12 +29,16 @@ func TestGenerateConfigCommand_Run(t *testing.T) { r, w, _ := os.Pipe() cm := NewGenerateConfigCommand(stdin, w, os.Stderr) err := cm.Run(context.Background()) - w.Close() - var buf bytes.Buffer - io.Copy(&buf, r) if err != nil { t.Fatalf("Config Run doesn't work: %s", err) - } else if !strings.Contains(buf.String(), ":10101") { + } + w.Close() + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(buf.String(), ":10101") { t.Fatalf("Unexpected config: %s", buf.String()) } } diff --git a/ctl/import_test.go b/ctl/import_test.go index e701af1b2..b20919203 100644 --- a/ctl/import_test.go +++ b/ctl/import_test.go @@ -58,7 +58,13 @@ func TestImportCommand_Basic(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import.csv") - file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("creating tempfile: %v", err) + } + _, err = file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } ctx := context.Background() if err != nil { t.Fatal(err) @@ -82,11 +88,14 @@ func TestImportCommand_Basic(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import.csv") - file.Write([]byte("1,2\n3,4\n5,6")) - ctx := context.Background() if err != nil { - t.Fatal(err) + t.Fatalf("creating tempfile: %v", err) } + _, err = file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } + ctx := context.Background() cmd := test.MustRunCluster(t, 1)[0] cm.Host = cmd.API.Node().URI.HostPort() @@ -110,17 +119,28 @@ func TestImportCommand_RunValue(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import-value.csv") - file.Write([]byte("1,2\n3,4\n5,6")) - ctx := context.Background() if err != nil { - t.Fatal(err) + t.Fatalf("creating tempfile: %v", err) } + _, err = file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } + ctx := context.Background() cmd := test.MustRunCluster(t, 1)[0] cm.Host = cmd.API.Node().URI.HostPort() - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max": 100}}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) + if err != nil { + t.Fatalf("http request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max": 100}}`))) + if err != nil { + t.Fatalf("http request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -136,7 +156,13 @@ func TestImportCommand_RunValue(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import-value.csv") - file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("creating tempfile: %v", err) + } + _, err = file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } ctx := context.Background() if err != nil { t.Fatal(err) @@ -145,8 +171,16 @@ func TestImportCommand_RunValue(t *testing.T) { cmd := test.MustRunCluster(t, 1)[0] cm.Host = cmd.API.Node().URI.HostPort() - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max": 100}}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max": 100}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -165,17 +199,28 @@ func TestImportCommand_RunKeys(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import-key.csv") - file.Write([]byte("foo1,bar2\nfoo3,bar4\nfoo5,bar6")) - ctx := context.Background() if err != nil { t.Fatal(err) } + _, err = file.Write([]byte("foo1,bar2\nfoo3,bar4\nfoo5,bar6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } + ctx := context.Background() cmd := test.MustRunCluster(t, 1)[0] cm.Host = cmd.API.Node().URI.HostPort() - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(`{"options":{"keys": true}}`))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"keys": true}}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(`{"options":{"keys": true}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"keys": true}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -192,7 +237,9 @@ func TestImportCommand_KeyReplication(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import-key.csv") - + if err != nil { + t.Fatal(err) + } // create a large import file in order to test the // translateStoreBufferSize growth logic. keyBytes := []byte{} @@ -205,11 +252,11 @@ func TestImportCommand_KeyReplication(t *testing.T) { x := "fooEND,barEND" keyBytes = append(keyBytes, x...) - file.Write(keyBytes) - ctx := context.Background() + _, err = file.Write(keyBytes) if err != nil { - t.Fatal(err) + t.Fatalf("writing to tempfile: %v", err) } + ctx := context.Background() c := test.MustRunCluster(t, 2) cmd0 := c[0] @@ -220,8 +267,16 @@ func TestImportCommand_KeyReplication(t *testing.T) { cm.Host = host0 - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(`{"options":{"keys": true}}`))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"keys": true}}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(`{"options":{"keys": true}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"keys": true}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -255,17 +310,28 @@ func TestImportCommand_RunValueKeys(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import-key.csv") - file.Write([]byte("foo1,2\nfoo3,4\nfoo5,6")) - ctx := context.Background() if err != nil { t.Fatal(err) } + _, err = file.Write([]byte("foo1,bar2\nfoo3,bar4\nfoo5,bar6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } + ctx := context.Background() cmd := test.MustRunCluster(t, 1)[0] cm.Host = cmd.API.Node().URI.HostPort() - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(`{"options":{"keys": true}}`))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max": 100}}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(`{"options":{"keys": true}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max": 100}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -286,9 +352,12 @@ func TestImportCommand_InvalidFile(t *testing.T) { cm.Index = "i" cm.Field = "f" file, err := ioutil.TempFile("", "import.csv") - file.Write([]byte("a,2\n3,5\n5,6")) if err != nil { - t.Fatal(err) + t.Fatalf("creating tempfile: %v", err) + } + _, err = file.Write([]byte("1,2\n3,4\n5,6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) } cm.Paths = []string{file.Name()} err = cm.Run(context.Background()) @@ -297,9 +366,12 @@ func TestImportCommand_InvalidFile(t *testing.T) { } file, err = ioutil.TempFile("", "import1.csv") - file.Write([]byte("1,\n3,\n5,6")) if err != nil { - t.Fatal(err) + t.Fatalf("creating tempfile: %v", err) + } + _, err = file.Write([]byte("1,\n3,\n5,6")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) } cm.Paths = []string{file.Name()} err = cm.Run(context.Background()) @@ -308,7 +380,10 @@ func TestImportCommand_InvalidFile(t *testing.T) { } file, err = ioutil.TempFile("", "import1.csv") - file.Write([]byte("1,2,34343\n1,3,54565,\n5,6,565")) + if err != nil { + t.Fatal(err) + } + _, err = file.Write([]byte("1,2,34343\n1,3,54565,\n5,6,565")) if err != nil { t.Fatal(err) } @@ -319,7 +394,10 @@ func TestImportCommand_InvalidFile(t *testing.T) { } file, err = ioutil.TempFile("", "import1.csv") - file.Write([]byte("1\n3\n5")) + if err != nil { + t.Fatal(err) + } + _, err = file.Write([]byte("1\n3\n5")) if err != nil { t.Fatal(err) } @@ -357,16 +435,27 @@ func TestImportCommand_BugOverwriteValue(t *testing.T) { stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) file, err := ioutil.TempFile("", "import-value.csv") - file.Write([]byte("0,17\n")) - ctx := context.Background() if err != nil { t.Fatal(err) } + _, err = file.Write([]byte("0,17\n")) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() cm.Host = cmd.API.Node().URI.HostPort() - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max":2147483648 }}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "int", "min": 0, "max":2147483648 }}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -381,7 +470,10 @@ func TestImportCommand_BugOverwriteValue(t *testing.T) { if err != nil { t.Fatalf("Error creating tempfile: %s", err) } - file.Write([]byte("0,16\n")) + _, err = file.Write([]byte("0,16\n")) + if err != nil { + t.Fatalf("writing bytes to tempfile: %v", err) + } cm.Paths = []string{file.Name()} err = cm.Run(ctx) if err != nil { @@ -393,7 +485,10 @@ func TestImportCommand_BugOverwriteValue(t *testing.T) { if err != nil { t.Fatalf("Error creating tempfile: %s", err) } - file.Write([]byte("0,19\n")) + _, err = file.Write([]byte("0,19\n")) + if err != nil { + t.Fatalf("writing bytes to tempfile: %v", err) + } cm.Paths = []string{file.Name()} err = cm.Run(ctx) if err != nil { @@ -411,8 +506,16 @@ func TestImportCommand_RunBool(t *testing.T) { cmd := test.MustRunCluster(t, 1)[0] cm.Host = cmd.API.Node().URI.HostPort() - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) - http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "bool"}}`))) + resp, err := http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i", strings.NewReader(""))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() + resp, err = http.DefaultClient.Do(MustNewHTTPRequest("POST", "http://"+cm.Host+"/index/i/field/f", strings.NewReader(`{"options":{"type": "bool"}}`))) + if err != nil { + t.Fatalf("posting request: %v", err) + } + resp.Body.Close() cm.Index = "i" cm.Field = "f" @@ -422,7 +525,10 @@ func TestImportCommand_RunBool(t *testing.T) { if err != nil { t.Fatal(err) } - file.Write([]byte("0,1\n1,2\n1,3")) + _, err = file.Write([]byte("0,1\n1,2\n1,3")) + if err != nil { + t.Fatalf("writing bytes to tempfile: %v", err) + } cm.Paths = []string{file.Name()} err = cm.Run(ctx) @@ -437,8 +543,10 @@ func TestImportCommand_RunBool(t *testing.T) { if err != nil { t.Fatal(err) } - file.Write([]byte("0,1\n1,2\n1,3\n2,4")) - + _, err = file.Write([]byte("0,1\n1,2\n1,3\n2,4")) + if err != nil { + t.Fatalf("writing bytes to tempfile: %v", err) + } cm.Paths = []string{file.Name()} err = cm.Run(ctx) if !strings.Contains(err.Error(), "bool field imports only support values 0 and 1") { diff --git a/ctl/inspect.go b/ctl/inspect.go index c0676f8da..204ffab3b 100644 --- a/ctl/inspect.go +++ b/ctl/inspect.go @@ -64,8 +64,12 @@ func (cmd *InspectCommand) Run(_ context.Context) error { if err != nil { return errors.Wrap(err, "mmapping") } - defer syscall.Munmap(data) - + defer func() { + err := syscall.Munmap(data) + if err != nil { + fmt.Fprintf(cmd.Stderr, "inspect command: munmap failed: %v", err) + } + }() // Attach the mmap file to the bitmap. t := time.Now() fmt.Fprintf(cmd.Stderr, "unmarshaling bitmap...") diff --git a/ctl/inspect_test.go b/ctl/inspect_test.go index c7a48d400..265d77a12 100644 --- a/ctl/inspect_test.go +++ b/ctl/inspect_test.go @@ -34,14 +34,23 @@ func TestInspectCommand_Run(t *testing.T) { if err != nil { t.Fatalf("Error creating tempfile: %s", err) } - file.Write([]byte("12358267538963")) + _, err = file.Write([]byte("12358267538963")) + if err != nil { + t.Fatalf("writing to tempfile: %v", err) + } file.Close() cm.Path = file.Name() err = cm.Run(context.Background()) + if err != nil { + t.Fatalf("can't run command: %v", err) + } w.Close() var buf bytes.Buffer - io.Copy(&buf, r) + _, err = io.Copy(&buf, r) + if err != nil { + t.Fatalf("copying data: %v", err) + } if !strings.Contains(buf.String(), "unmarshaling bitmap...") { t.Fatalf("Inspect doesn't work: %s", err) } diff --git a/diagnostics_internal_test.go b/diagnostics_internal_test.go index 99c0a83ac..6ddc98b32 100644 --- a/diagnostics_internal_test.go +++ b/diagnostics_internal_test.go @@ -22,6 +22,8 @@ import ( "runtime" "strings" "testing" + + "github.com/pilosa/pilosa/logger" ) func TestDiagnosticsClient(t *testing.T) { @@ -112,19 +114,34 @@ func TestDiagnosticsVersion_Check(t *testing.T) { // Mock server. server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(versionResponse{ + err := json.NewEncoder(w).Encode(versionResponse{ Version: "1.1.1", }) + if err != nil { + t.Fatalf("couldn't encode version response: %v", err) + } })) // Create a new client. d := newDiagnosticsCollector("localhost:10101") + logs := logger.NewCaptureLogger() + d.Logger = logs + version := "0.1.1" d.SetVersion(version) d.VersionURL = server.URL - d.CheckVersion() + err := d.CheckVersion() + if err != nil { + t.Fatalf("checking version: %v", err) + } + if len(logs.Prints) != 1 { + t.Fatalf("expected a version upgrade message") + } + if !strings.Contains(logs.Prints[0], "a newer version") { + t.Fatalf("expected version upgrade message, got '%s'", logs.Prints[0]) + } } func compareJSON(a, b []byte) (bool, error) { diff --git a/enterprise/b/btree.go b/enterprise/b/btree.go index c61f5c5b6..2fa5c24e8 100644 --- a/enterprise/b/btree.go +++ b/enterprise/b/btree.go @@ -873,7 +873,7 @@ func (e *enumerator) Next() (k uint64, v *roaring.Container, err error) { i := e.q.d[e.i] k, v = i.k, i.v e.k, e.hit = k, true - e.next() + _ = e.next() return k, v, nil } @@ -928,7 +928,7 @@ func (e *enumerator) Prev() (k uint64, v *roaring.Container, err error) { i := e.q.d[e.i] k, v = i.k, i.v e.k, e.hit = k, true - e.prev() + _ = e.prev() return k, v, err } diff --git a/enterprise/b/containers_btree.go b/enterprise/b/containers_btree.go index db5c9946d..a051d7cde 100644 --- a/enterprise/b/containers_btree.go +++ b/enterprise/b/containers_btree.go @@ -44,7 +44,8 @@ func NewBTreeBitmap(a ...uint64) *roaring.Bitmap { b := &roaring.Bitmap{ Containers: newBTreeContainers(), } - b.Add(a...) + // TODO: there's no way to report an error here + _, _ = b.Add(a...) return b } diff --git a/field.go b/field.go index bd4dd1aca..d83735893 100644 --- a/field.go +++ b/field.go @@ -612,7 +612,9 @@ func (f *Field) createBSIGroup(bsig *bsiGroup) error { if err := f.addBSIGroup(bsig); err != nil { return err } - f.saveMeta() + if err := f.saveMeta(); err != nil { + return errors.Wrap(err, "saving") + } return nil } diff --git a/field_internal_test.go b/field_internal_test.go index 0a26225bb..ad8337c31 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -370,7 +370,10 @@ func TestField_PersistAvailableShardsFootprint(t *testing.T) { // bm represents remote available shards. bm := roaring.NewBitmap() for i := uint64(0); i < 1204; i += 2 { - bm.Add(i) + _, err := bm.Add(i) + if err != nil { + t.Fatalf("adding bits: %v", err) + } } if err := f.AddRemoteAvailableShards(bm); err != nil { @@ -386,7 +389,10 @@ func TestField_PersistAvailableShardsFootprint(t *testing.T) { bm1 := roaring.NewBitmap() for i := uint64(1); i < 1204; i += 2 { - bm1.Add(i) + _, err := bm1.Add(i) + if err != nil { + t.Fatalf("adding bits: %v", err) + } } if err := f.AddRemoteAvailableShards(bm1); err != nil { diff --git a/field_test.go b/field_test.go index 2a0efc0b9..5911abd6a 100644 --- a/field_test.go +++ b/field_test.go @@ -208,17 +208,20 @@ func TestField_AvailableShards(t *testing.T) { } // Set remote shards and verify. - f.AddRemoteAvailableShards(roaring.NewBitmap(1, 2, 4)) + if err := f.AddRemoteAvailableShards(roaring.NewBitmap(1, 2, 4)); err != nil { + t.Fatalf("adding remote shards: %v", err) + } if diff := cmp.Diff(f.AvailableShards().Slice(), []uint64{0, 1, 2, 4}); diff != "" { t.Fatal(diff) } // Delete shards; only local shards should remain. - f.RemoveAvailableShard(0) - f.RemoveAvailableShard(1) - f.RemoveAvailableShard(2) - f.RemoveAvailableShard(3) - f.RemoveAvailableShard(4) + for i := uint64(0); i < 5; i++ { + err := f.RemoveAvailableShard(i) + if err != nil { + t.Fatalf("removing shard: %v", err) + } + } if diff := cmp.Diff(f.AvailableShards().Slice(), []uint64{0, 2}); diff != "" { t.Fatal(diff) } diff --git a/fragment.go b/fragment.go index 82288c914..2409a5fc5 100644 --- a/fragment.go +++ b/fragment.go @@ -1328,7 +1328,7 @@ type topOptions struct { func (f *fragment) Checksum() []byte { h := xxhash.New() for _, block := range f.Blocks() { - h.Write(block.Checksum) + _, _ = h.Write(block.Checksum) } return h.Sum(nil) } @@ -2332,7 +2332,7 @@ func (h *blockHasher) Sum() []byte { func (h *blockHasher) WriteValue(v uint64) { binary.BigEndian.PutUint64(h.buf[:], v) - h.hash.Write(h.buf[:]) + _, _ = h.hash.Write(h.buf[:]) } // fragmentSyncer syncs a local fragment to one on a remote host. diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 3930e98e2..8c3d65c66 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -654,7 +654,9 @@ func TestFragment_Range(t *testing.T) { func benchmarkSetValues(b *testing.B, bitDepth uint, f *fragment, cfunc func(uint64) uint64) { column := uint64(0) for i := 0; i < b.N; i++ { - f.setValue(column, bitDepth, uint64(i)) + // We're not checking the error because this is a benchmark. + // That does mean the result could be completely wrong... + _, _ = f.setValue(column, bitDepth, uint64(i)) column = cfunc(column) } } @@ -943,8 +945,14 @@ func TestFragment_Top_Filter(t *testing.T) { f.mustSetBits(102, 1, 2) f.RecalculateCache() // Assign attributes. - f.RowAttrStore.SetAttrs(101, map[string]interface{}{"x": int64(10)}) - f.RowAttrStore.SetAttrs(102, map[string]interface{}{"x": int64(20)}) + err := f.RowAttrStore.SetAttrs(101, map[string]interface{}{"x": int64(10)}) + if err != nil { + t.Fatalf("setAttrs: %v", err) + } + err = f.RowAttrStore.SetAttrs(102, map[string]interface{}{"x": int64(20)}) + if err != nil { + t.Fatalf("setAttrs: %v", err) + } // Retrieve top rows. if pairs, err := f.top(topOptions{ @@ -2064,7 +2072,10 @@ func BenchmarkImportRoaringUpdateConcurrent(b *testing.B) { for i := 0; i < b.N; i++ { for j := 0; j < concurrency; j++ { frags[j] = mustOpenFragment("i", "f", viewStandard, uint64(j), CacheTypeRanked) - frags[j].importRoaring(data, false) + err := frags[j].importRoaring(data, false) + if err != nil { + b.Fatalf("importing roaring: %v", err) + } } eg := errgroup.Group{} b.StartTimer() @@ -2275,7 +2286,10 @@ func BenchmarkImportRoaringIntoLargeFragment(b *testing.B) { func TestGetZipfRowsSliceRoaring(t *testing.T) { f := mustOpenFragment("i", "f", viewStandard, 0, DefaultCacheType) data := getZipfRowsSliceRoaring(10, 1, 0, ShardWidth) - f.importRoaring(data, false) + err := f.importRoaring(data, false) + if err != nil { + t.Fatalf("importing roaring: %v", err) + } if !reflect.DeepEqual(f.rows(0), []uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) { t.Fatalf("unexpected rows: %v", f.rows(0)) } @@ -2608,7 +2622,10 @@ func TestFragment_RoaringImport(t *testing.T) { if err != nil { t.Fatalf("writing to buffer: %v", err) } - f.importRoaring(buf.Bytes(), false) + err = f.importRoaring(buf.Bytes(), false) + if err != nil { + t.Fatalf("importing roaring: %v", err) + } exp := calcExpected(test[:num+1]...) for row, expCols := range exp { cols := f.row(uint64(row)).Columns() @@ -2680,7 +2697,10 @@ func TestFragment_RoaringImportTopN(t *testing.T) { if err != nil { t.Fatalf("writing to buffer: %v", err) } - f.importRoaring(buf.Bytes(), false) + err = f.importRoaring(buf.Bytes(), false) + if err != nil { + t.Fatalf("importing roaring: %v", err) + } rows, cols := toRowsCols(test.roaring) expPairs = calcTop(append(test.rowIDs, rows...), append(test.colIDs, cols...)) pairs, err = f.top(topOptions{}) @@ -2895,19 +2915,56 @@ func TestFragmentRowIterator(t *testing.T) { func TestUnionInPlaceMapped(t *testing.T) { f := mustOpenFragment("i", "f", "v", 0, CacheTypeNone) defer f.Clean(t) + // I know this doesn't actually matter in our current context, but + // strictly speaking, we do say you have to hold the lock while calling + // unprotectedWriteToFragment... + f.mu.Lock() + defer f.mu.Unlock() r0 := rand.New(rand.NewSource(2)) r1 := rand.New(rand.NewSource(1)) data0 := randPositions(1000000, r0) - setBM := roaring.NewBitmap() - setBM.OpWriter = nil - setBM.Add(data0...) - unprotectedWriteToFragment(f, setBM) - data1 := randPositions(1000000, r1) - setBM2 := roaring.NewBitmap() - setBM2.OpWriter = nil - setBM2.Add(data1...) + setBM0 := roaring.NewBitmap() + setBM0.OpWriter = nil + _, err := setBM0.Add(data0...) + if err != nil { + t.Fatalf("adding bits: %v", err) + } + count0 := setBM0.Count() - f.storage.UnionInPlace(setBM2) + data1 := randPositions(1000000, r1) + setBM1 := roaring.NewBitmap() + setBM1.OpWriter = nil + _, err = setBM1.Add(data1...) + if err != nil { + t.Fatalf("adding bits: %v", err) + } + count1 := setBM1.Count() + + // now we write setBM0 into f.storage. + err = unprotectedWriteToFragment(f, setBM0) + if err != nil { + t.Fatalf("trying to flush fragment to disk: %v", err) + } + countF := f.storage.Count() + + f.storage.UnionInPlace(setBM1) + countUnion := f.storage.Count() + + if count0 != countF { + t.Fatalf("writing bitmap to storage changed count: %d => %d", count0, countF) + } + min := count0 + if count1 > min { + min = count1 + } + max := count0 + count1 + // We don't know how many bits we should have, because of overlap, + // but it should be between the size of the largest bitmap and the + // sum of the bitmaps. + if countUnion < min || countUnion > max { + t.Fatalf("union of sets with cardinality %d and %d should be between %d and %d, got %d", + count0, count1, min, max, countUnion) + } } func randPositions(n int, r *rand.Rand) []uint64 { diff --git a/gopsutil/systeminfo.go b/gopsutil/systeminfo.go index b50885241..13ada756e 100644 --- a/gopsutil/systeminfo.go +++ b/gopsutil/systeminfo.go @@ -139,9 +139,7 @@ func (s *systemInfo) collectPlatformInfo() error { // we have no way to know, let's try runtime s.cpuLogicalCores = runtime.NumCPU() } - if err != nil { - return err - } + return nil } return nil } diff --git a/holder.go b/holder.go index d751c90b2..40b21e538 100644 --- a/holder.go +++ b/holder.go @@ -594,9 +594,10 @@ func (h *Holder) loadNodeID() (string, error) { } nodeIDBytes, err := ioutil.ReadFile(idPath) - if err == nil { - nodeID = strings.TrimSpace(string(nodeIDBytes)) - } else if os.IsNotExist(err) { + // apparently it's safe to call IsNotExist on something that might + // be nil: + // https://github.com/golang/go/issues/31065 + if os.IsNotExist(err) { nodeID = uuid.NewV4().String() err = ioutil.WriteFile(idPath, []byte(nodeID), 0600) if err != nil { @@ -605,6 +606,7 @@ func (h *Holder) loadNodeID() (string, error) { } else if err != nil { return "", errors.Wrap(err, "reading file") } + nodeID = strings.TrimSpace(string(nodeIDBytes)) return nodeID, nil } diff --git a/holder_internal_test.go b/holder_internal_test.go index b13df7809..0c96a81ea 100644 --- a/holder_internal_test.go +++ b/holder_internal_test.go @@ -112,8 +112,10 @@ func TestHolder_Optn(t *testing.T) { } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard"), 0000); err != nil { t.Fatal(err) } - defer os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard"), 0777) - + defer func() { + // we don't care about a failure here + _ = os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard"), 0755) + }() if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("unexpected error: %s", err) } @@ -136,7 +138,10 @@ func TestHolder_Optn(t *testing.T) { } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments"), 0000); err != nil { t.Fatal(err) } - defer os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments"), 0777) + defer func() { + // we don't care about a failure here + _ = os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments"), 0755) + }() if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("unexpected error: %s", err) @@ -165,8 +170,9 @@ func TestHolder_Optn(t *testing.T) { } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0.cache"), 0000); err != nil { t.Fatal(err) } - defer os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0.cache"), 0666) - + defer func() { + _ = os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0.cache"), 0644) + }() if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("unexpected error: %s", err) } @@ -209,8 +215,14 @@ func TestHolderCleaner_CleanHolder(t *testing.T) { hldr0.SetBit("y", "z", 10, (2*ShardWidth)+7) // Set highest shard. - hldr0.Field("i", "f").AddRemoteAvailableShards(roaring.NewBitmap(0, 1)) - hldr0.Field("y", "z").AddRemoteAvailableShards(roaring.NewBitmap(0, 1, 2)) + err := hldr0.Field("i", "f").AddRemoteAvailableShards(roaring.NewBitmap(0, 1)) + if err != nil { + t.Fatalf("adding remote shards: %v", err) + } + err = hldr0.Field("y", "z").AddRemoteAvailableShards(roaring.NewBitmap(0, 1, 2)) + if err != nil { + t.Fatalf("adding remote shards: %v", err) + } // Keep replication the same and ensure we get the expected results. cluster.ReplicaN = 2 @@ -292,8 +304,20 @@ func TestHolderCleaner_CleanHolder(t *testing.T) { func TestHolderCleaner_Reopen(t *testing.T) { h := NewHolder() h.Path = "path" - h.Open() - h.Close() - h.Open() - h.Close() + err := h.Open() + if err != nil { + t.Fatalf("couldn't open holder: %v", err) + } + err = h.Close() + if err != nil { + t.Fatalf("couldn't close holder: %v", err) + } + err = h.Open() + if err != nil { + t.Fatalf("couldn't open holder: %v", err) + } + err = h.Close() + if err != nil { + t.Fatalf("couldn't close holder: %v", err) + } } diff --git a/holder_test.go b/holder_test.go index 1bfe4615a..525ca1dc7 100644 --- a/holder_test.go +++ b/holder_test.go @@ -67,7 +67,9 @@ func TestHolder_Open(t *testing.T) { } else if err := os.Chmod(h.IndexPath("test"), 0000); err != nil { t.Fatal(err) } - defer os.Chmod(h.IndexPath("test"), 0777) + defer func() { + _ = os.Chmod(h.IndexPath("test"), 0755) + }() if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("unexpected error: %s", err) @@ -106,8 +108,9 @@ func TestHolder_Open(t *testing.T) { } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar"), 0000); err != nil { t.Fatal(err) } - defer os.Chmod(filepath.Join(h.Path, "foo", "bar"), 0777) - + defer func() { + _ = os.Chmod(filepath.Join(h.Path, "foo", "bar"), 0755) + }() if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("unexpected error: %s", err) } @@ -167,8 +170,9 @@ func TestHolder_Open(t *testing.T) { } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 0000); err != nil { t.Fatal(err) } - defer os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 0666) - + defer func() { + _ = os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 0644) + }() if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("unexpected error: %s", err) } diff --git a/http/client.go b/http/client.go index 92f6269fe..f8ad7cb37 100644 --- a/http/client.go +++ b/http/client.go @@ -629,7 +629,10 @@ func (c *InternalClient) ImportRoaring(ctx context.Context, uri *pilosa.URI, ind dec := json.NewDecoder(resp.Body) rbody := &pilosa.ImportResponse{} - dec.Decode(rbody) + err = dec.Decode(rbody) + if err != nil { + return errors.Wrap(err, "decoding response body") + } if rbody.Err != "" { return errors.Wrap(errors.New(rbody.Err), "importing roaring") } diff --git a/http/client_test.go b/http/client_test.go index 2b65f82d4..0bda138ca 100644 --- a/http/client_test.go +++ b/http/client_test.go @@ -116,9 +116,18 @@ func TestClient_MultiNode(t *testing.T) { // Rebuild the RankCache. // We have to do this to avoid the 10-second cache invalidation delay // built into cache.Invalidate() - c[0].RecalculateCaches() - c[1].RecalculateCaches() - c[2].RecalculateCaches() + err = c[0].RecalculateCaches() + if err != nil { + t.Fatalf("recalculating cache: %v", err) + } + err = c[1].RecalculateCaches() + if err != nil { + t.Fatalf("recalculating cache: %v", err) + } + err = c[2].RecalculateCaches() + if err != nil { + t.Fatalf("recalculating cache: %v", err) + } // Connect to each node to compare results. client := make([]*Client, 3) diff --git a/http/handler.go b/http/handler.go index 5b79deef6..bbb6e28a2 100644 --- a/http/handler.go +++ b/http/handler.go @@ -320,6 +320,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // successResponse is a general success/error struct for http responses. type successResponse struct { + h *Handler Success bool `json:"success"` Error *Error `json:"error,omitempty"` } @@ -367,8 +368,18 @@ func (r *successResponse) write(w http.ResponseWriter, err error) { // Write the response. if statusCode == 0 { - w.Write(msg) - w.Write([]byte("\n")) + _, err := w.Write(msg) + if err != nil { + r.h.logger.Printf("error writing response: %v", err) + http.Error(w, string(msg), http.StatusInternalServerError) + return + } + _, err = w.Write([]byte("\n")) + if err != nil { + r.h.logger.Printf("error writing newline after response: %v", err) + http.Error(w, string(msg), http.StatusInternalServerError) + return + } } else { http.Error(w, string(msg), statusCode) } @@ -449,7 +460,10 @@ func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { req, err := h.readQueryRequest(r) if err != nil { w.WriteHeader(http.StatusBadRequest) - h.writeQueryResponse(w, r, &pilosa.QueryResponse{Err: err}) + e := h.writeQueryResponse(w, r, &pilosa.QueryResponse{Err: err}) + if e != nil { + h.logger.Printf("write query response error: %v (while trying to write another error: %v)", e, err) + } return } // TODO: Remove @@ -463,7 +477,10 @@ func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { default: w.WriteHeader(http.StatusBadRequest) } - h.writeQueryResponse(w, r, &pilosa.QueryResponse{Err: err}) + e := h.writeQueryResponse(w, r, &pilosa.QueryResponse{Err: err}) + if e != nil { + h.logger.Printf("write query response error: %v (while trying to write another error: %v)", e, err) + } return } @@ -612,7 +629,7 @@ func (h *Handler) handleDeleteIndex(w http.ResponseWriter, r *http.Request) { indexName := mux.Vars(r)["index"] - resp := successResponse{} + resp := successResponse{h: h} err := h.api.DeleteIndex(r.Context(), indexName) resp.write(w, err) } @@ -625,7 +642,7 @@ func (h *Handler) handlePostIndex(w http.ResponseWriter, r *http.Request) { } indexName := mux.Vars(r)["index"] - resp := successResponse{} + resp := successResponse{h: h} // Decode request. req := postIndexRequest{ @@ -694,7 +711,7 @@ func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) { indexName := mux.Vars(r)["index"] fieldName := mux.Vars(r)["field"] - resp := successResponse{} + resp := successResponse{h: h} // Decode request. var req postFieldRequest @@ -847,7 +864,7 @@ func (h *Handler) handleDeleteField(w http.ResponseWriter, r *http.Request) { indexName := mux.Vars(r)["index"] fieldName := mux.Vars(r)["field"] - resp := successResponse{} + resp := successResponse{h: h} err := h.api.DeleteField(r.Context(), indexName, fieldName) resp.write(w, err) } @@ -863,7 +880,7 @@ func (h *Handler) handleDeleteRemoteAvailableShard(w http.ResponseWriter, r *htt fieldName := mux.Vars(r)["field"] shardID, _ := strconv.ParseUint(mux.Vars(r)["shardID"], 10, 64) - resp := successResponse{} + resp := successResponse{h: h} err := h.api.DeleteAvailableShard(r.Context(), indexName, fieldName, shardID) resp.write(w, err) } @@ -1080,7 +1097,10 @@ func (h *Handler) handlePostImport(w http.ResponseWriter, r *http.Request) { } // Write response. - w.Write(buf) + _, err = w.Write(buf) + if err != nil { + h.logger.Printf("writing import response: %v", err) + } } // handleGetExport handles /export requests. @@ -1179,7 +1199,10 @@ func (h *Handler) handleGetFragmentBlockData(w http.ResponseWriter, r *http.Requ // Write response. w.Header().Set("Content-Type", "application/protobuf") w.Header().Set("Content-Length", strconv.Itoa(len(buf))) - w.Write(buf) + _, err = w.Write(buf) + if err != nil { + h.logger.Printf("writing fragment/block/data response: %v", err) + } } // handleGetFragmentBlocks handles GET /internal/fragment/blocks requests. diff --git a/logger/logger.go b/logger/logger.go index ed5a2dc26..9e895d482 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -15,6 +15,7 @@ package logger import ( + "fmt" "io" "log" ) @@ -82,3 +83,25 @@ func (vb *verboseLogger) Debugf(format string, v ...interface{}) { func (vb *verboseLogger) Logger() *log.Logger { return vb.logger } + +// CaptureLogger is a logger that stores all the print and debug messages +// it sees, useful for testing. +type CaptureLogger struct { + Prints []string + Debugs []string +} + +// NewCaptureLogger yields a CaptureLogger. +func NewCaptureLogger() *CaptureLogger { + return &CaptureLogger{} +} + +// Printf formats a message and appends it to Prints. +func (cl *CaptureLogger) Printf(format string, v ...interface{}) { + cl.Prints = append(cl.Prints, fmt.Sprintf(format, v...)) +} + +// Debugf formats a message and appends it to Debugs. +func (cl *CaptureLogger) Debugf(format string, v ...interface{}) { + cl.Debugs = append(cl.Debugs, fmt.Sprintf(format, v...)) +} diff --git a/lru/lru.go b/lru/lru.go index ba0121a9a..7f2e6dc22 100644 --- a/lru/lru.go +++ b/lru/lru.go @@ -83,7 +83,7 @@ func (c *Cache) Get(key Key) (value interface{}, ok bool) { } // remove removes the provided key from the cache. -func (c *Cache) remove(key Key) { // nolint: staticcheck +func (c *Cache) remove(key Key) { // nolint: staticcheck,unused if c.cache == nil { return } @@ -121,7 +121,7 @@ func (c *Cache) Len() int { } // clear purges all stored items from the cache. -func (c *Cache) clear() { // nolint: staticcheck +func (c *Cache) clear() { // nolint: staticcheck,unused if c.OnEvicted != nil { for _, e := range c.cache { kv := e.Value.(*entry) diff --git a/pql/pql.peg.go b/pql/pql.peg.go index acec51d19..f1ae962fc 100644 --- a/pql/pql.peg.go +++ b/pql/pql.peg.go @@ -15,7 +15,7 @@ const endSymbol rune = 1114112 type pegRule uint8 const ( - ruleUnknown pegRule = iota + ruleUnknown pegRule = iota // nolint:varcheck,deadcode,unused ruleCalls ruleCall ruleallargs diff --git a/roaring/btree.go b/roaring/btree.go index 76427059f..192993c3b 100644 --- a/roaring/btree.go +++ b/roaring/btree.go @@ -870,8 +870,10 @@ func (e *enumerator) Next() (k uint64, v *Container, err error) { i := e.q.d[e.i] k, v = i.k, i.v e.k, e.hit = k, true - e.next() - return k, v, nil + // Any error returned would be stashed in e.err, and would come up + // on the next call. + _ = e.next() + return k, v, err } func (e *enumerator) next() error { @@ -925,7 +927,9 @@ func (e *enumerator) Prev() (k uint64, v *Container, err error) { i := e.q.d[e.i] k, v = i.k, i.v e.k, e.hit = k, true - e.prev() + // Any error returned would be stashed in e.err, and would come up + // on the next call. + _ = e.prev() return k, v, err } diff --git a/roaring/containers_btree.go b/roaring/containers_btree.go index 1d08f598c..5934a1244 100644 --- a/roaring/containers_btree.go +++ b/roaring/containers_btree.go @@ -35,7 +35,8 @@ func NewBTreeBitmap(a ...uint64) *Bitmap { b := &Bitmap{ Containers: newBTreeContainers(), } - b.Add(a...) + // TODO: We have no way to report this. + _, _ = b.Add(a...) return b } diff --git a/roaring/roaring.go b/roaring/roaring.go index a8ccd81e2..78b5f4ca8 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -137,7 +137,10 @@ func NewBitmap(a ...uint64) *Bitmap { b := &Bitmap{ Containers: newSliceContainers(), } - b.AddN(a...) + // TODO: We have no way to report this. We aren't in a server context + // so we haven't got a logger, nothing is checking for nil returns + // from this... + _, _ = b.AddN(a...) return b } @@ -3695,8 +3698,8 @@ func (op *op) WriteTo(w io.Writer) (n int64, err error) { // Add checksum at the end. h := fnv.New32a() - h.Write(buf[0:9]) - h.Write(buf[13:]) + _, _ = h.Write(buf[0:9]) + _, _ = h.Write(buf[13:]) binary.LittleEndian.PutUint32(buf[9:13], h.Sum32()) // Write to writer. @@ -3719,13 +3722,13 @@ func (op *op) UnmarshalBinary(data []byte) error { // Verify checksum. h := fnv.New32a() - h.Write(data[0:9]) + _, _ = h.Write(data[0:9]) if op.typ > 1 { if len(data) < int(13+op.value*8) { return fmt.Errorf("op data truncated - expected %d, got %d", 13+op.value*8, len(data)) } - h.Write(data[13 : 13+op.value*8]) + _, _ = h.Write(data[13 : 13+op.value*8]) op.values = make([]uint64, op.value) for i := uint64(0); i < op.value; i++ { start := 13 + i*8 diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index e88356a37..8c3e7a930 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -2127,10 +2127,14 @@ func TestIteratorBitmap(t *testing.T) { // but won't update to RLE until Optimize() is called b := NewFileBitmap() for i := uint64(61000); i < 71000; i++ { - b.Add(i) + if _, err := b.Add(i); err != nil { + t.Fatalf("adding bit: %v", err) + } } for i := uint64(75000); i < 75100; i++ { - b.Add(i) + if _, err := b.Add(i); err != nil { + t.Fatalf("adding bit: %v", err) + } } if !b.Containers.Get(0).isBitmap() { t.Fatalf("wrong container type") @@ -2393,7 +2397,9 @@ func TestRunBinSearch(t *testing.T) { } func TestBitmap_RemoveEmptyContainers(t *testing.T) { bm1 := NewFileBitmap(1<<16, 2<<16, 3<<16) - bm1.Remove(2 << 16) + if _, err := bm1.Remove(2 << 16); err != nil { + t.Fatalf("removing a bit: %v", err) + } if bm1.countEmptyContainers() != 1 { t.Fatalf("Should be 1 empty container ") } @@ -2406,13 +2412,17 @@ func TestBitmap_RemoveEmptyContainers(t *testing.T) { func TestBitmap_BitmapWriteToWithEmpty(t *testing.T) { bm1 := NewFileBitmap(1<<16, 2<<16, 3<<16) - bm1.Remove(2 << 16) + if _, err := bm1.Remove(2 << 16); err != nil { + t.Fatalf("removing a bit: %v", err) + } var buf bytes.Buffer if _, err := bm1.WriteTo(&buf); err != nil { t.Fatalf("Failure to write to bitmap buffer. ") } bm0 := NewFileBitmap() - bm0.UnmarshalBinary(buf.Bytes()) + if err := bm0.UnmarshalBinary(buf.Bytes()); err != nil { + t.Fatalf("unmarshalling: %v", err) + } if bm0.countEmptyContainers() != 0 { t.Fatalf("Should be no empty containers ") } @@ -2559,7 +2569,9 @@ func TestIntersectArrayBitmap(t *testing.T) { func TestBitmapClone(t *testing.T) { b := NewFileBitmap() for i := uint64(61000); i < 71000; i++ { - b.Add(i) + if _, err := b.Add(i); err != nil { + t.Fatalf("adding bit: %v", err) + } } c := b.Clone() if err := bitmapsEqual(b, c); err != nil { @@ -3770,24 +3782,45 @@ func TestBitmapAny(t *testing.T) { if bm.Any() { t.Error("empty bitmap should have Any()==false") } - bm.Add(1) + _, err := bm.Add(1) + if err != nil { + t.Errorf("couldn't add a bit: %v", err) + } if !bm.Any() { t.Error("bitmap with 1 bit should have Any()==true") } - bm.Add(100000) + _, err = bm.Add(100000) + if err != nil { + t.Errorf("couldn't add a bit: %v", err) + } if !bm.Any() { t.Error("bitmap with 2 bits should have Any()==true") } - bm.Remove(1) + changed, err := bm.Remove(1) + if err != nil { + t.Errorf("couldn't remove a bit: %v", err) + } + if changed != true { + t.Error("removing a set bit should have been a change") + } if !bm.Any() { t.Error("bitmap with 1 bit left after removing 1 should have Any()==true") } - bm.Add(1) + _, err = bm.Add(1) + if err != nil { + t.Errorf("couldn't remove a bit: %v", err) + } + if changed != true { + t.Error("re-addintg a previously set bit should have been a change") + } bm = bm.Difference(NewBTreeBitmap(1)) if !bm.Any() { t.Error("bitmap with 1 bit left after differencing 1 should have Any()==true") } - bm.Remove(100000) + _, err = bm.Remove(100000) + if err != nil { + t.Errorf("couldn't remove a bit: %v", err) + } if bm.Any() { t.Error("shouldn't be any left") } diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index ebf88a212..b4f792629 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -164,11 +164,15 @@ func TestCheckBitmap(t *testing.T) { x := 0 for i := uint64(61000); i < 71000; i++ { x++ - b.Add(i) + if _, err := b.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } } for i := uint64(75000); i < 75100; i++ { x++ - b.Add(i) + if _, err := b.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } } err := b.Check() if err != nil { @@ -198,7 +202,7 @@ func TestCheckFullRun(t *testing.T) { if i%16384 == 0 { b.Optimize() // convert to runs } - b.Add(i) + _, _ = b.Add(i) } err := b.Check() if err != nil { @@ -238,7 +242,13 @@ func TestBitmap_Contains_Empty(t *testing.T) { // Ensure an empty bitmap does nothing when removing an element. func TestBitmap_Remove_Empty(t *testing.T) { - roaring.NewFileBitmap().Remove(1000) + changed, err := roaring.NewFileBitmap().Remove(1000) + if err != nil { + t.Fatalf("got an error removing a bit from an empty bitmap: %v", err) + } + if changed != false { + t.Fatalf("change reported removing a bit from an empty bitmap") + } } // Ensure a bitmap can return a slice of values. @@ -289,7 +299,9 @@ func TestBitmap_ForEachRange(t *testing.T) { func TestBitmap_Max(t *testing.T) { bm := roaring.NewFileBitmap() for i := uint64(1000); i <= 100000; i++ { - bm.Add(i) + if _, err := bm.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } if v := bm.Max(); v != i { t.Fatalf("max: got=%d; want=%d", v, i) @@ -310,7 +322,9 @@ func TestBitmap_BitmapCountRangeEdgeCase(t *testing.T) { } else { start += 2 } - bm0.Add(start) + if _, err := bm0.Add(start); err != nil { + t.Fatalf("adding bit: %v", err) + } } a := bm0.Count() r := bm0.CountRange(s, e) @@ -323,9 +337,13 @@ func TestBitmap_BitmapCountRangeEdgeCase(t *testing.T) { func TestBitmap_BitmapCountRange(t *testing.T) { bm0 := roaring.NewFileBitmap(0, 2683177) for i := uint64(628); i < 2683301; i++ { - bm0.Add(i) + if _, err := bm0.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } + } + if _, err := bm0.Add(2683307); err != nil { + t.Fatalf("adding bits: %v", err) } - bm0.Add(2683307) if n := bm0.CountRange(1, 2683311); n != 2682674 { t.Fatalf("unexpected n: %d", n) } @@ -389,7 +407,9 @@ func TestBitmap_Intersection(t *testing.T) { bm0 := roaring.NewFileBitmap(0, 2683177) bm1 := roaring.NewFileBitmap() for i := uint64(628); i < 2683301; i++ { - bm1.Add(i) + if _, err := bm1.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } } result := bm0.Intersect(bm1) @@ -403,9 +423,13 @@ func TestBitmap_Union1(t *testing.T) { bm0 := roaring.NewFileBitmap(0, 2683177) bm1 := roaring.NewFileBitmap() for i := uint64(628); i < 2683301; i++ { - bm1.Add(i) + if _, err := bm1.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } + } + if _, err := bm1.Add(4000000); err != nil { + t.Fatalf("adding bits: %v", err) } - bm1.Add(4000000) result := bm0.Union(bm1) if n := result.Count(); n != 2682675 { @@ -429,9 +453,13 @@ func TestBitmap_UnionInPlace1(t *testing.T) { result = roaring.NewBitmap() ) for i := uint64(628); i < 2683301; i++ { - bm1.Add(i) + if _, err := bm1.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } + } + if _, err := bm1.Add(4000000); err != nil { + t.Fatalf("adding bits: %v", err) } - bm1.Add(4000000) result.UnionInPlace(bm0, bm1) if n := result.Count(); n != 2682675 { @@ -504,7 +532,9 @@ func TestBitmap_UnionInPlaceProp(t *testing.T) { // size of a container to ensure we generate a maxRange container. for x := start; x < (start + 2*(0xffff+1)); x++ { set[uint64(x)] = struct{}{} - bitmap.Add(uint64(x)) + if _, err := bitmap.Add(uint64(x)); err != nil { + t.Fatalf("adding bits: %v", err) + } } } @@ -513,7 +543,9 @@ func TestBitmap_UnionInPlaceProp(t *testing.T) { for x := 0; x < numIntsPerBatch; x++ { num := uint64(rng.Intn(maxUint64Val)) set[num] = struct{}{} - bitmap.Add(num) + if _, err := bitmap.Add(num); err != nil { + t.Fatalf("adding bits: %v", err) + } } sets = append(sets, set) @@ -588,12 +620,16 @@ func TestBitmap_IntersectArrayArray(t *testing.T) { func TestBitmap_IntersectBitmapBitmap(t *testing.T) { bm0 := roaring.NewFileBitmap() for i := uint64(0); i < 65536; i += 2 { - bm0.Add(i) + if _, err := bm0.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } } bm1 := roaring.NewFileBitmap() for i := uint64(0); i < 65536; i += 3 { - bm1.Add(i) + if _, err := bm1.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } } result := bm0.Intersect(bm1) @@ -620,7 +656,9 @@ func TestBitmap_IntersectRunRun(t *testing.T) { offset := (runLen / 2) + spaceLen for i := uint64(0); i < (65536 - runLen - offset); i += (runLen + spaceLen) { for j := uint64(0); j < runLen; j++ { - bm2.Add(offset + i + j) + if _, err := bm2.Add(offset + i + j); err != nil { + t.Fatalf("adding bits: %v", err) + } } } bm2.Optimize() // convert to runs @@ -629,7 +667,9 @@ func TestBitmap_IntersectRunRun(t *testing.T) { spaceLen = uint64(1) for i := uint64(0); i < (65536 - runLen); i += (runLen + spaceLen) { for j := uint64(0); j < runLen; j++ { - bm3.Add(i + j) + if _, err := bm3.Add(i + j); err != nil { + t.Fatalf("adding bits: %v", err) + } } } bm3.Optimize() // convert to runs @@ -643,7 +683,7 @@ func TestBitmap_Difference(t *testing.T) { bm0 := roaring.NewFileBitmap(0, 2683177) bm1 := roaring.NewFileBitmap() for i := uint64(628); i < 2683301; i++ { - bm1.Add(i) + _, _ = bm1.Add(i) } result := bm0.Difference(bm1) if n := result.Count(); n != 1 { @@ -771,7 +811,7 @@ func TestBitmap_Xor_ArrayBitmap(t *testing.T) { bm0 := roaring.NewFileBitmap(1, 70, 200, 4097, 4098) bm1 := roaring.NewFileBitmap() for i := uint64(0); i < 10000; i += 2 { - bm1.Add(i) + _, _ = bm1.Add(i) } result := bm0.Xor(bm1) @@ -802,11 +842,11 @@ func TestBitmap_Xor_BitmapBitmap(t *testing.T) { bm1 := roaring.NewFileBitmap() for i := uint64(0); i < 10000; i += 2 { - bm1.Add(i) + _, _ = bm1.Add(i) } for i := uint64(1); i < 10000; i += 2 { - bm0.Add(i) + _, _ = bm0.Add(i) } result := bm0.Xor(bm1) @@ -847,7 +887,9 @@ func TestBitmap_Flip_Bitmap(t *testing.T) { bm := roaring.NewFileBitmap() size := uint64(10000) for i := uint64(0); i < size; i += 2 { - bm.Add(i) + if _, err := bm.Add(i); err != nil { + t.Fatalf("adding bits: %v", err) + } } results := bm.Flip(0, size-1) if n := results.Count(); n != size/2 { @@ -921,7 +963,7 @@ func TestBitmap_IntersectionCount_RunRun(t *testing.T) { func TestBitmap_IntersectionCount_BitmapRun(t *testing.T) { bm0 := roaring.NewFileBitmap() for i := uint64(3); i <= 1000006; i += 2 { - bm0.Add(i) + _, _ = bm0.Add(i) } bm1 := roaring.NewFileBitmap(0, 1, 2, 3, 4, 5, 1000000, 1000002, 1000003, 1000004, 1000005, 1000006) bm1.Optimize() // convert to runs @@ -938,7 +980,7 @@ func TestBitmap_IntersectionCount_ArrayBitmap(t *testing.T) { bm0 := roaring.NewFileBitmap(1, 70, 200, 4097, 4098) bm1 := roaring.NewFileBitmap() for i := uint64(0); i <= 10000; i += 2 { - bm1.Add(i) + _, _ = bm1.Add(i) } if n := bm0.IntersectionCount(bm1); n != 3 { @@ -953,15 +995,15 @@ func TestBitmap_IntersectionCount_BitmapBitmap(t *testing.T) { bm0 := roaring.NewFileBitmap() bm1 := roaring.NewFileBitmap() for i := uint64(0); i <= 10000; i += 2 { - bm0.Add(i) - bm1.Add(i + 1) + _, _ = bm0.Add(i) + _, _ = bm1.Add(i + 1) } - bm0.Add(1000) - bm1.Add(1000) + _, _ = bm0.Add(1000) + _, _ = bm1.Add(1000) - bm0.Add(2000) - bm1.Add(2000) + _, _ = bm0.Add(2000) + _, _ = bm1.Add(2000) if n := bm0.IntersectionCount(bm1); n != 2 { t.Fatalf("unexpected n: %d", n) @@ -1005,7 +1047,7 @@ func TestBitmap_Quick_LargeValue(t *testing.T) { testBitmapQuick(t, 10000, 0, ma // Ensure a bitmap can perform basic operations on randomly generated values. func testBitmapQuick(t *testing.T, n int, min, max uint64) { - quick.Check(func(a []uint64) bool { + err := quick.Check(func(a []uint64) bool { bm := roaring.NewFileBitmap() m := make(map[uint64]struct{}) @@ -1067,6 +1109,9 @@ func testBitmapQuick(t *testing.T, n int, min, max uint64) { values[0] = reflect.ValueOf(GenerateUint64Slice(n, min, max, false, rand)) }, }) + if err != nil { + t.Fatalf("quick check failed: %v", err) + } } func TestBitmap_Marshal_Quick_Array1(t *testing.T) { testBitmapMarshalQuick(t, 1000, 1000, 2000, false) } @@ -1091,7 +1136,7 @@ func testBitmapMarshalQuick(t *testing.T, n int, min, max uint64, sorted bool) { t.Skip("short") } - quick.Check(func(a0, a1 []uint64) bool { + err := quick.Check(func(a0, a1 []uint64) bool { // Create bitmap with initial values set. bm := roaring.NewFileBitmap(a0...) @@ -1145,6 +1190,9 @@ func testBitmapMarshalQuick(t *testing.T, n int, min, max uint64, sorted bool) { values[1] = reflect.ValueOf(GenerateUint64Slice(100, min, max, sorted, rand)) }, }) + if err != nil { + t.Fatalf("quick check failed: %v", err) + } } // Ensure iterator can iterate over all the values on the bitmap. @@ -1167,13 +1215,13 @@ func TestIterator(t *testing.T) { t.Run("run", func(t *testing.T) { bm1 := roaring.NewFileBitmap() for i := uint64(0); i < 11; i += 1 { - bm1.Add(i) + _, _ = bm1.Add(i) } bm1.Optimize() bm2 := roaring.NewFileBitmap() for i := uint64(0); i < 12; i += 1 { - bm2.Add(i) + _, _ = bm2.Add(i) } bm2.Optimize() @@ -1203,23 +1251,24 @@ func TestIterator(t *testing.T) { // testBM creates a bitmap with 3 containers: array, bitmap, and run. func testBM() *roaring.Bitmap { - + // We should possibly be testing the adds for errors, but we + // don't have a clean way to return an error, so we don't right now. bm := roaring.NewFileBitmap() //the array for i := uint64(0); i < 1024; i += 4 { - bm.Add((1 << 16) + i) + _, _ = bm.Add((1 << 16) + i) } //the bitmap for i := uint64(0); i < 16384; i += 2 { - bm.Add((2 << 16) + i) + _, _ = bm.Add((2 << 16) + i) } //small run for i := uint64(0); i < 1024; i += 1 { - bm.Add((3 << 16) + i) + _, _ = bm.Add((3 << 16) + i) } //large run for i := uint64(0); i < 65535; i += 1 { - bm.Add((4 << 16) + i) + _, _ = bm.Add((4 << 16) + i) } bm.Optimize() //count 75007 @@ -1275,9 +1324,13 @@ func isAllType(b *roaring.Bitmap, typ string) bool { return true } +// getBenchData yields some sample data func getBenchData(tb testing.TB) *benchmarkSampleData { data := &sampleData if data.a1 == nil { + // throughout this, we ignore any errors from bitmap adds, + // because errors in those should result in the Optimize + // pass producing the wrong values, so we can just check there. const max = (1 << 24) / 64 // Build bitmap with array container. @@ -1285,28 +1338,28 @@ func getBenchData(tb testing.TB) *benchmarkSampleData { data.a2 = roaring.NewFileBitmap() // two lists of different lengths for i, n := 0, roaring.ArrayMaxSize/3; i < n; i++ { - data.a1.Add(uint64(rand.Intn(max))) - data.a2.Add(uint64(rand.Intn(max))) + _, _ = data.a1.Add(uint64(rand.Intn(max))) + _, _ = data.a2.Add(uint64(rand.Intn(max))) } for i, n := 0, roaring.ArrayMaxSize/3; i < n; i++ { - data.a1.Add(uint64(rand.Intn(max))) + _, _ = data.a1.Add(uint64(rand.Intn(max))) } // Build bitmap with bitmap container. data.b = roaring.NewFileBitmap() for i, n := 0, MaxContainerVal/3; i < n; i++ { - data.b.Add(uint64(i * 3)) + _, _ = data.b.Add(uint64(i * 3)) } // build bitmap with run container data.r1 = roaring.NewFileBitmap() for i, n := 0, MaxContainerVal; i < n; i++ { - data.r1.Add(uint64(i)) + _, _ = data.r1.Add(uint64(i)) } // build bitmap with multiple runs data.r2 = roaring.NewFileBitmap() for i, n := 0, MaxContainerVal; i < n; i++ { - data.r2.Add(uint64(i)) + _, _ = data.r2.Add(uint64(i)) // break the runs up, this should produce 16 runs, which // is small enough to make RLE tempting if i&0xfff == 0xfff { @@ -1467,7 +1520,7 @@ func BenchmarkContainerLinear(b *testing.B) { bm := bmMaker() for row := uint64(1); row < NumRows; row++ { for col := uint64(1); col < NumColums; col++ { - bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) } } } @@ -1483,7 +1536,7 @@ func BenchmarkContainerReverse(b *testing.B) { bm := bmMaker() for row := NumRows - 1; row >= 1; row-- { for col := NumColums - 1; col >= 1; col-- { - bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) } } } @@ -1498,7 +1551,7 @@ func BenchmarkContainerColumn(b *testing.B) { bm := bmMaker() for col := uint64(1); col < NumColums; col++ { for row := uint64(1); row < NumRows; row++ { - bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) } } } @@ -1514,8 +1567,8 @@ func BenchmarkContainerOutsideIn(b *testing.B) { bm := bmMaker() for col := uint64(1); col < NumColums; col++ { for row := uint64(1); row < middle; row++ { - bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) - bm.Add((NumRows-row)*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add(row*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add((NumRows-row)*pilosa.ShardWidth + (col * MaxContainerVal)) } } } @@ -1532,8 +1585,8 @@ func BenchmarkContainerInsideOut(b *testing.B) { bm := bmMaker() for col := uint64(1); col < NumColums; col++ { for row := uint64(1); row <= middle; row++ { - bm.Add((middle+row)*pilosa.ShardWidth + (col * MaxContainerVal)) - bm.Add((middle-row)*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add((middle+row)*pilosa.ShardWidth + (col * MaxContainerVal)) + _, _ = bm.Add((middle-row)*pilosa.ShardWidth + (col * MaxContainerVal)) } } } @@ -1545,7 +1598,7 @@ func BenchmarkSliceAscending(b *testing.B) { for n := 0; n < b.N; n++ { bm := roaring.NewFileBitmap() for col := uint64(0); col < pilosa.ShardWidth; col++ { - bm.Add(col) + _, _ = bm.Add(col) } } } @@ -1554,9 +1607,9 @@ func BenchmarkSliceDescending(b *testing.B) { for n := 0; n < b.N; n++ { bm := roaring.NewFileBitmap() for col := uint64(pilosa.ShardWidth); col > uint64(0); col-- { - bm.Add(col) + _, _ = bm.Add(col) } - bm.Add(0) + _, _ = bm.Add(0) } } @@ -1565,14 +1618,14 @@ func BenchmarkSliceAscendingStriped(b *testing.B) { bm := roaring.NewFileBitmap() l := uint64(pilosa.ShardWidth / 8) for col := uint64(0); col < l; col++ { - bm.Add(l*0 + col) - bm.Add(l*1 + col) - bm.Add(l*2 + col) - bm.Add(l*3 + col) - bm.Add(l*4 + col) - bm.Add(l*5 + col) - bm.Add(l*6 + col) - bm.Add(l*7 + col) + _, _ = bm.Add(l*0 + col) + _, _ = bm.Add(l*1 + col) + _, _ = bm.Add(l*2 + col) + _, _ = bm.Add(l*3 + col) + _, _ = bm.Add(l*4 + col) + _, _ = bm.Add(l*5 + col) + _, _ = bm.Add(l*6 + col) + _, _ = bm.Add(l*7 + col) } } } @@ -1582,14 +1635,14 @@ func BenchmarkSliceDescendingStriped(b *testing.B) { bm := roaring.NewFileBitmap() l := uint64(pilosa.ShardWidth / 8) for col := uint64(l); col < l+1; col-- { - bm.Add(l*7 + col) - bm.Add(l*6 + col) - bm.Add(l*5 + col) - bm.Add(l*4 + col) - bm.Add(l*3 + col) - bm.Add(l*2 + col) - bm.Add(l*1 + col) - bm.Add(l*0 + col) + _, _ = bm.Add(l*7 + col) + _, _ = bm.Add(l*6 + col) + _, _ = bm.Add(l*5 + col) + _, _ = bm.Add(l*4 + col) + _, _ = bm.Add(l*3 + col) + _, _ = bm.Add(l*2 + col) + _, _ = bm.Add(l*1 + col) + _, _ = bm.Add(l*0 + col) } } } diff --git a/server.go b/server.go index 3e989e530..a4d05b144 100644 --- a/server.go +++ b/server.go @@ -559,7 +559,7 @@ func (s *Server) receiveMessage(m Message) error { return err } case *SetCoordinatorMessage: - s.cluster.setCoordinator(obj.New) + return s.cluster.setCoordinator(obj.New) case *UpdateCoordinatorMessage: s.cluster.updateCoordinator(obj.New) case *NodeStateMessage: @@ -705,7 +705,10 @@ func (s *Server) monitorDiagnostics() { s.diagnostics.Set("GoRoutines", runtime.NumGoroutine()) s.diagnostics.EnrichWithMemoryInfo() s.diagnostics.EnrichWithSchemaProperties() - s.diagnostics.CheckVersion() + err = s.diagnostics.CheckVersion() + if err != nil { + s.logger.Printf("can't check version: %v", err) + } err = s.diagnostics.Flush() if err != nil { s.logger.Printf("diagnostics error: %s", err) diff --git a/server/handler_test.go b/server/handler_test.go index ff9654933..082869081 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -846,7 +846,10 @@ func TestClusterTranslator(t *testing.T) { cluster := make(test.Cluster, 2) cluster[0] = test.NewCommandNode(true) cluster[0].Config.Gossip.Port = "0" - cluster[0].Start() + err := cluster[0].Start() + if err != nil { + t.Fatalf("starting cluster 1: %v", err) + } httpTranslateStore := http.NewTranslateStore(cluster[0].URL()) cluster[1] = test.NewCommandNode(false, server.OptCommandServerOptions( @@ -855,7 +858,10 @@ func TestClusterTranslator(t *testing.T) { ) cluster[1].Config.Gossip.Port = "0" cluster[1].Config.Gossip.Seeds = []string{cluster[0].GossipAddress()} - cluster[1].Start() + err = cluster[1].Start() + if err != nil { + t.Fatalf("starting cluster 1: %v", err) + } test.MustDo("POST", cluster[0].URL()+"/index/i0", "{\"options\": {\"keys\": true}}") test.MustDo("POST", cluster[0].URL()+"/index/i0/field/f0", "{\"options\": {\"keys\": true}}") diff --git a/server/server_test.go b/server/server_test.go index 1cc3bd87f..19b79c6ea 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -900,7 +900,7 @@ func TestClusterExhaustingConnections(t *testing.T) { return nil }) } - err := eg.Wait() + err = eg.Wait() if err != nil { t.Fatalf("setting lots of shards: %v", err) } @@ -929,7 +929,10 @@ func TestClusterExhaustingConnectionsImport(t *testing.T) { bm := roaring.NewBitmap() bm.DirectAdd(0) buf := &bytes.Buffer{} - bm.WriteTo(buf) + _, err := bm.WriteTo(buf) + if err != nil { + t.Fatalf("writing to buffer: %v", err) + } data := buf.Bytes() eg := errgroup.Group{} diff --git a/utils_internal_test.go b/utils_internal_test.go index 321f4846d..3396cb4e9 100644 --- a/utils_internal_test.go +++ b/utils_internal_test.go @@ -57,15 +57,15 @@ func NewTestCluster(n int) *cluster { // NewTestURI is a test URI creator that intentionally swallows errors. func NewTestURI(scheme, host string, port uint16) URI { uri := defaultURI() - uri.setScheme(scheme) - uri.setHost(host) + _ = uri.setScheme(scheme) + _ = uri.setHost(host) uri.SetPort(port) return *uri } func NewTestURIFromHostPort(host string, port uint16) URI { uri := defaultURI() - uri.setHost(host) + _ = uri.setHost(host) uri.SetPort(port) return *uri } diff --git a/view.go b/view.go index 5780e682c..b1e5dee78 100644 --- a/view.go +++ b/view.go @@ -173,7 +173,7 @@ func (v *view) availableShards() *roaring.Bitmap { b := roaring.NewBitmap() for shard := range v.fragments { - b.Add(shard) // ignore error, no writer attached + _, _ = b.Add(shard) // ignore error, no writer attached } return b }