From e94bab354b7251b1e35a9c296e6b88ec32962580 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 16 Nov 2021 21:03:37 -0800 Subject: [PATCH 1/7] Add InitialMmapSize to bolt options --- physical/raft/bolt_386_test.go | 45 ++++++++++++++++++++++++++++ physical/raft/bolt_amd64_test.go | 45 ++++++++++++++++++++++++++++ physical/raft/fsm.go | 11 ++++--- physical/raft/raft.go | 50 ++++++++++++++++++++++---------- 4 files changed, 130 insertions(+), 21 deletions(-) create mode 100644 physical/raft/bolt_386_test.go create mode 100644 physical/raft/bolt_amd64_test.go diff --git a/physical/raft/bolt_386_test.go b/physical/raft/bolt_386_test.go new file mode 100644 index 000000000000..32f9ea661d06 --- /dev/null +++ b/physical/raft/bolt_386_test.go @@ -0,0 +1,45 @@ +package raft + +import ( + "os" + "strconv" + "testing" +) + +func Test_BoltOptions(t *testing.T) { + t.Parallel() + key := "VAULT_RAFT_INITIAL_MMAP_SIZE" + + testCases := []struct { + name string + env string + expectedSize int + }{ + {"none", "", 0}, + {"5MB", strconv.Itoa(5 * 1024 * 1024), 5 * 1024 * 1024}, + {"negative", "-1", 0}, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if tc.env != "" { + current := os.Getenv(key) + defer os.Setenv(key, current) + os.Setenv(key, tc.env) + } + + o, err := boltOptions() + if err != nil { + t.Error(err) + } + + if o.InitialMmapSize != tc.expectedSize { + t.Errorf("expected InitialMmapSize to be %d but it was %d", tc.expectedSize, o.InitialMmapSize) + } + }) + } +} diff --git a/physical/raft/bolt_amd64_test.go b/physical/raft/bolt_amd64_test.go new file mode 100644 index 000000000000..0978ffebc29e --- /dev/null +++ b/physical/raft/bolt_amd64_test.go @@ -0,0 +1,45 @@ +package raft + +import ( + "os" + "strconv" + "testing" +) + +func Test_BoltOptions(t *testing.T) { + t.Parallel() + key := "VAULT_RAFT_INITIAL_MMAP_SIZE" + + testCases := []struct { + name string + env string + expectedSize int + }{ + {"none", "", 100 * 1024 * 1024 * 1024}, + {"5MB", strconv.Itoa(5 * 1024 * 1024), 5 * 1024 * 1024}, + {"negative", "-1", 0}, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if tc.env != "" { + current := os.Getenv(key) + defer os.Setenv(key, current) + os.Setenv(key, tc.env) + } + + o, err := boltOptions() + if err != nil { + t.Error(err) + } + + if o.InitialMmapSize != tc.expectedSize { + t.Errorf("expected InitialMmapSize to be %d but it was %d", tc.expectedSize, o.InitialMmapSize) + } + }) + } +} diff --git a/physical/raft/fsm.go b/physical/raft/fsm.go index 66b2b4b9c563..53d0f1cc47a6 100644 --- a/physical/raft/fsm.go +++ b/physical/raft/fsm.go @@ -168,13 +168,12 @@ func (f *FSM) openDBFile(dbPath string) error { } } - freelistType, noFreelistSync := freelistOptions() + opts, err := boltOptions() + if err != nil { + return err + } start := time.Now() - boltDB, err := bolt.Open(dbPath, 0o600, &bolt.Options{ - Timeout: 1 * time.Second, - FreelistType: freelistType, - NoFreelistSync: noFreelistSync, - }) + boltDB, err := bolt.Open(dbPath, 0o600, opts) if err != nil { return err } diff --git a/physical/raft/raft.go b/physical/raft/raft.go index 47b96d7c0dcb..1bfead7397c3 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -364,13 +364,13 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend } // Create the backend raft store for logs and stable storage. - freelistType, noFreelistSync := freelistOptions() + opts, err := boltOptions() + if err != nil { + return nil, err + } raftOptions := raftboltdb.Options{ - Path: filepath.Join(path, "raft.db"), - BoltOptions: &bolt.Options{ - FreelistType: freelistType, - NoFreelistSync: noFreelistSync, - }, + Path: filepath.Join(path, "raft.db"), + BoltOptions: opts, } store, err := raftboltdb.New(raftOptions) if err != nil { @@ -1644,20 +1644,40 @@ func (s sealer) Open(ctx context.Context, ct []byte) ([]byte, error) { return s.access.Decrypt(ctx, &eblob, nil) } -// freelistOptions returns the freelist type and nofreelistsync values to use -// when opening boltdb files, based on our preferred defaults, and the possible -// presence of overriding environment variables. -func freelistOptions() (bolt.FreelistType, bool) { - freelistType := bolt.FreelistMapType - noFreelistSync := true +// boltOptions returns a bolt.Options struct, suitable for passing to +// bolt.Open(), pre-configured with all of our preferred defaults. +func boltOptions() (*bolt.Options, error) { + o := &bolt.Options{ + Timeout: 1 * time.Second, + FreelistType: bolt.FreelistMapType, + NoFreelistSync: true, + } if os.Getenv("VAULT_RAFT_FREELIST_TYPE") == "array" { - freelistType = bolt.FreelistArrayType + o.FreelistType = bolt.FreelistArrayType } if os.Getenv("VAULT_RAFT_FREELIST_SYNC") != "" { - noFreelistSync = false + o.NoFreelistSync = false + } + + // By default, we want to set InitialMmapSize to 100GB, but only on 64bit platforms. + // Otherwise, we set it to whatever the value of VAULT_RAFT_INITIAL_MMAP_SIZE + // is, assuming it can be parsed as an int. Bolt itself sets this to 0 by default, + // so if users are wanting to turn this off, they can also set it to 0. Setting it + // to a negative value is the same as not setting it at all. + if os.Getenv("VAULT_RAFT_INITIAL_MMAP_SIZE") == "" { + if strconv.IntSize == 64 { + o.InitialMmapSize = 100 * 1024 * 1024 * 1024 + } + } else { + imms, err := strconv.Atoi(os.Getenv("VAULT_RAFT_INITIAL_MMAP_SIZE")) + if err != nil { + return nil, err + } else if imms >= 0 { + o.InitialMmapSize = imms + } } - return freelistType, noFreelistSync + return o, nil } From 8d1a32deffb85e1ff848695edd040a41cdf0e272 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 17 Nov 2021 12:05:10 -0800 Subject: [PATCH 2/7] Add 32 and 64bit specific sizes for InitialMmapSize --- physical/raft/raft.go | 4 +--- physical/raft/vars_32bit.go | 5 +++++ physical/raft/vars_64bit.go | 5 +++++ 3 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 physical/raft/vars_32bit.go create mode 100644 physical/raft/vars_64bit.go diff --git a/physical/raft/raft.go b/physical/raft/raft.go index 1bfead7397c3..b7e98bbb87a1 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -1667,9 +1667,7 @@ func boltOptions() (*bolt.Options, error) { // so if users are wanting to turn this off, they can also set it to 0. Setting it // to a negative value is the same as not setting it at all. if os.Getenv("VAULT_RAFT_INITIAL_MMAP_SIZE") == "" { - if strconv.IntSize == 64 { - o.InitialMmapSize = 100 * 1024 * 1024 * 1024 - } + o.InitialMmapSize = initialMmapSize } else { imms, err := strconv.Atoi(os.Getenv("VAULT_RAFT_INITIAL_MMAP_SIZE")) if err != nil { diff --git a/physical/raft/vars_32bit.go b/physical/raft/vars_32bit.go new file mode 100644 index 000000000000..27deb5887324 --- /dev/null +++ b/physical/raft/vars_32bit.go @@ -0,0 +1,5 @@ +// +build 386 arm + +package raft + +const initialMmapSize = 0 diff --git a/physical/raft/vars_64bit.go b/physical/raft/vars_64bit.go new file mode 100644 index 000000000000..61befb13fbd5 --- /dev/null +++ b/physical/raft/vars_64bit.go @@ -0,0 +1,5 @@ +// +build amd64 arm64 s390x + +package raft + +const initialMmapSize = 100 * 1024 * 1024 * 1024 // 100GB From cb77da051042e9be0d71525288c171b40225249d Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 18 Nov 2021 11:26:47 -0800 Subject: [PATCH 3/7] correct build tags and file names --- physical/raft/{bolt_386_test.go => bolt_32bit_test.go} | 2 ++ physical/raft/{bolt_amd64_test.go => bolt_64bit_test.go} | 2 ++ physical/raft/vars_64bit.go | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) rename physical/raft/{bolt_386_test.go => bolt_32bit_test.go} (97%) rename physical/raft/{bolt_amd64_test.go => bolt_64bit_test.go} (97%) diff --git a/physical/raft/bolt_386_test.go b/physical/raft/bolt_32bit_test.go similarity index 97% rename from physical/raft/bolt_386_test.go rename to physical/raft/bolt_32bit_test.go index 32f9ea661d06..253b57aa6ea9 100644 --- a/physical/raft/bolt_386_test.go +++ b/physical/raft/bolt_32bit_test.go @@ -1,3 +1,5 @@ +// +build 386 arm + package raft import ( diff --git a/physical/raft/bolt_amd64_test.go b/physical/raft/bolt_64bit_test.go similarity index 97% rename from physical/raft/bolt_amd64_test.go rename to physical/raft/bolt_64bit_test.go index 0978ffebc29e..fce6439cdff1 100644 --- a/physical/raft/bolt_amd64_test.go +++ b/physical/raft/bolt_64bit_test.go @@ -1,3 +1,5 @@ +// +build !386,!arm + package raft import ( diff --git a/physical/raft/vars_64bit.go b/physical/raft/vars_64bit.go index 61befb13fbd5..1a1716cd3000 100644 --- a/physical/raft/vars_64bit.go +++ b/physical/raft/vars_64bit.go @@ -1,4 +1,4 @@ -// +build amd64 arm64 s390x +// +build !386,!arm package raft From 6e6a1a418a93bbd31d66c0441ad3e0f780ac7f16 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 18 Nov 2021 14:57:48 -0800 Subject: [PATCH 4/7] Stop returning errors from boltOptions() --- physical/raft/fsm.go | 5 +---- physical/raft/raft.go | 16 +++++++--------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/physical/raft/fsm.go b/physical/raft/fsm.go index 53d0f1cc47a6..8c2e79fbde15 100644 --- a/physical/raft/fsm.go +++ b/physical/raft/fsm.go @@ -168,10 +168,7 @@ func (f *FSM) openDBFile(dbPath string) error { } } - opts, err := boltOptions() - if err != nil { - return err - } + opts := boltOptions() start := time.Now() boltDB, err := bolt.Open(dbPath, 0o600, opts) if err != nil { diff --git a/physical/raft/raft.go b/physical/raft/raft.go index b7e98bbb87a1..8c59fd07ae9c 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -364,10 +364,7 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend } // Create the backend raft store for logs and stable storage. - opts, err := boltOptions() - if err != nil { - return nil, err - } + opts := boltOptions() raftOptions := raftboltdb.Options{ Path: filepath.Join(path, "raft.db"), BoltOptions: opts, @@ -1646,7 +1643,7 @@ func (s sealer) Open(ctx context.Context, ct []byte) ([]byte, error) { // boltOptions returns a bolt.Options struct, suitable for passing to // bolt.Open(), pre-configured with all of our preferred defaults. -func boltOptions() (*bolt.Options, error) { +func boltOptions() *bolt.Options { o := &bolt.Options{ Timeout: 1 * time.Second, FreelistType: bolt.FreelistMapType, @@ -1670,12 +1667,13 @@ func boltOptions() (*bolt.Options, error) { o.InitialMmapSize = initialMmapSize } else { imms, err := strconv.Atoi(os.Getenv("VAULT_RAFT_INITIAL_MMAP_SIZE")) - if err != nil { - return nil, err - } else if imms >= 0 { + + // If there's an error here, it means they passed something that's not convertible to + // a number. Rather than fail startup, just ignore it. + if err == nil && imms > 0 { o.InitialMmapSize = imms } } - return o, nil + return o } From 4e6816e40297c02a1ed50f8841fe7e386f4071db Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 18 Nov 2021 15:06:24 -0800 Subject: [PATCH 5/7] add changelog --- changelog/13178.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/13178.txt diff --git a/changelog/13178.txt b/changelog/13178.txt new file mode 100644 index 000000000000..10bb3b890dae --- /dev/null +++ b/changelog/13178.txt @@ -0,0 +1,3 @@ +```release-note:improvement +raft: set InitialMmapSize to 100GB on 64bit architectures +``` From dacde22629efd3414455f9a28f7135550d2d7d14 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 22 Nov 2021 09:55:30 -0800 Subject: [PATCH 6/7] fixing tests --- physical/raft/bolt_64bit_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/physical/raft/bolt_64bit_test.go b/physical/raft/bolt_64bit_test.go index fce6439cdff1..f6a1cbd72594 100644 --- a/physical/raft/bolt_64bit_test.go +++ b/physical/raft/bolt_64bit_test.go @@ -34,10 +34,7 @@ func Test_BoltOptions(t *testing.T) { os.Setenv(key, tc.env) } - o, err := boltOptions() - if err != nil { - t.Error(err) - } + o := boltOptions() if o.InitialMmapSize != tc.expectedSize { t.Errorf("expected InitialMmapSize to be %d but it was %d", tc.expectedSize, o.InitialMmapSize) From b2291ab26fc770bb94b45042c1d9b50edbdbc3c4 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 22 Nov 2021 11:33:29 -0800 Subject: [PATCH 7/7] parallel setting of env vars is bad --- physical/raft/bolt_32bit_test.go | 2 -- physical/raft/bolt_64bit_test.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/physical/raft/bolt_32bit_test.go b/physical/raft/bolt_32bit_test.go index 253b57aa6ea9..57ab4cee96bb 100644 --- a/physical/raft/bolt_32bit_test.go +++ b/physical/raft/bolt_32bit_test.go @@ -26,8 +26,6 @@ func Test_BoltOptions(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if tc.env != "" { current := os.Getenv(key) defer os.Setenv(key, current) diff --git a/physical/raft/bolt_64bit_test.go b/physical/raft/bolt_64bit_test.go index f6a1cbd72594..20ec4ea43f0b 100644 --- a/physical/raft/bolt_64bit_test.go +++ b/physical/raft/bolt_64bit_test.go @@ -26,8 +26,6 @@ func Test_BoltOptions(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if tc.env != "" { current := os.Getenv(key) defer os.Setenv(key, current)