From ea2860265c5498fd550a585562cee218ded0f05a Mon Sep 17 00:00:00 2001
From: jaffee <jaffee@umbel.com>
Date: Wed, 18 Jan 2017 19:35:49 -0600
Subject: [PATCH] docs updates for benchmarking

---
 bench/bench.go       | 20 ++++++++------------
 bench/diagonal.go    | 10 ++++++++++
 bench/doc.go         | 37 +++++++++++++++++++++++++++++++++++++
 bench/import.go      | 10 ++++++++++
 bench/multidb.go     |  7 +++++++
 bench/prettify.go    |  2 ++
 bench/query.go       |  6 ++++++
 bench/random.go      |  7 +++++++
 bench/randquery.go   |  7 +++++++
 bench/sliceheight.go |  9 +++++++++
 bench/stats.go       |  6 ++++++
 bench/zipf.go        |  8 ++++++++
 12 files changed, 117 insertions(+), 12 deletions(-)
 create mode 100644 bench/doc.go

diff --git a/bench/bench.go b/bench/bench.go
index bbb9e0764..73cc625a8 100644
--- a/bench/bench.go
+++ b/bench/bench.go
@@ -8,18 +8,20 @@ import "context"
 // benchmark, and not any setup.
 type Benchmark interface {
 	// Init takes a list of hosts and an agent number. It is generally expected
-	// to set up a connection to pilosa using whatever client it chooses. These
+	// to set up a connection to pilosa using whatever client it chooses. The
 	// agentNum should be used to parameterize the benchmark's configuration if
 	// it is being run simultaneously on multiple "agents". E.G. the agentNum
 	// might be used to make a random seed different for each agent, or have
-	// each agent set a different set of bits. A Benchmark should document how
-	// the agentNum affects it.
+	// each agent set a different set of bits. Init's doc string should document
+	// how the agentNum affects it.
 	Init(hosts []string, agentNum int) error
 
 	// Run runs the benchmark. The return value of Run is kept generic so that
 	// any relevant statistics or metrics that may be specific to the benchmark
 	// in question can be reported. TODO guidelines for what gets included in
-	// results and what will get added by other stuff.
+	// results and what will get added by other stuff. Run does not need to
+	// report total run time in `results`, as that will be added by calling
+	// code.
 	Run(ctx context.Context) map[string]interface{}
 }
 
@@ -31,13 +33,7 @@ type Command interface {
 	// is so that multiple benchmarks can be specified at the command line.
 	ConsumeFlags(args []string) ([]string, error)
 
-	// Usage returns information on how to use this benchmark.
+	// Usage returns information on how to use this benchmark. The usage string
+	// should explain how the agent num affects the benchmark's operation.
 	Usage() string
 }
-
-// agentizeNum is a helper which combines the loop iteration (n) with the total
-// number of iterations and the agentNum in order to produce a globally unique
-// number across all loop iterations on all agents.
-func agentizeNum(n, iterations, agentNum int) int {
-	return n + (agentNum * iterations)
-}
diff --git a/bench/diagonal.go b/bench/diagonal.go
index c512c3eda..4cb0e354c 100644
--- a/bench/diagonal.go
+++ b/bench/diagonal.go
@@ -20,6 +20,8 @@ type DiagonalSetBits struct {
 	DB            string `json:"db"`
 }
 
