Repository: nats-io/nats-streaming-server
Pull request #1159 (2021-02-18 20:37:19)

[ADDED] Configuration parameters for RAFT's BoltDB store
This replaces PRs #1155 and #1156.

In 1155, I made boltdb's default freelist type to be map instead
of array, and in 1156 added options to enable array type.

This PR keeps the default to array and allow user to change to
map instead (so the config param is now `bolt_free_list_map` instead
of `bolt_free_list_array` that was introduced in 1156.

The reason is to minimize possible unexpected behavior in deployments
that did not have any issue.
I am not sure of all the extent (performance wise) of changing to
map instead of array and would rather it to be an explicit choice
made by users, and only if they observe a performance degradation
that can be attributed to the freelist.

We can always make it the default later.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>

coveralls: 
[![Coverage Status](https://coveralls.io/builds/37251313/badge)](https://coveralls.io/builds/37251313)

Coverage decreased (-0.04%) to 91.451% when pulling **a739667fd61bccd278086d1be91bfb3255256d0c on bolt_config** into **73f25317f5a5e7d5df5de2262f75bd9a16159a6b on master**.


Patch:
diff --git a/server/clustering.go b/server/clustering.go
index aae5000..af2675d 100644
--- a/server/clustering.go
+++ b/server/clustering.go
@@ -111,14 +111,13 @@ type ClusteringOptions struct {
 	// speed up recovery since there is no need for a full database re-sync.
 	BoltFreeListSync bool
 
-	// BoltFreeListArray sets the backend freelist type to "array".
-	// There are two options:
-	// - "array" which is simple but suffers dramatic performance degradation if database is
-	//   large and framentation in freelist is common.
-	// - "hashmap" which is faster in almost all circumstances but doesn't guarantee
-	//   that it offers the smallest page id available. In normal case it is safe.
-	// Since v0.21.0, the default is "hashmap". Set this option to "true" to use "array" instead.
-	BoltFreeListArray bool
+	// BoltFreeListMap sets the backend freelist type to use a map instead of
+	// the default array type.
+	// The "array" type (the default) is simple but suffers dramatic performance
+	// degradation if database is large and framentation in freelist is common.
+	// The "hashmap which is faster in almost all circumstances but doesn't guarantee
+	// that it offers the smallest page id available. In normal case it is safe.
+	BoltFreeListMap bool
 }
 
 // raftNode is a handle to a member in a Raft consensus group.
diff --git a/server/conf.go b/server/conf.go
index 8fe7cec..a102e91 100644
--- a/server/conf.go
+++ b/server/conf.go
@@ -356,11 +356,11 @@ func parseCluster(itf interface{}, opts *Options) error {
 				return err
 			}
 			opts.Clustering.BoltFreeListSync = v.(bool)
-		case "bolt_free_list_array":
+		case "bolt_free_list_map":
 			if err := checkType(k, reflect.Bool, v); err != nil {
 				return err
 			}
-			opts.Clustering.BoltFreeListArray = v.(bool)
+			opts.Clustering.BoltFreeListMap = v.(bool)
 		case "nodes_connections":
 			if err := checkType(k, reflect.Bool, v); err != nil {
 				return err
diff --git a/server/raft_log.go b/server/raft_log.go
index d3d637f..ad59c9c 100644
--- a/server/raft_log.go
+++ b/server/raft_log.go
@@ -68,9 +68,9 @@ func newRaftLog(log logger.Logger, fileName string, opts *Options) (*raftLog, er
 		fileName: fileName,
 		codec:    &codec.MsgpackHandle{},
 	}
-	freeListType := bolt.FreelistMapType
-	if opts.Clustering.BoltFreeListArray {
-		freeListType = bolt.FreelistArrayType
+	freeListType := bolt.FreelistArrayType
+	if opts.Clustering.BoltFreeListMap {
+		freeListType = bolt.FreelistMapType
 	}
 	dbOpts := &bolt.Options{
 		NoSync:         !opts.Clustering.Sync,


Test patch:
diff --git a/server/conf_test.go b/server/conf_test.go
index 0e81d9a..e54e163 100644
--- a/server/conf_test.go
+++ b/server/conf_test.go
@@ -278,8 +278,8 @@ func TestParseConfig(t *testing.T) {
 	if !opts.Clustering.BoltFreeListSync {
 		t.Fatal("Expected BoltFreeListSync to be true")
 	}
-	if !opts.Clustering.BoltFreeListArray {
-		t.Fatal("Expected BoltFreeListArray to be true")
+	if !opts.Clustering.BoltFreeListMap {
+		t.Fatal("Expected BoltFreeListMap to be true")
 	}
 	if !opts.Clustering.NodesConnections {
 		t.Fatal("Expected NodesConnections to be true")
@@ -515,7 +515,7 @@ func TestParseWrongTypes(t *testing.T) {
 	expectFailureFor(t, "cluster:{raft_commit_timeout:\"not_a_time\"}", wrongTimeErr)
 	expectFailureFor(t, "cluster:{allow_add_remove_node:1}", wrongTypeErr)
 	expectFailureFor(t, "cluster:{bolt_free_list_sync:123}", wrongTypeErr)
-	expectFailureFor(t, "cluster:{bolt_free_list_array:123}", wrongTypeErr)
+	expectFailureFor(t, "cluster:{bolt_free_list_map:123}", wrongTypeErr)
 	expectFailureFor(t, "sql:{driver:false}", wrongTypeErr)
 	expectFailureFor(t, "sql:{source:false}", wrongTypeErr)
 	expectFailureFor(t, "sql:{no_caching:123}", wrongTypeErr)
diff --git a/test/configs/test_parse.conf b/test/configs/test_parse.conf
index 47225b7..0dd3829 100644
--- a/test/configs/test_parse.conf
+++ b/test/configs/test_parse.conf
@@ -93,7 +93,7 @@ streaming: {
       raft_commit_timeout: "50ms"
       allow_add_remove_node: true
       bolt_free_list_sync: true
-      bolt_free_list_array: true
+      bolt_free_list_map: true
       nodes_connections: true
   }