From a26ebe3e23cecf92c531e57363a770dc1cd2bc0c Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 17 Jul 2018 14:30:03 -0500 Subject: [PATCH 1/8] frag.rows() and frag.rowsForColumn() from #1496 --- fragment.go | 57 ++++++++++++++++++++++++++++++++++++++- fragment_internal_test.go | 28 +++++++++++++++++++ roaring/roaring.go | 6 ++--- 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/fragment.go b/fragment.go index 835c1abe8..697f11f92 100644 --- a/fragment.go +++ b/fragment.go @@ -153,7 +153,6 @@ func (f *fragment) Open() error { pos := f.storage.Max() f.maxRowID = pos / ShardWidth f.stats.Gauge("rows", float64(f.maxRowID), 1.0) - return nil }(); err != nil { f.close() @@ -1680,6 +1679,62 @@ func (f *fragment) readCacheFromArchive(r io.Reader) error { return nil } +func (f *fragment) rows() []uint64 { + i, _ := f.storage.Containers.Iterator(0) + rows := make([]uint64, 0) + + var lastRow uint64 + lastRow = math.MaxUint64 + + // Loop over the existing containers. + for i.Next() { + key, _ := i.Value() + + // virtual row for the current container + vRow := key >> 4 + + // skip dups + if vRow == lastRow { + continue + } + + rows = append(rows, vRow) + lastRow = vRow + } + return rows + +} + +func (f *fragment) rowsForColumn(columnID uint64) []uint64 { + colID := columnID % ShardWidth + i, _ := f.storage.Containers.Iterator(0) + + colKey := uint64(0) + colVal := uint16(colID & 0xFFFF) + + rows := make([]uint64, 0) + + // Loop over the existing containers. + for i.Next() { + key, c := i.Value() + + // virtual row for the current container + vRow := key >> 4 + + // column container key for virtual row + colKey = ((vRow * ShardWidth) + colID) >> 16 + + if colKey != key { + continue + } + + if c.Contains(colVal) { + rows = append(rows, vRow) + } + } + return rows +} + // FragmentBlock represents info about a subsection of the rows in a block. // This is used for comparing data in remote blocks for active anti-entropy. type FragmentBlock struct { diff --git a/fragment_internal_test.go b/fragment_internal_test.go index e665a10ba..f487ba40c 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -1278,3 +1278,31 @@ func (f *fragment) mustSetBits(rowID uint64, columnIDs ...uint64) { } } } + +// Test Various methods of retrieving RowIDs +func TestFragment_RowsIteration(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Close() + expected1 := make([]uint64, 0) + expected2 := make([]uint64, 0) + for i := uint64(100); i < uint64(200); i++ { + if _, err := f.setBit(i, i%2); err != nil { + t.Fatal(err) + } + expected1 = append(expected1, i) + if i%2 == 1 { + expected2 = append(expected2, i) + } + } + + ids := f.rows() + if !reflect.DeepEqual(expected1, ids) { + t.Fatalf("Do not match %v %v", expected1, ids) + + } + + ids = f.rowsForColumn(1) + if !reflect.DeepEqual(expected2, ids) { + t.Fatalf("Do not match %v %v", expected2, ids) + } +} diff --git a/roaring/roaring.go b/roaring/roaring.go index 3756d1db7..3a0418aa3 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -177,7 +177,7 @@ func (b *Bitmap) Contains(v uint64) bool { if c == nil { return false } - return c.contains(lowbits(v)) + return c.Contains(lowbits(v)) } // Remove removes values from the bitmap. @@ -1272,8 +1272,8 @@ func (c *Container) runAdd(v uint16) bool { return true } -// contains returns true if v is in the container. -func (c *Container) contains(v uint16) bool { +// Contains returns true if v is in the container. +func (c *Container) Contains(v uint16) bool { if c.isArray() { return c.arrayContains(v) } else if c.isRun() { From 07abb505cc622cbdf40a628998343767820d234a Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 17 Jul 2018 15:06:51 -0500 Subject: [PATCH 2/8] add more testing to row iteration --- fragment_internal_test.go | 86 +++++++++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 18 deletions(-) diff --git a/fragment_internal_test.go b/fragment_internal_test.go index f487ba40c..0d4e9fa49 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -1281,28 +1281,78 @@ func (f *fragment) mustSetBits(rowID uint64, columnIDs ...uint64) { // Test Various methods of retrieving RowIDs func TestFragment_RowsIteration(t *testing.T) { - f := mustOpenFragment("i", "f", viewStandard, 0, "") - defer f.Close() - expected1 := make([]uint64, 0) - expected2 := make([]uint64, 0) - for i := uint64(100); i < uint64(200); i++ { - if _, err := f.setBit(i, i%2); err != nil { + t.Run("firstContainer", func(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Close() + + expectedAll := make([]uint64, 0) + expectedOdd := make([]uint64, 0) + for i := uint64(100); i < uint64(200); i++ { + if _, err := f.setBit(i, i%2); err != nil { + t.Fatal(err) + } + expectedAll = append(expectedAll, i) + if i%2 == 1 { + expectedOdd = append(expectedOdd, i) + } + } + + ids := f.rows() + if !reflect.DeepEqual(expectedAll, ids) { + t.Fatalf("Do not match %v %v", expectedAll, ids) + } + + ids = f.rowsForColumn(1) + if !reflect.DeepEqual(expectedOdd, ids) { + t.Fatalf("Do not match %v %v", expectedOdd, ids) + } + }) + + t.Run("secondRow", func(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Close() + + expected := []uint64{1, 2} + if _, err := f.setBit(1, 66000); err != nil { + t.Fatal(err) + } else if _, err := f.setBit(2, 66000); err != nil { + t.Fatal(err) + } else if _, err := f.setBit(2, 166000); err != nil { t.Fatal(err) } - expected1 = append(expected1, i) - if i%2 == 1 { - expected2 = append(expected2, i) + + ids := f.rows() + if !reflect.DeepEqual(expected, ids) { + t.Fatalf("Do not match %v %v", expected, ids) } - } - ids := f.rows() - if !reflect.DeepEqual(expected1, ids) { - t.Fatalf("Do not match %v %v", expected1, ids) + ids = f.rowsForColumn(66000) + if !reflect.DeepEqual(expected, ids) { + t.Fatalf("Do not match %v %v", expected, ids) + } + }) - } + t.Run("combinations", func(t *testing.T) { + f := mustOpenFragment("i", "f", viewStandard, 0, "") + defer f.Close() - ids = f.rowsForColumn(1) - if !reflect.DeepEqual(expected2, ids) { - t.Fatalf("Do not match %v %v", expected2, ids) - } + expectedRows := make([]uint64, 0) + for r := uint64(1); r < uint64(10000); r += 100 { + expectedRows = append(expectedRows, r) + for c := uint64(1); c < uint64(ShardWidth-1); c += 10000 { + if _, err := f.setBit(r, c); err != nil { + t.Fatal(err) + } + + ids := f.rows() + if !reflect.DeepEqual(expectedRows, ids) { + t.Fatalf("Do not match %v %v", expectedRows, ids) + } + ids = f.rowsForColumn(c) + if !reflect.DeepEqual(expectedRows, ids) { + t.Fatalf("Do not match %v %v", expectedRows, ids) + } + } + } + }) } From dc5d7a66ef433f446105d0b802e7a6aad6b738aa Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 17 Jul 2018 15:56:48 -0500 Subject: [PATCH 3/8] ensure that changing ShardWidth is supported in rowsForColumn() --- fragment.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/fragment.go b/fragment.go index 697f11f92..371b4ef46 100644 --- a/fragment.go +++ b/fragment.go @@ -47,6 +47,13 @@ const ( // ShardWidth is the number of column IDs in a shard. ShardWidth = 1048576 + // containersPerRowSegment is dependent upon ShardWidth, + // and it represents the number of containers per shard row + // (or rowSegment). Since containers are set in roaring + // to be 2^16, then this const should be ShardWidth / 2^16. + // It is represented as the exponent n of 2^n. + containersPerRowSegment = 4 + // snapshotExt is the file extension used for an in-process snapshot. snapshotExt = ".snapshotting" @@ -1691,7 +1698,7 @@ func (f *fragment) rows() []uint64 { key, _ := i.Value() // virtual row for the current container - vRow := key >> 4 + vRow := key >> containersPerRowSegment // skip dups if vRow == lastRow { @@ -1719,7 +1726,7 @@ func (f *fragment) rowsForColumn(columnID uint64) []uint64 { key, c := i.Value() // virtual row for the current container - vRow := key >> 4 + vRow := key >> containersPerRowSegment // column container key for virtual row colKey = ((vRow * ShardWidth) + colID) >> 16 From a962a0c5262e581e517ab6eaeb1c850025e0af0a Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Tue, 17 Jul 2018 16:33:47 -0500 Subject: [PATCH 4/8] Fix linter issues: deadcode --- .travis.yml | 6 ++---- Makefile | 27 ++++++++++++++++++++++++--- attr.go | 6 ------ cluster.go | 3 --- cluster_internal_test.go | 3 ++- executor.go | 9 --------- fragment_internal_test.go | 4 +++- http/handler.go | 13 +------------ roaring/roaring.go | 36 ++---------------------------------- view_internal_test.go | 4 +++- 10 files changed, 37 insertions(+), 74 deletions(-) diff --git a/.travis.yml b/.travis.yml index 382a3d65a..6c77fe0fc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,10 +21,8 @@ jobs: include: - stage: metalinter install: - - make -B install-dep vendor - - go get -u github.com/alecthomas/gometalinter - - gometalinter --install - script: make metalinter + - make -B install-dep vendor install-gometalinter + script: make gometalinter env: - GOARCH=amd64 - stage: deploy diff --git a/Makefile b/Makefile index 7a1ed5db7..65df8f894 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build check-clean clean cover cover-viz default docker docker-build docker-test generate generate-protoc generate-pql install install-build-deps install-dep install-protoc install-protoc-gen-gofast install-peg metalinter prerelease prerelease-upload release release-build require-dep require-protoc require-protoc-gen-gofast require-peg test +.PHONY: build check-clean clean cover cover-viz default docker docker-build docker-test generate generate-protoc generate-pql gometalinter install install-build-deps install-dep install-gometalinter install-protoc install-protoc-gen-gofast install-peg prerelease prerelease-upload release release-build require-dep require-gometalinter require-protoc require-protoc-gen-gofast require-peg test CLONE_URL=github.com/pilosa/pilosa VERSION := $(shell git describe --tags 2> /dev/null || echo unknown) @@ -108,8 +108,21 @@ docker-build: docker-test: docker run --rm -v $(PWD):/go/src/$(CLONE_URL) -w /go/src/$(CLONE_URL) golang:$(GO_VERSION) go test -tags='$(BUILD_TAGS)' $(TESTFLAGS) ./... -metalinter: - gometalinter --vendor --disable-all --enable=gotype --enable=gotypex --enable=gofmt --enable=goimports --enable=interfacer --enable=misspell --enable=unparam --deadline=60s --exclude "^internal/.*\.pb\.go" ./... +# Run gometalinter with custom flags +gometalinter: require-gometalinter + gometalinter --vendor --disable-all \ + --deadline=60s \ + --enable=deadcode \ + --enable=gofmt \ + --enable=goimports \ + --enable=gotype \ + --enable=gotypex \ + --enable=interfacer \ + --enable=misspell \ + --enable=unparam \ + --exclude "^internal/.*\.pb\.go" \ + --exclude "^pql/pql.peg.go" \ + ./... ###################### # Build dependencies # @@ -134,6 +147,9 @@ require-protoc: require-peg: $(call require,peg) +require-gometalinter: + $(call require,gometalinter) + install-build-deps: install-dep install-protoc-gen-gofast install-protoc install-stringer install-peg install-dep: @@ -150,3 +166,8 @@ install-protoc: install-peg: go get github.com/pointlander/peg + +install-gometalinter: + go get -u github.com/alecthomas/gometalinter + gometalinter --install + go get github.com/remyoudompheng/go-misc/deadcode diff --git a/attr.go b/attr.go index b6096a994..629cbf587 100644 --- a/attr.go +++ b/attr.go @@ -204,12 +204,6 @@ func DecodeAttrs(v []byte) (map[string]interface{}, error) { return decodeAttrs(pb.GetAttrs()), nil } -func newMemAttrStore() AttrStore { - return &memAttrStore{ - store: make(map[uint64]map[string]interface{}), - } -} - // memAttrStore represents an in-memory implementation of the AttrStore interface. type memAttrStore struct { store map[uint64]map[string]interface{} diff --git a/cluster.go b/cluster.go index 6699a5e56..01f80c029 100644 --- a/cluster.go +++ b/cluster.go @@ -792,9 +792,6 @@ type Hasher interface { Hash(key uint64, n int) int } -// newHasher returns a new instance of the default hasher. -func newHasher() Hasher { return &jmphasher{} } - // jmphasher represents an implementation of jmphash. Implements Hasher. type jmphasher struct{} diff --git a/cluster_internal_test.go b/cluster_internal_test.go index 58b40ccbf..07a6521d4 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -372,7 +372,8 @@ func TestHasher(t *testing.T) { {0x0ddc0ffeebadf00d, []int{0, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 15, 15, 15, 15}}, } { for i, v := range tt.bucket { - if got := newHasher().Hash(tt.key, i+1); got != v { + hasher := &jmphasher{} + if got := hasher.Hash(tt.key, i+1); got != v { t.Errorf("hash(%v,%v)=%v, want %v", tt.key, i+1, got, v) } } diff --git a/executor.go b/executor.go index d0af5b241..52f851e5a 100644 --- a/executor.go +++ b/executor.go @@ -1672,15 +1672,6 @@ type execOptions struct { ExcludeColumns bool } -// decodeError returns an error representation of s if s is non-blank. -// Returns nil if s is blank. -func decodeError(s string) error { - if s == "" { - return nil - } - return errors.New(s) -} - // hasOnlySetRowAttrs returns true if calls only contains SetRowAttrs() calls. func hasOnlySetRowAttrs(calls []*pql.Call) bool { if len(calls) == 0 { diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 0d4e9fa49..a04ea5387 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -1250,7 +1250,9 @@ func mustOpenFragment(index, field, view string, shard uint64, cacheType string) f := newFragment(file.Name(), index, field, view, shard) f.CacheType = cacheType - f.RowAttrStore = newMemAttrStore() + f.RowAttrStore = &memAttrStore{ + store: make(map[uint64]map[string]interface{}), + } if err := f.Open(); err != nil { panic(err) diff --git a/http/handler.go b/http/handler.go index a69d9990c..7a3d611ac 100644 --- a/http/handler.go +++ b/http/handler.go @@ -1122,12 +1122,9 @@ func (h *Handler) handleGetVersion(w http.ResponseWriter, r *http.Request) { // QueryResult types. const ( - queryResultTypeNil uint32 = iota - QueryResultTypeRow + QueryResultTypeRow uint32 = iota QueryResultTypePairs - queryResultTypeValCount QueryResultTypeUint64 - queryResultTypeBool ) // parseUint64Slice returns a slice of uint64s from a comma-delimited string. @@ -1149,14 +1146,6 @@ func parseUint64Slice(s string) ([]uint64, error) { return a, nil } -// errorString returns the string representation of err. -func errorString(err error) string { - if err == nil { - return "" - } - return err.Error() -} - func (h *Handler) handlePostClusterResizeSetCoordinator(w http.ResponseWriter, r *http.Request) { if !validHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) diff --git a/roaring/roaring.go b/roaring/roaring.go index 3a0418aa3..7c2a4e11b 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1800,7 +1800,7 @@ type containerInfo struct { // flip returns a new container containing the inverse of all // bits in a. -func flip(a *Container) *Container { +func flip(a *Container) *Container { // nolint: deadcode if a.isArray() { return flipArray(a) } else if a.isRun() { @@ -3304,7 +3304,7 @@ func xorBitmapRun(a, b *Container) *Container { return output } -func bitmapsEqual(b, c *Bitmap) error { +func bitmapsEqual(b, c *Bitmap) error { // nolint: deadcode if b.OpWriter != c.OpWriter { return errors.New("opWriters not equal") } @@ -3336,22 +3336,6 @@ func popcount(x uint64) uint64 { return uint64(bits.OnesCount64(x)) } -func popcountSlice(s []uint64) uint64 { - cnt := uint64(0) - for _, x := range s { - cnt += popcount(x) - } - return cnt -} - -func popcountMaskSlice(s, m []uint64) uint64 { - cnt := uint64(0) - for i := range s { - cnt += popcount(s[i] &^ m[i]) - } - return cnt -} - func popcountAndSlice(s, m []uint64) uint64 { cnt := uint64(0) for i := range s { @@ -3359,19 +3343,3 @@ func popcountAndSlice(s, m []uint64) uint64 { } return cnt } - -func popcountOrSlice(s, m []uint64) uint64 { - cnt := uint64(0) - for i := range s { - cnt += popcount(s[i] | m[i]) - } - return cnt -} - -func popcountXorSlice(s, m []uint64) uint64 { - cnt := uint64(0) - for i := range s { - cnt += popcount(s[i] ^ m[i]) - } - return cnt -} diff --git a/view_internal_test.go b/view_internal_test.go index 6fe0b0e33..3f00fb38a 100644 --- a/view_internal_test.go +++ b/view_internal_test.go @@ -30,7 +30,9 @@ func mustOpenView(index, field, name string) *view { if err := v.open(); err != nil { panic(err) } - v.rowAttrStore = newMemAttrStore() + v.rowAttrStore = &memAttrStore{ + store: make(map[uint64]map[string]interface{}), + } return v } From 0b86bbb4f5fc5e88378693ef9b65100a5726ca7b Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 18 Jul 2018 11:58:27 -0500 Subject: [PATCH 5/8] Fix linter issues: gochecknoinits --- Makefile | 1 + broadcast.go | 6 +----- cmd/check.go | 6 +----- cmd/config.go | 4 ---- cmd/export.go | 6 +----- cmd/generate_config.go | 6 +----- cmd/import.go | 4 ---- cmd/inspect.go | 6 +----- cmd/root.go | 16 +++++++++------- cmd/server.go | 4 ---- enterprise/b/btree.go | 13 ++----------- enterprise/enterprise.go | 2 +- gc.go | 6 +----- http/client_test.go | 19 ++++++------------- logger.go | 6 +----- server/server.go | 7 +++---- stats.go | 6 +----- version.go | 4 +++- 18 files changed, 33 insertions(+), 89 deletions(-) diff --git a/Makefile b/Makefile index 65df8f894..9fd7c00ca 100644 --- a/Makefile +++ b/Makefile @@ -113,6 +113,7 @@ gometalinter: require-gometalinter gometalinter --vendor --disable-all \ --deadline=60s \ --enable=deadcode \ + --enable=gochecknoinits \ --enable=gofmt \ --enable=goimports \ --enable=gotype \ diff --git a/broadcast.go b/broadcast.go index a3ea01a4f..6f2245992 100644 --- a/broadcast.go +++ b/broadcast.go @@ -37,12 +37,8 @@ type broadcaster interface { // TODO add at least a single "isMessage()" method. type Message interface{} -func init() { - NopBroadcaster = &nopBroadcaster{} -} - // NopBroadcaster represents a Broadcaster that doesn't do anything. -var NopBroadcaster broadcaster +var NopBroadcaster broadcaster = &nopBroadcaster{} type nopBroadcaster struct{} diff --git a/cmd/check.go b/cmd/check.go index 8785a78e1..f9adf0980 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -27,7 +27,7 @@ import ( var checker *ctl.CheckCommand -func newCheckCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { +func newCheckCommand(_ io.Reader, _, _ io.Writer) *cobra.Command { checker = ctl.NewCheckCommand(os.Stdin, os.Stdout, os.Stderr) checkCmd := &cobra.Command{ Use: "check [path2]...", @@ -48,7 +48,3 @@ Performs a consistency check on data files. } return checkCmd } - -func init() { - subcommandFns["check"] = newCheckCommand -} diff --git a/cmd/config.go b/cmd/config.go index 3d65fa131..0288c34f0 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -49,7 +49,3 @@ func newConfigCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command return confCmd } - -func init() { - subcommandFns["config"] = newConfigCommand -} diff --git a/cmd/export.go b/cmd/export.go index d0f63edbf..0a087e82a 100644 --- a/cmd/export.go +++ b/cmd/export.go @@ -26,7 +26,7 @@ import ( var Exporter *ctl.ExportCommand -func newExportCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { +func newExportCommand(_ io.Reader, _, _ io.Writer) *cobra.Command { Exporter = ctl.NewExportCommand(os.Stdin, os.Stdout, os.Stderr) exportCmd := &cobra.Command{ Use: "export", @@ -58,7 +58,3 @@ The file does not contain any headers. return exportCmd } - -func init() { - subcommandFns["export"] = newExportCommand -} diff --git a/cmd/generate_config.go b/cmd/generate_config.go index 0b5b81462..7ff4b0833 100644 --- a/cmd/generate_config.go +++ b/cmd/generate_config.go @@ -26,7 +26,7 @@ import ( var generateConf *ctl.GenerateConfigCommand -func newGenerateConfigCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { +func newGenerateConfigCommand(_ io.Reader, _, _ io.Writer) *cobra.Command { generateConf = ctl.NewGenerateConfigCommand(os.Stdin, os.Stdout, os.Stderr) confCmd := &cobra.Command{ Use: "generate-config", @@ -43,7 +43,3 @@ func newGenerateConfigCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra. return confCmd } - -func init() { - subcommandFns["generate-config"] = newGenerateConfigCommand -} diff --git a/cmd/import.go b/cmd/import.go index 7b4d00efd..c726b805e 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -65,7 +65,3 @@ omitted. If it is present then its format should be YYYY-MM-DDTHH:MM. return importCmd } - -func init() { - subcommandFns["import"] = newImportCommand -} diff --git a/cmd/inspect.go b/cmd/inspect.go index 096787337..f0f948807 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -27,7 +27,7 @@ import ( var inspector *ctl.InspectCommand -func newInspectCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { +func newInspectCommand(_ io.Reader, _, _ io.Writer) *cobra.Command { inspector = ctl.NewInspectCommand(os.Stdin, os.Stdout, os.Stderr) inspectCmd := &cobra.Command{ @@ -51,7 +51,3 @@ Inspects a data file and provides stats. } return inspectCmd } - -func init() { - subcommandFns["inspect"] = newInspectCommand -} diff --git a/cmd/root.go b/cmd/root.go index fd55cbcef..64b8b1ca9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -25,10 +25,6 @@ import ( "github.com/spf13/viper" ) -// TODO maybe give this an Add method which will ensure two command -// with same name aren't added -var subcommandFns = map[string]func(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command{} - func NewRootCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { productName := "Pilosa " + pilosa.Version if pilosa.EnterpriseEnabled { @@ -69,9 +65,15 @@ Build Time: ` + pilosa.BuildTime + "\n", rc.PersistentFlags().Bool("dry-run", false, "stop before executing") _ = rc.PersistentFlags().MarkHidden("dry-run") rc.PersistentFlags().StringP("config", "c", "", "Configuration file to read from.") - for _, subcomFn := range subcommandFns { - rc.AddCommand(subcomFn(stdin, stdout, stderr)) - } + + rc.AddCommand(newCheckCommand(stdin, stdout, stderr)) + rc.AddCommand(newConfigCommand(stdin, stdout, stderr)) + rc.AddCommand(newExportCommand(stdin, stdout, stderr)) + rc.AddCommand(newGenerateConfigCommand(stdin, stdout, stderr)) + rc.AddCommand(newImportCommand(stdin, stdout, stderr)) + rc.AddCommand(newInspectCommand(stdin, stdout, stderr)) + rc.AddCommand(newServeCmd(stdin, stdout, stderr)) + rc.SetOutput(stderr) return rc } diff --git a/cmd/server.go b/cmd/server.go index 83d18504c..d4834672e 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -50,7 +50,3 @@ on the configured port.`, ctl.BuildServerFlags(serveCmd, Server) return serveCmd } - -func init() { - subcommandFns["server"] = newServeCmd -} diff --git a/enterprise/b/btree.go b/enterprise/b/btree.go index c732e194d..3deb139b6 100644 --- a/enterprise/b/btree.go +++ b/enterprise/b/btree.go @@ -32,7 +32,6 @@ package b import ( - "fmt" "io" "sync" @@ -40,20 +39,12 @@ import ( ) const ( + // kx must be >= 2 kx = 128 //TODO benchmark tune this number if using custom key/value type(s). + // kd must be >= 1 kd = 128 //TODO benchmark tune this number if using custom key/value type(s). ) -func init() { - if kd < 1 { - panic(fmt.Errorf("kd %d: out of range", kd)) - } - - if kx < 2 { - panic(fmt.Errorf("kx %d: out of range", kx)) - } -} - var ( btDPool = sync.Pool{New: func() interface{} { return &d{} }} btEPool = btEpool{sync.Pool{New: func() interface{} { return &enumerator{} }}} diff --git a/enterprise/enterprise.go b/enterprise/enterprise.go index db9d6e2dd..f69ef1dd7 100644 --- a/enterprise/enterprise.go +++ b/enterprise/enterprise.go @@ -26,7 +26,7 @@ import ( "github.com/pilosa/pilosa/roaring" ) -func init() { +func init() { // nolint: gochecknoinits // Replace Bitmap constructor with B+Tree implementation roaring.NewFileBitmap = b.NewBTreeBitmap } diff --git a/gc.go b/gc.go index 23dd0f0d0..1456c22c2 100644 --- a/gc.go +++ b/gc.go @@ -23,12 +23,8 @@ type GCNotifier interface { AfterGC() <-chan struct{} } -func init() { - NopGCNotifier = &nopGCNotifier{} -} - // NopGCNotifier represents a GCNotifier that doesn't do anything. -var NopGCNotifier GCNotifier +var NopGCNotifier GCNotifier = &nopGCNotifier{} type nopGCNotifier struct{} diff --git a/http/client_test.go b/http/client_test.go index 2944a7586..0101bd2a6 100644 --- a/http/client_test.go +++ b/http/client_test.go @@ -29,13 +29,6 @@ import ( "github.com/pilosa/pilosa/test" ) -var defaultClient *gohttp.Client - -func init() { - defaultClient = http.GetHTTPClient(nil) - -} - // Test distributed TopN Row count across 3 nodes. func TestClient_MultiNode(t *testing.T) { c := test.MustRunCluster(t, 3, @@ -125,9 +118,9 @@ func TestClient_MultiNode(t *testing.T) { // Connect to each node to compare results. client := make([]*Client, 3) - client[0] = MustNewClient(c[0].URL(), defaultClient) - client[1] = MustNewClient(c[1].URL(), defaultClient) - client[2] = MustNewClient(c[2].URL(), defaultClient) + client[0] = MustNewClient(c[0].URL(), http.GetHTTPClient(nil)) + client[1] = MustNewClient(c[1].URL(), http.GetHTTPClient(nil)) + client[2] = MustNewClient(c[2].URL(), http.GetHTTPClient(nil)) topN := 4 queryRequest := &pilosa.QueryRequest{ @@ -191,7 +184,7 @@ func TestClient_Import(t *testing.T) { hldr.Row("i", "f", 0) // Send import request. - c := MustNewClient(host, defaultClient) + c := MustNewClient(host, http.GetHTTPClient(nil)) if err := c.Import(context.Background(), "i", "f", 0, []pilosa.Bit{ {RowID: 0, ColumnID: 1}, {RowID: 0, ColumnID: 5}, @@ -226,7 +219,7 @@ func TestClient_ImportValue(t *testing.T) { } // Send import request. - c := MustNewClient(host, defaultClient) + c := MustNewClient(host, http.GetHTTPClient(nil)) if err := c.ImportValue(context.Background(), "i", "f", 0, []pilosa.FieldValue{ {ColumnID: 1, Value: -10}, {ColumnID: 2, Value: 20}, @@ -287,7 +280,7 @@ func TestClient_FragmentBlocks(t *testing.T) { // Set a bit on a different shard. hldr.SetBit("i", "f", 0, 1) - c := MustNewClient(cmd.URL(), defaultClient) + c := MustNewClient(cmd.URL(), http.GetHTTPClient(nil)) blocks, err := c.FragmentBlocks(context.Background(), nil, "i", "f", 0) if err != nil { t.Fatal(err) diff --git a/logger.go b/logger.go index 28b35b999..074da8a36 100644 --- a/logger.go +++ b/logger.go @@ -28,12 +28,8 @@ type Logger interface { Debugf(format string, v ...interface{}) } -func init() { - NopLogger = &nopLogger{} -} - // NopLogger represents a Logger that doesn't do anything. -var NopLogger Logger +var NopLogger Logger = &nopLogger{} type nopLogger struct{} diff --git a/server/server.go b/server/server.go index 9060256af..2433def7a 100644 --- a/server/server.go +++ b/server/server.go @@ -44,10 +44,6 @@ import ( "github.com/pkg/errors" ) -func init() { - rand.Seed(time.Now().UTC().UnixNano()) -} - type loggerLogger interface { pilosa.Logger Logger() *log.Logger @@ -126,6 +122,9 @@ func NewCommand(stdin io.Reader, stdout, stderr io.Writer, opts ...CommandOption func (m *Command) Start() (err error) { defer close(m.Started) + // Seed random number generator + rand.Seed(time.Now().UTC().UnixNano()) + // SetupServer err = m.SetupServer() if err != nil { diff --git a/stats.go b/stats.go index 130a91a64..8f23c77aa 100644 --- a/stats.go +++ b/stats.go @@ -22,10 +22,6 @@ import ( "time" ) -func init() { - NopStatsClient = &nopStatsClient{} -} - // Expvar global expvar map. var Expvar = expvar.NewMap("index") @@ -66,7 +62,7 @@ type StatsClient interface { } // NopStatsClient represents a client that doesn't do anything. -var NopStatsClient StatsClient +var NopStatsClient StatsClient = &nopStatsClient{} type nopStatsClient struct{} diff --git a/version.go b/version.go index 04096f7cc..4164dee32 100644 --- a/version.go +++ b/version.go @@ -19,7 +19,9 @@ var EnterpriseEnabled = false var Version = "v0.0.0" var BuildTime = "not recorded" -func init() { +// init sets the EnterpriseEnabled bool, based on the Enterprise string. +// This is needed because bools cannot be set with ldflags. +func init() { // nolint: gochecknoinits if Enterprise == "1" { EnterpriseEnabled = true } From d4510172d3a400a6d52bee5eb9b1bba668a4ca4a Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 18 Jul 2018 14:02:56 -0500 Subject: [PATCH 6/8] Fix linter issues: ineffassign --- Makefile | 1 + ctl/import_test.go | 6 ++++++ ctl/inspect_test.go | 3 +++ fragment.go | 5 +++-- roaring/roaring_internal_test.go | 2 +- test/pilosa.go | 3 +++ test/pilosa_test.go | 3 +++ 7 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 65df8f894..e7dcb5e77 100644 --- a/Makefile +++ b/Makefile @@ -117,6 +117,7 @@ gometalinter: require-gometalinter --enable=goimports \ --enable=gotype \ --enable=gotypex \ + --enable=ineffassign \ --enable=interfacer \ --enable=misspell \ --enable=unparam \ diff --git a/ctl/import_test.go b/ctl/import_test.go index 56970e9be..a41f6d525 100644 --- a/ctl/import_test.go +++ b/ctl/import_test.go @@ -203,6 +203,9 @@ func TestImportCommand_BugOverwriteValue(t *testing.T) { file.Close() file, err = ioutil.TempFile("", "import-value2.csv") + if err != nil { + t.Fatalf("Error creating tempfile: %s", err) + } file.Write([]byte("0,16\n")) cm.Paths = []string{file.Name()} err = cm.Run(ctx) @@ -212,6 +215,9 @@ func TestImportCommand_BugOverwriteValue(t *testing.T) { file.Close() file, err = ioutil.TempFile("", "import-value3.csv") + if err != nil { + t.Fatalf("Error creating tempfile: %s", err) + } file.Write([]byte("0,19\n")) cm.Paths = []string{file.Name()} err = cm.Run(ctx) diff --git a/ctl/inspect_test.go b/ctl/inspect_test.go index 5cf34481d..c7a48d400 100644 --- a/ctl/inspect_test.go +++ b/ctl/inspect_test.go @@ -31,6 +31,9 @@ func TestInspectCommand_Run(t *testing.T) { cm := NewInspectCommand(stdin, w, w) file, err := ioutil.TempFile("", "inspectTest") + if err != nil { + t.Fatalf("Error creating tempfile: %s", err) + } file.Write([]byte("12358267538963")) file.Close() cm.Path = file.Name() diff --git a/fragment.go b/fragment.go index 371b4ef46..b20478e54 100644 --- a/fragment.go +++ b/fragment.go @@ -584,9 +584,9 @@ func (f *fragment) sum(filter *Row, bitDepth uint) (sum, count uint64, err error // // 10*(2^0) + 4*(2^1) + 3*(2^2) = 30 // + var cnt uint64 for i := uint(0); i < bitDepth; i++ { row := f.row(uint64(i)) - cnt := uint64(0) if filter != nil { cnt = row.intersectionCount(filter) } else { @@ -1713,10 +1713,11 @@ func (f *fragment) rows() []uint64 { } func (f *fragment) rowsForColumn(columnID uint64) []uint64 { + var colKey uint64 + colID := columnID % ShardWidth i, _ := f.storage.Containers.Iterator(0) - colKey := uint64(0) colVal := uint16(colID & 0xFFFF) rows := make([]uint64, 0) diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 5836782b8..3a56e30f3 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -2276,7 +2276,7 @@ func TestIteratorRuns(t *testing.T) { t.Fatalf("iterator did not seek correctly in multiple containers: %v\n", itr) } - val, eof = itr.Next() + itr.Next() val, eof = itr.Next() if !(val == 0 && eof) { t.Fatalf("iterator did not eof correctly: %d, %v\n", val, eof) diff --git a/test/pilosa.go b/test/pilosa.go index 001949906..507dd0af3 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -243,6 +243,9 @@ func MustDo(method, urlStr string, body string) *httpResponse { urlStr, strings.NewReader(body), ) + if err != nil { + panic(err) + } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") diff --git a/test/pilosa_test.go b/test/pilosa_test.go index 25bb808df..1f00ff68b 100644 --- a/test/pilosa_test.go +++ b/test/pilosa_test.go @@ -40,6 +40,9 @@ func TestNewCluster(t *testing.T) { cluster[0].URL()+"/status", strings.NewReader(""), ) + if err != nil { + t.Fatalf("creating http request: %v", err) + } req.Header.Set("Accept", "application/json") From 2aa4d6b12f66509d127b1bcf3612bcc87b4df1f1 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 18 Jul 2018 15:23:36 -0500 Subject: [PATCH 7/8] Fix linter issues: nakedret --- Makefile | 1 + boltdb/attrstore.go | 9 ++++--- enterprise/b/btree.go | 46 ++++++++++++++++++++------------ enterprise/b/containers_btree.go | 5 ++-- field.go | 12 ++++----- fragment.go | 17 ++++++------ http/handler.go | 8 +++--- lru/lru.go | 6 ++--- roaring/roaring.go | 23 ++++++++-------- 9 files changed, 73 insertions(+), 54 deletions(-) diff --git a/Makefile b/Makefile index 65df8f894..bf1b208f4 100644 --- a/Makefile +++ b/Makefile @@ -119,6 +119,7 @@ gometalinter: require-gometalinter --enable=gotypex \ --enable=interfacer \ --enable=misspell \ + --enable=nakedret \ --enable=unparam \ --exclude "^internal/.*\.pb\.go" \ --exclude "^pql/pql.peg.go" \ diff --git a/boltdb/attrstore.go b/boltdb/attrstore.go index 423abcc36..710dccfac 100644 --- a/boltdb/attrstore.go +++ b/boltdb/attrstore.go @@ -120,17 +120,20 @@ func (s *attrStore) Close() error { } // Attrs returns a set of attributes by ID. -func (s *attrStore) Attrs(id uint64) (m map[string]interface{}, err error) { +func (s *attrStore) Attrs(id uint64) (map[string]interface{}, error) { s.mu.RLock() defer s.mu.RUnlock() + var m map[string]interface{} + // Check cache for map. if m = s.attrCache.Get(id); m != nil { return m, nil } // Find attributes from storage. - if err = s.db.View(func(tx *bolt.Tx) error { + if err := s.db.View(func(tx *bolt.Tx) error { + var err error m, err = txAttrs(tx, id) if err != nil { return err @@ -143,7 +146,7 @@ func (s *attrStore) Attrs(id uint64) (m map[string]interface{}, err error) { // Add to cache. s.attrCache.Set(id, m) - return + return m, nil } // SetAttrs sets attribute values for a given ID. diff --git a/enterprise/b/btree.go b/enterprise/b/btree.go index c732e194d..7489333eb 100644 --- a/enterprise/b/btree.go +++ b/enterprise/b/btree.go @@ -193,7 +193,8 @@ func (q *x) insert(i int, k uint64, ch interface{}) *x { return q } -func (q *x) siblings(i int) (l, r *d) { +func (q *x) siblings(i int) (*d, *d) { + var l, r *d if i >= 0 { if i > 0 { l = q.x[i-1].ch.(*d) @@ -202,7 +203,7 @@ func (q *x) siblings(i int) (l, r *d) { r = q.x[i+1].ch.(*d) } } - return + return l, r } // -------------------------------------------------------------------------- d @@ -419,12 +420,14 @@ func (t *tree) find(q interface{}, k uint64) (i int, ok bool) { // First returns the first item of the tree in the key collating order, or // (zero-value, zero-value) if the tree is empty. -func (t *tree) First() (k uint64, v *roaring.Container) { +func (t *tree) First() (uint64, *roaring.Container) { + var k uint64 + var v *roaring.Container if q := t.first; q != nil { q := &q.d[0] k, v = q.k, q.v } - return + return k, v } // Get returns the value associated with k and true if it exists. Otherwise Get @@ -470,12 +473,14 @@ func (t *tree) insert(q *d, i int, k uint64, v *roaring.Container) *d { // Last returns the last item of the tree in the key collating order, or // (zero-value, zero-value) if the tree is empty. -func (t *tree) Last() (k uint64, v *roaring.Container) { +func (t *tree) Last() (uint64, *roaring.Container) { + var k uint64 + var v *roaring.Container if q := t.last; q != nil { q := &q.d[q.c-1] k, v = q.k, q.v } - return + return k, v } // Len returns the number of items in the tree. @@ -858,9 +863,12 @@ func (e *enumerator) Close() { // Next returns the currently enumerated item, if it exists and moves to the // next item in the key collation order. If there is no item to return, err == // io.EOF is returned. -func (e *enumerator) Next() (k uint64, v *roaring.Container, err error) { +func (e *enumerator) Next() (uint64, *roaring.Container, error) { + var k uint64 + var v *roaring.Container + var err error if err = e.err; err != nil { - return + return 0, nil, err } if e.ver != e.t.ver { @@ -870,12 +878,12 @@ func (e *enumerator) Next() (k uint64, v *roaring.Container, err error) { } if e.q == nil { e.err, err = io.EOF, io.EOF - return + return 0, nil, err } if e.i >= e.q.c { if err = e.next(); err != nil { - return + return 0, nil, err } } @@ -883,7 +891,7 @@ func (e *enumerator) Next() (k uint64, v *roaring.Container, err error) { k, v = i.k, i.v e.k, e.hit = k, true e.next() - return + return k, v, nil } func (e *enumerator) next() error { @@ -906,9 +914,13 @@ func (e *enumerator) next() error { // Prev returns the currently enumerated item, if it exists and moves to the // previous item in the key collation order. If there is no item to return, err // == io.EOF is returned. -func (e *enumerator) Prev() (k uint64, v *roaring.Container, err error) { +func (e *enumerator) Prev() (uint64, *roaring.Container, error) { + var k uint64 + var v *roaring.Container + var err error + if err = e.err; err != nil { - return + return 0, nil, err } if e.ver != e.t.ver { @@ -918,19 +930,19 @@ func (e *enumerator) Prev() (k uint64, v *roaring.Container, err error) { } if e.q == nil { e.err, err = io.EOF, io.EOF - return + return 0, nil, err } if !e.hit { // move to previous because Seek overshoots if there's no hit if err = e.prev(); err != nil { - return + return 0, nil, err } } if e.i >= e.q.c { if err = e.prev(); err != nil { - return + return 0, nil, err } } @@ -938,7 +950,7 @@ func (e *enumerator) Prev() (k uint64, v *roaring.Container, err error) { k, v = i.k, i.v e.k, e.hit = k, true e.prev() - return + return k, v, err } func (e *enumerator) prev() error { diff --git a/enterprise/b/containers_btree.go b/enterprise/b/containers_btree.go index 95fcb09b3..13a5a97c7 100644 --- a/enterprise/b/containers_btree.go +++ b/enterprise/b/containers_btree.go @@ -121,14 +121,15 @@ func (btc *bTreeContainers) GetOrCreate(key uint64) *roaring.Container { return btc.lastContainer } -func (btc *bTreeContainers) Count() (n uint64) { +func (btc *bTreeContainers) Count() uint64 { + var n uint64 e, _ := btc.tree.Seek(0) _, c, err := e.Next() for err != io.EOF { n += uint64(c.N()) _, c, err = e.Next() } - return + return n } func (btc *bTreeContainers) Clone() roaring.Containers { diff --git a/field.go b/field.go index a1bc17759..135ffd0eb 100644 --- a/field.go +++ b/field.go @@ -796,8 +796,8 @@ func groupCompare(a, b string, offset int) (lt, eq bool) { return v < 0, v == 0 } -func (f *Field) allTimeViewsSortedByQuantum() (me []*view) { - me = make([]*view, len(f.viewMap), len(f.viewMap)) +func (f *Field) allTimeViewsSortedByQuantum() []*view { + me := make([]*view, len(f.viewMap), len(f.viewMap)) prefix := viewStandard + "_" offset := len(viewStandard) + 1 i := 0 @@ -811,8 +811,8 @@ func (f *Field) allTimeViewsSortedByQuantum() (me []*view) { year := strings.Index(me[0].name, "_") + 4 month := year + 2 day := month + 2 - sort.Slice(me, func(i, j int) (lt bool) { - var eq bool + sort.Slice(me, func(i, j int) bool { + var eq, lt bool // group by quantum from year to hour if lt, eq = groupCompare(me[i].name, me[j].name, year); eq { if lt, eq = groupCompare(me[i].name, me[j].name, month); eq { @@ -821,9 +821,9 @@ func (f *Field) allTimeViewsSortedByQuantum() (me []*view) { } } } - return + return lt }) - return + return me } // Value reads a field value for a column. diff --git a/fragment.go b/fragment.go index 371b4ef46..30763fc78 100644 --- a/fragment.go +++ b/fragment.go @@ -1162,15 +1162,16 @@ func (f *fragment) readContiguousChecksums(a *[]FragmentBlock, blockID int) (n i } // blockData returns bits in a block as row & column ID pairs. -func (f *fragment) blockData(id int) (rowIDs, columnIDs []uint64) { +func (f *fragment) blockData(id int) ([]uint64, []uint64) { f.mu.Lock() defer f.mu.Unlock() - + rowIDs := make([]uint64, 0) + columnIDs := make([]uint64, 0) f.storage.ForEachRange(uint64(id)*HashBlockSize*ShardWidth, (uint64(id)+1)*HashBlockSize*ShardWidth, func(i uint64) { rowIDs = append(rowIDs, i/ShardWidth) columnIDs = append(columnIDs, i%ShardWidth) }) - return + return rowIDs, columnIDs } // mergeBlock compares the block's bits and computes a diff with another set of block bits. @@ -1965,12 +1966,12 @@ func (s *fragmentSyncer) syncBlock(id int) error { return nil } -func madvise(b []byte, advice int) (err error) { // nolint: unparam - _, _, e1 := syscall.Syscall(syscall.SYS_MADVISE, uintptr(unsafe.Pointer(&b[0])), uintptr(len(b)), uintptr(advice)) - if e1 != 0 { - err = e1 +func madvise(b []byte, advice int) error { // nolint: unparam + _, _, err := syscall.Syscall(syscall.SYS_MADVISE, uintptr(unsafe.Pointer(&b[0])), uintptr(len(b)), uintptr(advice)) + if err != 0 { + return err } - return + return nil } // pairSet is a list of equal length row and column id lists. diff --git a/http/handler.go b/http/handler.go index 7a3d611ac..9e3037d64 100644 --- a/http/handler.go +++ b/http/handler.go @@ -299,10 +299,12 @@ type successResponse struct { // check determines success or failure based on the error. // It also returns the corresponding http status code. -func (r *successResponse) check(err error) (statusCode int) { +func (r *successResponse) check(err error) int { + var statusCode int + if err == nil { r.Success = true - return + return 0 } cause := errors.Cause(err) @@ -322,7 +324,7 @@ func (r *successResponse) check(err error) (statusCode int) { r.Success = false r.Error = &Error{Message: cause.Error()} - return + return statusCode } // write sends a response to the http.ResponseWriter based on the success diff --git a/lru/lru.go b/lru/lru.go index 6b3ed3daa..e384e2c34 100644 --- a/lru/lru.go +++ b/lru/lru.go @@ -71,15 +71,15 @@ func (c *Cache) Add(key Key, value interface{}) { } // Get looks up a key's value from the cache. -func (c *Cache) Get(key Key) (value interface{}, ok bool) { +func (c *Cache) Get(key Key) (interface{}, bool) { if c.cache == nil { - return + return nil, false } if ele, hit := c.cache[key]; hit { c.ll.MoveToFront(ele) return ele.Value.(*entry).value, true } - return + return nil, false } // remove removes the provided key from the cache. diff --git a/roaring/roaring.go b/roaring/roaring.go index 7c2a4e11b..a608fac89 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1895,7 +1895,8 @@ func intersectionCountArrayRun(a, b *Container) (n int) { return n } -func intersectionCountRunRun(a, b *Container) (n int) { +func intersectionCountRunRun(a, b *Container) int { + var n int na, nb := len(a.runs), len(b.runs) for i, j := 0, 0; i < na && j < nb; { va, vb := a.runs[i], b.runs[j] @@ -1923,7 +1924,7 @@ func intersectionCountRunRun(a, b *Container) (n int) { i++ } } - return + return n } func intersectionCountBitmapRun(a, b *Container) (n int) { @@ -3144,22 +3145,20 @@ func xorArrayRun(a, b *Container) *Container { } // xorCompare computes first exclusive run between two runs. -func xorCompare(x *xorstm) (r1 interval16, hasData bool) { - hasData = false +func xorCompare(x *xorstm) (interval16, bool) { + var r1 interval16 + var hasData bool + if !x.vaValid || !x.vbValid { if x.vbValid { x.vbValid = false - r1 = x.vb - hasData = true - return + return x.vb, true } if x.vaValid { x.vaValid = false - r1 = x.va - hasData = true - return + return x.va, true } - return + return r1, false } if x.va.last < x.vb.start { //va before @@ -3232,7 +3231,7 @@ func xorCompare(x *xorstm) (r1 interval16, hasData bool) { } } } - return + return r1, hasData } //stm is state machine used to "xor" iterate over runs. From 0a9e6bca7a3853230218ac3a481f7a54fcbd954d Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Thu, 19 Jul 2018 08:01:44 -0500 Subject: [PATCH 8/8] Re-add return variable names removed in 2aa4d6b1. --- boltdb/attrstore.go | 7 ++----- enterprise/b/btree.go | 22 +++++----------------- enterprise/b/containers_btree.go | 3 +-- field.go | 8 ++++---- fragment.go | 4 +--- http/handler.go | 4 +--- lru/lru.go | 2 +- roaring/roaring.go | 9 +++------ 8 files changed, 18 insertions(+), 41 deletions(-) diff --git a/boltdb/attrstore.go b/boltdb/attrstore.go index 710dccfac..c6fa9bb76 100644 --- a/boltdb/attrstore.go +++ b/boltdb/attrstore.go @@ -120,20 +120,17 @@ func (s *attrStore) Close() error { } // Attrs returns a set of attributes by ID. -func (s *attrStore) Attrs(id uint64) (map[string]interface{}, error) { +func (s *attrStore) Attrs(id uint64) (m map[string]interface{}, err error) { s.mu.RLock() defer s.mu.RUnlock() - var m map[string]interface{} - // Check cache for map. if m = s.attrCache.Get(id); m != nil { return m, nil } // Find attributes from storage. - if err := s.db.View(func(tx *bolt.Tx) error { - var err error + if err = s.db.View(func(tx *bolt.Tx) error { m, err = txAttrs(tx, id) if err != nil { return err diff --git a/enterprise/b/btree.go b/enterprise/b/btree.go index 8081500c6..7b9b34e76 100644 --- a/enterprise/b/btree.go +++ b/enterprise/b/btree.go @@ -184,8 +184,7 @@ func (q *x) insert(i int, k uint64, ch interface{}) *x { return q } -func (q *x) siblings(i int) (*d, *d) { - var l, r *d +func (q *x) siblings(i int) (l, r *d) { if i >= 0 { if i > 0 { l = q.x[i-1].ch.(*d) @@ -411,9 +410,7 @@ func (t *tree) find(q interface{}, k uint64) (i int, ok bool) { // First returns the first item of the tree in the key collating order, or // (zero-value, zero-value) if the tree is empty. -func (t *tree) First() (uint64, *roaring.Container) { - var k uint64 - var v *roaring.Container +func (t *tree) First() (k uint64, v *roaring.Container) { if q := t.first; q != nil { q := &q.d[0] k, v = q.k, q.v @@ -464,9 +461,7 @@ func (t *tree) insert(q *d, i int, k uint64, v *roaring.Container) *d { // Last returns the last item of the tree in the key collating order, or // (zero-value, zero-value) if the tree is empty. -func (t *tree) Last() (uint64, *roaring.Container) { - var k uint64 - var v *roaring.Container +func (t *tree) Last() (k uint64, v *roaring.Container) { if q := t.last; q != nil { q := &q.d[q.c-1] k, v = q.k, q.v @@ -854,10 +849,7 @@ func (e *enumerator) Close() { // Next returns the currently enumerated item, if it exists and moves to the // next item in the key collation order. If there is no item to return, err == // io.EOF is returned. -func (e *enumerator) Next() (uint64, *roaring.Container, error) { - var k uint64 - var v *roaring.Container - var err error +func (e *enumerator) Next() (k uint64, v *roaring.Container, err error) { if err = e.err; err != nil { return 0, nil, err } @@ -905,11 +897,7 @@ func (e *enumerator) next() error { // Prev returns the currently enumerated item, if it exists and moves to the // previous item in the key collation order. If there is no item to return, err // == io.EOF is returned. -func (e *enumerator) Prev() (uint64, *roaring.Container, error) { - var k uint64 - var v *roaring.Container - var err error - +func (e *enumerator) Prev() (k uint64, v *roaring.Container, err error) { if err = e.err; err != nil { return 0, nil, err } diff --git a/enterprise/b/containers_btree.go b/enterprise/b/containers_btree.go index 13a5a97c7..0ed240548 100644 --- a/enterprise/b/containers_btree.go +++ b/enterprise/b/containers_btree.go @@ -121,8 +121,7 @@ func (btc *bTreeContainers) GetOrCreate(key uint64) *roaring.Container { return btc.lastContainer } -func (btc *bTreeContainers) Count() uint64 { - var n uint64 +func (btc *bTreeContainers) Count() (n uint64) { e, _ := btc.tree.Seek(0) _, c, err := e.Next() for err != io.EOF { diff --git a/field.go b/field.go index 135ffd0eb..c89b55bd3 100644 --- a/field.go +++ b/field.go @@ -796,8 +796,8 @@ func groupCompare(a, b string, offset int) (lt, eq bool) { return v < 0, v == 0 } -func (f *Field) allTimeViewsSortedByQuantum() []*view { - me := make([]*view, len(f.viewMap), len(f.viewMap)) +func (f *Field) allTimeViewsSortedByQuantum() (me []*view) { + me = make([]*view, len(f.viewMap), len(f.viewMap)) prefix := viewStandard + "_" offset := len(viewStandard) + 1 i := 0 @@ -811,8 +811,8 @@ func (f *Field) allTimeViewsSortedByQuantum() []*view { year := strings.Index(me[0].name, "_") + 4 month := year + 2 day := month + 2 - sort.Slice(me, func(i, j int) bool { - var eq, lt bool + sort.Slice(me, func(i, j int) (lt bool) { + var eq bool // group by quantum from year to hour if lt, eq = groupCompare(me[i].name, me[j].name, year); eq { if lt, eq = groupCompare(me[i].name, me[j].name, month); eq { diff --git a/fragment.go b/fragment.go index ff469c3e0..3bc72efdc 100644 --- a/fragment.go +++ b/fragment.go @@ -1162,11 +1162,9 @@ func (f *fragment) readContiguousChecksums(a *[]FragmentBlock, blockID int) (n i } // blockData returns bits in a block as row & column ID pairs. -func (f *fragment) blockData(id int) ([]uint64, []uint64) { +func (f *fragment) blockData(id int) (rowIDs, columnIDs []uint64) { f.mu.Lock() defer f.mu.Unlock() - rowIDs := make([]uint64, 0) - columnIDs := make([]uint64, 0) f.storage.ForEachRange(uint64(id)*HashBlockSize*ShardWidth, (uint64(id)+1)*HashBlockSize*ShardWidth, func(i uint64) { rowIDs = append(rowIDs, i/ShardWidth) columnIDs = append(columnIDs, i%ShardWidth) diff --git a/http/handler.go b/http/handler.go index 9e3037d64..3f47297d4 100644 --- a/http/handler.go +++ b/http/handler.go @@ -299,9 +299,7 @@ type successResponse struct { // check determines success or failure based on the error. // It also returns the corresponding http status code. -func (r *successResponse) check(err error) int { - var statusCode int - +func (r *successResponse) check(err error) (statusCode int) { if err == nil { r.Success = true return 0 diff --git a/lru/lru.go b/lru/lru.go index e384e2c34..86450690b 100644 --- a/lru/lru.go +++ b/lru/lru.go @@ -71,7 +71,7 @@ func (c *Cache) Add(key Key, value interface{}) { } // Get looks up a key's value from the cache. -func (c *Cache) Get(key Key) (interface{}, bool) { +func (c *Cache) Get(key Key) (value interface{}, ok bool) { if c.cache == nil { return nil, false } diff --git a/roaring/roaring.go b/roaring/roaring.go index a608fac89..e7333325c 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1895,8 +1895,7 @@ func intersectionCountArrayRun(a, b *Container) (n int) { return n } -func intersectionCountRunRun(a, b *Container) int { - var n int +func intersectionCountRunRun(a, b *Container) (n int) { na, nb := len(a.runs), len(b.runs) for i, j := 0, 0; i < na && j < nb; { va, vb := a.runs[i], b.runs[j] @@ -3145,10 +3144,8 @@ func xorArrayRun(a, b *Container) *Container { } // xorCompare computes first exclusive run between two runs. -func xorCompare(x *xorstm) (interval16, bool) { - var r1 interval16 - var hasData bool - +func xorCompare(x *xorstm) (r1 interval16, hasData bool) { + hasData = false if !x.vaValid || !x.vbValid { if x.vbValid { x.vbValid = false