From 52ba336461be7d5ec2f4caba98cf4b38ce05ddda Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Thu, 20 Sep 2018 16:05:28 -0500
Subject: [PATCH] Address code review feedback

---
 ctl/server.go         |  2 +-
 docs/configuration.md | 12 ++++++++++++
 holder.go             | 23 ++---------------------
 server.go             |  9 ++-------
 server/server.go      |  6 ++----
 test/pilosa.go        |  6 ++----
 translate.go          |  3 +--
 7 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/ctl/server.go b/ctl/server.go
index 194633a48..7f384ec7b 100644
--- a/ctl/server.go
+++ b/ctl/server.go
@@ -45,7 +45,7 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) {
 
 	// Translation
 	flags.StringVarP(&srv.Config.Translation.PrimaryURL, "translation.primary-url", "", srv.Config.Translation.PrimaryURL, "DEPRECATED: URL for primary translation node for replication.")
-	flags.IntVarP(&srv.Config.Translation.MapSize, "translation.map-size", "", srv.Config.Translation.MapSize, "Size of mmap to allocate for key translation.")
+	flags.IntVarP(&srv.Config.Translation.MapSize, "translation.map-size", "", srv.Config.Translation.MapSize, "Size in bytes of mmap to allocate for key translation.")
 
 	// Gossip
 	flags.StringVarP(&srv.Config.Gossip.Port, "gossip.port", "", srv.Config.Gossip.Port, "Port to which pilosa should bind for internal state sharing.")
diff --git a/docs/configuration.md b/docs/configuration.md
index 22a67ddcf..5750441ef 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -307,6 +307,18 @@ The config file is in the [toml format](https://github.com/toml-lang/toml) and h
     skip-verify = true
     ```
 
+#### Translation Map Size
+
+* Description: Size in bytes of mmap to allocate for key translation
+* Flag: `translation.map-size`
+* Env: `PILOSA_TRANSLATION_MAP_SIZE`
+* Config:
+
+    ```toml
+    [translation]
+    map-size = 10737418240
+    ```
+
 ### Example Cluster Configuration
 
 A three node cluster running on different hosts could be minimally configured as follows:
diff --git a/holder.go b/holder.go
index 5cffeea9b..794104e53 100644
--- a/holder.go
+++ b/holder.go
@@ -77,19 +77,9 @@ type Holder struct {
 	Logger Logger
 }
 
-// HolderOption is a functional option type for pilosa.Holder
-type HolderOption func(f *Holder) error
-
-func OptHolderTranslateFileMapSize(mapSize int) HolderOption {
-	return func(h *Holder) error {
-		h.translateFile = NewTranslateFile(OptTranslateFileMapSize(mapSize))
-		return nil
-	}
-}
-
 // NewHolder returns a new instance of Holder.
-func NewHolder(opts ...HolderOption) *Holder {
-	h := &Holder{
+func NewHolder() *Holder {
+	return &Holder{
 		indexes: make(map[string]*Index),
 		closing: make(chan struct{}),
 
@@ -107,15 +97,6 @@ func NewHolder(opts ...HolderOption) *Holder {
 
 		Logger: NopLogger,
 	}
-
-	for _, opt := range opts {
-		err := opt(h)
-		if err != nil {
-			// TODO (2.0): Change func signature to return error
-			panic(errors.Wrap(err, "applying option"))
-		}
-	}
-	return h
 }
 
 // Open initializes the root data directory for the holder.
diff --git a/server.go b/server.go
index 708fd853b..ce6319f45 100644
--- a/server.go
+++ b/server.go
@@ -234,14 +234,9 @@ func OptServerClusterHasher(h Hasher) ServerOption {
 	}
 }
 
-func OptServerHolderOptions(opts ...HolderOption) ServerOption {
+func OptServerTranslateFileMapSize(mapSize int) ServerOption {
 	return func(s *Server) error {
-		for _, opt := range opts {
-			err := opt(s.holder)
-			if err != nil {
-				return errors.Wrap(err, "applying option")
-			}
-		}
+		s.holder.translateFile = NewTranslateFile(OptTranslateFileMapSize(mapSize))
 		return nil
 	}
 }
diff --git a/server/server.go b/server/server.go
index 61942522f..b4ac595fa 100644
--- a/server/server.go
+++ b/server/server.go
@@ -286,10 +286,8 @@ func (m *Command) SetupServer() error {
 	if m.Config.Translation.MapSize > 0 {
 		serverOptions = append(
 			serverOptions,
-			pilosa.OptServerHolderOptions(
-				pilosa.OptHolderTranslateFileMapSize(
-					m.Config.Translation.MapSize,
-				),
+			pilosa.OptServerTranslateFileMapSize(
+				m.Config.Translation.MapSize,
 			),
 		)
 	}
diff --git a/test/pilosa.go b/test/pilosa.go
index a2e6c1c58..f8f60eebe 100644
--- a/test/pilosa.go
+++ b/test/pilosa.go
@@ -64,10 +64,8 @@ func newCommand(opts ...server.CommandOption) *Command {
 	opts = append([]server.CommandOption{
 		server.OptCommandCloseTimeout(time.Millisecond * 2),
 		server.OptCommandServerOptions(
-			pilosa.OptServerHolderOptions(
-				pilosa.OptHolderTranslateFileMapSize(
-					2 << 25,
-				),
+			pilosa.OptServerTranslateFileMapSize(
+				2 << 25,
 			),
 		),
 	}, opts...)
diff --git a/translate.go b/translate.go
index fc4a9d3c7..2ef289f45 100644
--- a/translate.go
+++ b/translate.go
@@ -93,7 +93,7 @@ func OptTranslateFileMapSize(mapSize int) TranslateFileOption {
 
 // NewTranslateFile returns a new instance of TranslateFile.
 func NewTranslateFile(opts ...TranslateFileOption) *TranslateFile {
-	var defaultMapSize64 int64 = 1 << 33
+	var defaultMapSize64 int64 = 10 * (1 << 30)
 	var defaultMapSize int
 
 	if ^uint(0)>>32 > 0 {
@@ -103,7 +103,6 @@ func NewTranslateFile(opts ...TranslateFileOption) *TranslateFile {
 		// Use 2GB default map size on 32-bit systems
 		defaultMapSize = (1 << 31) - 1
 	}
-
 	f := &TranslateFile{
 		writeNotify: make(chan struct{}),
 		closing:     make(chan struct{}),