+// Init sets up the pilosa client and modifies the configured values based on
+// the agent num.
 func (b *DiagonalSetBits) Init(hosts []string, agentNum int) error {
 	b.Name = "diagonal-set-bits"
 	b.BaseBitmapID = b.BaseBitmapID + (agentNum * b.Iterations)
@@ -27,10 +29,15 @@ func (b *DiagonalSetBits) Init(hosts []string, agentNum int) error {
 	return b.HasClient.Init(hosts, agentNum)
 }
 
+// Usage returns the usage message to be printed.
 func (b *DiagonalSetBits) Usage() string {
 	return `
 diagonal-set-bits sets bits with increasing profile id and bitmap id.
 
+Agent num offsets both the base profile id and base bitmap id by the number of
+iterations, so that only bits on the main diagonal are set, and agents don't
+overlap at all.
+
 Usage: diagonal-set-bits [arguments]
 
 The following arguments are available:
@@ -53,6 +60,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *DiagonalSetBits) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("DiagonalSetBits", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
diff --git a/bench/doc.go b/bench/doc.go
new file mode 100644
index 000000000..1c61a2b20
--- /dev/null
+++ b/bench/doc.go
@@ -0,0 +1,37 @@
+// bench contains benchmarks and common utilities useful to benchmarks
+//
+// In order to write new benchmarks, one must satisfy the Benchmark and Command
+// interfaces in bench.go. In order to use the benchmark from pilosactl, it
+// needs to be wired in in two places. The first is BagentCommand.ParseFlags,
+// where a case statement needs to be added, and the second is just adding the
+// benchmark to the BagentCommand.Usage usage string.
+//
+// When writing a new benchmark, there are a few things to keep in mind other
+// than just implementing the interface:
+//
+// The benchmark should modify it's own configuration in its Init method based
+// on the agentNum it is given. How it modifies is specific to the benchmark,
+// but the idea is that it should make sense to call the benchmark with the same
+// configuration, but multiple different agent numbers, and it should do useful
+// work each time (i.e. not just setting the same bits, or running the same
+// queries).
+//
+// The Init method should do everything that needs to be done to get the
+// benchmark to a runnable state - all code in run should be the stuff that we
+// actually want to time.
+//
+// The Run method does not need to report the total runtime - that is collected
+// by calling code.
+//
+// Usage should follow the format in other benchmarks, and explain how the
+// benchmark uses agentNum to modify its behavior
+//
+//
+// Files:
+//
+// 1. client.go contains pilosa client code which is shared by many benchmarks
+// 2. errgroup.go contains the ErrGroup implementation copied from golang.org/x/
+// so as not to pull in a bunch of useless deps.
+// 3. stats.go contains useful code for gathering stats about a series of timed
+// operations.
+package bench
diff --git a/bench/import.go b/bench/import.go
index bfddd4c5c..2d88b23a3 100644
--- a/bench/import.go
+++ b/bench/import.go
@@ -13,6 +13,7 @@ import (
 	"github.com/pilosa/pilosa/pilosactl"
 )
 
+// NewImport returns an Import Benchmark which pilosactl importer configured.
 func NewImport(stdin io.Reader, stdout, stderr io.Writer) *Import {
 	return &Import{
 		ImportCommand: pilosactl.NewImportCommand(stdin, stdout, stderr),
@@ -36,10 +37,13 @@ type Import struct {
 	*pilosactl.ImportCommand
 }
 
+// Usage returns the usage message to be printed.
 func (b *Import) Usage() string {
 	return `
 import generates an import file and imports using pilosa's bulk import interface
 
+Agent num can have various effects - see -agent-controls flag.
+
 Usage: import [arguments]
 
 The following arguments are available:
@@ -81,6 +85,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *Import) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("Import", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
@@ -103,6 +110,7 @@ func (b *Import) ConsumeFlags(args []string) ([]string, error) {
 	return fs.Args(), nil
 }
 
+// Init generates import data based on the agent num and fields of 'b'.
 func (b *Import) Init(hosts []string, agentNum int) error {
 	if len(hosts) == 0 {
 		return fmt.Errorf("Need at least one host")
@@ -151,12 +159,14 @@ func (b *Import) Run(ctx context.Context) map[string]interface{} {
 	return results
 }
 
+// Int64Slice is a sortable slice of 64 bit signed ints
 type Int64Slice []int64
 
 func (s Int64Slice) Len() int           { return len(s) }
 func (s Int64Slice) Less(i, j int) bool { return s[i] < s[j] }
 func (s Int64Slice) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
 
+// GenerateImportCSV writes a generated csv to 'w' which is in the form pilosactl expects for imports.
 func GenerateImportCSV(w io.Writer, baseBitmapID, maxBitmapID, baseProfileID, maxProfileID, minBitsPerMap, maxBitsPerMap, seed int64, randomOrder bool) int {
 	src := rand.NewSource(seed)
 	rng := rand.New(src)
diff --git a/bench/multidb.go b/bench/multidb.go
index 6e00a2bfe..8ea8cd135 100644
--- a/bench/multidb.go
+++ b/bench/multidb.go
@@ -19,16 +19,20 @@ type MultiDBSetBits struct {
 	Database      string `json:"database"`
 }
 
+// Init sets up the db name based on the agentNum and sets up the pilosa client.
 func (b *MultiDBSetBits) Init(hosts []string, agentNum int) error {
 	b.Name = "multi-db-set-bits"
 	b.Database = b.Database + strconv.Itoa(agentNum)
 	return b.HasClient.Init(hosts, agentNum)
 }
 
+// Usage returns the usage message to be printed.
 func (b *MultiDBSetBits) Usage() string {
 	return `
 multi-db-set-bits sets bits with increasing profile id and bitmap id using a different DB for each agent.
 
+Agent num changes the database being written to.
+
 Usage: multi-db-set-bits [arguments]
 
 The following arguments are available:
@@ -48,6 +52,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *MultiDBSetBits) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("MultiDBSetBits", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
diff --git a/bench/prettify.go b/bench/prettify.go
index c31f4118c..1902d412b 100644
--- a/bench/prettify.go
+++ b/bench/prettify.go
@@ -5,6 +5,8 @@ import "time"
 // wrapper type to force human-readable JSON output
 type PrettyDuration time.Duration
 
+// MarshalJSON returns a nicely formatted duration, instead of it just being
+// treated like an int.
 func (d PrettyDuration) MarshalJSON() ([]byte, error) {
 	s := time.Duration(d).String()
 	return []byte("\"" + s + "\""), nil
diff --git a/bench/query.go b/bench/query.go
index fdb65afac..aae645e18 100644
--- a/bench/query.go
+++ b/bench/query.go
@@ -6,6 +6,7 @@ import (
 	"github.com/pilosa/pilosa/pql"
 )
 
+// NewQueryGenerator initializes a new QueryGenerator
 func NewQueryGenerator(seed int64) *QueryGenerator {
 	return &QueryGenerator{
 		IDToFrameFn: func(id uint64) string { return "frame.n" },
@@ -14,12 +15,15 @@ func NewQueryGenerator(seed int64) *QueryGenerator {
 	}
 }
 
+// QueryGenerator holds the configuration and state for randomly generating
+// queries.
 type QueryGenerator struct {
 	IDToFrameFn func(id uint64) string
 	R           *rand.Rand
 	Frames      []string
 }
 
+// Random returns a randomly generated query.
 func (q *QueryGenerator) Random(maxN, depth, maxargs int, idmin, idmax uint64) pql.Call {
 	// TODO: handle depth==1 or 0
 	val := q.R.Intn(5)
@@ -31,6 +35,7 @@ func (q *QueryGenerator) Random(maxN, depth, maxargs int, idmin, idmax uint64) p
 	}
 }
 
+// RandomTopN returns a randomly generated TopN query.
 func (q *QueryGenerator) RandomTopN(maxN, depth, maxargs int, idmin, idmax uint64) *pql.TopN {
 	frameIdx := q.R.Intn(len(q.Frames))
 	return &pql.TopN{
@@ -40,6 +45,7 @@ func (q *QueryGenerator) RandomTopN(maxN, depth, maxargs int, idmin, idmax uint6
 	}
 }
 
+// RandomBitmapCall returns a randomly generate query which is a pql.BitmapCall.
 func (q *QueryGenerator) RandomBitmapCall(depth, maxargs int, idmin, idmax uint64) pql.BitmapCall {
 	if depth <= 1 {
 		bitmapID := q.R.Int63n(int64(idmax)-int64(idmin)) + int64(idmin)
diff --git a/bench/random.go b/bench/random.go
index 394a03b55..e3e89400c 100644
--- a/bench/random.go
+++ b/bench/random.go
@@ -24,16 +24,20 @@ type RandomSetBits struct {
 	DB             string `json:"db"`
 }
 
+// Init adds the agent num to the random seed and initializes the client.
 func (b *RandomSetBits) Init(hosts []string, agentNum int) error {
 	b.Name = "random-set-bits"
 	b.Seed = b.Seed + int64(agentNum)
 	return b.HasClient.Init(hosts, agentNum)
 }
 
+// Usage returns the usage message to be printed.
 func (b *RandomSetBits) Usage() string {
 	return `
 random-set-bits sets random bits
 
+Agent number modifies the random seed.
+
 Usage: random-set-bits [arguments]
 
 The following arguments are available:
@@ -64,6 +68,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *RandomSetBits) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("RandomSetBits", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
diff --git a/bench/randquery.go b/bench/randquery.go
index 09cf8dddb..11fa7016b 100644
--- a/bench/randquery.go
+++ b/bench/randquery.go
@@ -23,16 +23,20 @@ type RandomQuery struct {
 	DBs           []string `json:"dbs"`
 }
 
+// Init adds the agent num to the random seed and initializes the client.
 func (b *RandomQuery) Init(hosts []string, agentNum int) error {
 	b.Name = "random-query"
 	b.Seed = b.Seed + int64(agentNum)
 	return b.HasClient.Init(hosts, agentNum)
 }
 
+// Usage returns the usage message to be printed.
 func (b *RandomQuery) Usage() string {
 	return `
 random-query constructs random queries
 
+Agent number modifies the random seed.
+
 Usage: random-query [arguments]
 
 The following arguments are available:
@@ -66,6 +70,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *RandomQuery) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("RandomQuery", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
diff --git a/bench/sliceheight.go b/bench/sliceheight.go
index abed6675b..dbfdd144f 100644
--- a/bench/sliceheight.go
+++ b/bench/sliceheight.go
@@ -12,6 +12,8 @@ import (
 	"github.com/pilosa/pilosa/pql"
 )
 
+// NewSliceHeight creates a new slice height benchmark with stdin/out/err
+// initialized.
 func NewSliceHeight(stdin io.Reader, stdout, stderr io.Writer) *SliceHeight {
 	return &SliceHeight{
 		Stdin:  stdin,
@@ -38,10 +40,13 @@ type SliceHeight struct {
 	Stderr io.Writer `json:"-"`
 }
 
+// Usage returns the usage message to be printed.
 func (b *SliceHeight) Usage() string {
 	return `
 slice-height repeatedly imports more bitmaps into a single slice and tests query times in between.
 
+Agent number has no effect on this benchmark.
+
 Usage: slice-height [arguments]
 
 The following arguments are available:
@@ -66,6 +71,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *SliceHeight) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("SliceHeight", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
@@ -84,6 +92,7 @@ func (b *SliceHeight) ConsumeFlags(args []string) ([]string, error) {
 	return fs.Args(), nil
 }
 
+// Init sets up the slice height benchmark.
 func (b *SliceHeight) Init(hosts []string, agentNum int) error {
 	b.Name = "slice-height"
 	b.hosts = hosts
diff --git a/bench/stats.go b/bench/stats.go
index de0bf34a3..7a32987f9 100644
--- a/bench/stats.go
+++ b/bench/stats.go
@@ -5,6 +5,7 @@ import (
 	"time"
 )
 
+// Stats object helps track timing stats.
 type Stats struct {
 	Min            time.Duration
 	Max            time.Duration
@@ -16,6 +17,7 @@ type Stats struct {
 	SaveAll        bool
 }
 
+// NewStats gets a Stats object.
 func NewStats() *Stats {
 	return &Stats{
 		Min: 1<<63 - 1,
@@ -23,6 +25,7 @@ func NewStats() *Stats {
 	}
 }
 
+// Add adds a new time to the stats object.
 func (s *Stats) Add(td time.Duration) {
 	if s.SaveAll {
 		s.All = append(s.All, td)
@@ -43,10 +46,13 @@ func (s *Stats) Add(td time.Duration) {
 	s.sumSquareDelta += float64(delta * (td - s.Mean))
 }
 
+// Avg returns the average of all durations Added to the Stats object.
 func (s *Stats) Avg() time.Duration {
 	return s.Total / time.Duration(s.Num)
 }
 
+// AddToResults serializes the summary of Stats and adds them to the results
+// map.
 func AddToResults(s *Stats, results map[string]interface{}) {
 	results["min"] = s.Min
 	results["max"] = s.Max
diff --git a/bench/zipf.go b/bench/zipf.go
index 967c20331..6343cd4c6 100644
--- a/bench/zipf.go
+++ b/bench/zipf.go
@@ -35,6 +35,7 @@ type ZipfSetBits struct {
 	profilePerm     *PermutationGenerator
 }
 
+// Usage returns the usage message to be printed.
 func (b *ZipfSetBits) Usage() string {
 	return `
 zipf-set-bits sets random bits according to the Zipf distribution.
@@ -44,6 +45,8 @@ the "sharpness" of the distribution, with higher exponent being sharper.
 Ratio, in the range (0, 1), with a default value of 0.25, controls the
 maximum variation of the distribution, with higher ratio being more uniform.
 
+Agent number modifies random seed.
+
 Usage: zipf-set-bits [arguments]
 
 The following arguments are available:
@@ -86,6 +89,9 @@ The following arguments are available:
 `[1:]
 }
 
+// ConsumeFlags parses all flags up to the next non flag argument (argument does
+// not start with "-" and isn't the value of a flag). It returns the remaining
+// args.
 func (b *ZipfSetBits) ConsumeFlags(args []string) ([]string, error) {
 	fs := flag.NewFlagSet("ZipfSetBits", flag.ContinueOnError)
 	fs.SetOutput(ioutil.Discard)
@@ -120,6 +126,8 @@ func getZipfOffset(N int64, exp, ratio float64) float64 {
 	return z * float64(N-1) / (1 - z)
 }
 
+// Init sets up the benchmark based on the agent number and initializes the
+// client.
 func (b *ZipfSetBits) Init(hosts []string, agentNum int) error {
 	b.Name = "zipf-set-bits"
 	b.Seed = b.Seed + int64(agentNum)