From 349458b8f5b1c9fde18e7182b75c39c2cc4f2f85 Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Fri, 21 Jun 2024 16:23:16 -0700 Subject: [PATCH 1/7] add warning for when MAP_POPULATE mmap flag not set --- physical/raft/fsm.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/physical/raft/fsm.go b/physical/raft/fsm.go index a0197cdaa6a2..635ca75697d1 100644 --- a/physical/raft/fsm.go +++ b/physical/raft/fsm.go @@ -31,6 +31,7 @@ import ( "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/sdk/plugin/pb" + "golang.org/x/sys/unix" ) const ( @@ -260,6 +261,10 @@ func (f *FSM) openDBFile(dbPath string) error { } opts := boltOptions(dbPath) + if opts.MmapFlags != unix.MAP_POPULATE { + f.logger.Warn("the MAP_POPULATE mmap flag was not used. This may be due to Vault's database file being larger than the available memory on the system, or due to the VAULT_RAFT_DISABLE_MAP_POPULATE environment variable being set. As a result, Vault may be slower to start up.") + } + start := time.Now() boltDB, err := bolt.Open(dbPath, 0o600, opts) if err != nil { From 035008528db68dcc711c28316e8b448071ebe259 Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Thu, 22 Aug 2024 09:09:59 -0700 Subject: [PATCH 2/7] Make mmap flags method handle any flags, where MAP_POPULATE is just one of them --- physical/raft/bolt_linux.go | 32 +++++++++++++++++++++++++++----- physical/raft/fsm.go | 4 ++-- physical/raft/raft.go | 5 ++++- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/physical/raft/bolt_linux.go b/physical/raft/bolt_linux.go index 811c148a1050..d38fd9f9705d 100644 --- a/physical/raft/bolt_linux.go +++ b/physical/raft/bolt_linux.go @@ -13,27 +13,49 @@ import ( func init() { getMmapFlags = getMmapFlagsLinux + usingMapPopulate = usingMapPopulateLinux } func getMmapFlagsLinux(dbPath string) int { + if setMapPopulateFlag(dbPath) { + return unix.MAP_POPULATE + } + + return 0 +} + +// setMapPopulateFlag determines whether we should set the MAP_POPULATE flag, which +// prepopulates page tables to be mapped in the virtual memory space, +// helping reduce slowness at runtime caused by page faults. +// We only want to set this flag if we've determined there's enough memory on the system available to do so. +func setMapPopulateFlag(dbPath string) bool { if os.Getenv("VAULT_RAFT_DISABLE_MAP_POPULATE") != "" { - return 0 + return false } stat, err := os.Stat(dbPath) if err != nil { - return 0 + return false } size := stat.Size() v, err := mem.VirtualMemoryWithContext(context.Background()) if err != nil { - return 0 + return false } // We won't worry about swap, since we already tell people not to use it. if v.Total > uint64(size) { - return unix.MAP_POPULATE + return true } - return 0 + return false +} + +// the unix.MAP_POPULATE constant only exists on Linux, +// so reference to this constant can only live in a *_linux.go file +func usingMapPopulateLinux(mmapFlags int) bool { + if mmapFlags == unix.MAP_POPULATE { + return true + } + return false } diff --git a/physical/raft/fsm.go b/physical/raft/fsm.go index 635ca75697d1..fe1277da57b9 100644 --- a/physical/raft/fsm.go +++ b/physical/raft/fsm.go @@ -13,6 +13,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strconv" "strings" "sync" @@ -31,7 +32,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/sdk/plugin/pb" - "golang.org/x/sys/unix" ) const ( @@ -261,7 +261,7 @@ func (f *FSM) openDBFile(dbPath string) error { } opts := boltOptions(dbPath) - if opts.MmapFlags != unix.MAP_POPULATE { + if runtime.GOOS == "linux" && !usingMapPopulate(opts.MmapFlags) { f.logger.Warn("the MAP_POPULATE mmap flag was not used. This may be due to Vault's database file being larger than the available memory on the system, or due to the VAULT_RAFT_DISABLE_MAP_POPULATE environment variable being set. As a result, Vault may be slower to start up.") } diff --git a/physical/raft/raft.go b/physical/raft/raft.go index d07dedea4042..e6fc4ab397b6 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -74,7 +74,10 @@ const ( defaultMaxBatchSize = 128 * 1024 ) -var getMmapFlags = func(string) int { return 0 } +var ( + getMmapFlags = func(string) int { return 0 } + usingMapPopulate = func(int) bool { return false } +) // Verify RaftBackend satisfies the correct interfaces var ( From 2bf5681a6b6e0b31917a5f5d280e7f8b81f0ecf7 Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Wed, 18 Sep 2024 09:45:07 -0700 Subject: [PATCH 3/7] Only have the log print out on restores --- physical/raft/bolt_linux.go | 4 ++-- physical/raft/fsm.go | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/physical/raft/bolt_linux.go b/physical/raft/bolt_linux.go index d38fd9f9705d..bec6f7e386c7 100644 --- a/physical/raft/bolt_linux.go +++ b/physical/raft/bolt_linux.go @@ -53,8 +53,8 @@ func setMapPopulateFlag(dbPath string) bool { // the unix.MAP_POPULATE constant only exists on Linux, // so reference to this constant can only live in a *_linux.go file -func usingMapPopulateLinux(mmapFlags int) bool { - if mmapFlags == unix.MAP_POPULATE { +func usingMapPopulateLinux(mmapFlag int) bool { + if mmapFlag == unix.MAP_POPULATE { return true } return false diff --git a/physical/raft/fsm.go b/physical/raft/fsm.go index fe1277da57b9..b353313f84d3 100644 --- a/physical/raft/fsm.go +++ b/physical/raft/fsm.go @@ -243,6 +243,8 @@ func (r *RaftBackend) SetFSMApplyCallback(f func()) { } func (f *FSM) openDBFile(dbPath string) error { + var isNewDB bool + if len(dbPath) == 0 { return errors.New("can not open empty filename") } @@ -250,6 +252,7 @@ func (f *FSM) openDBFile(dbPath string) error { st, err := os.Stat(dbPath) switch { case err != nil && os.IsNotExist(err): + isNewDB = true case err != nil: return fmt.Errorf("error checking raft FSM db file %q: %v", dbPath, err) default: @@ -261,7 +264,8 @@ func (f *FSM) openDBFile(dbPath string) error { } opts := boltOptions(dbPath) - if runtime.GOOS == "linux" && !usingMapPopulate(opts.MmapFlags) { + if runtime.GOOS == "linux" && !isNewDB && !usingMapPopulate(opts.MmapFlags) { + // we're on linux, doing a snapshot restore, and MAP_POPULATE is not set f.logger.Warn("the MAP_POPULATE mmap flag was not used. This may be due to Vault's database file being larger than the available memory on the system, or due to the VAULT_RAFT_DISABLE_MAP_POPULATE environment variable being set. As a result, Vault may be slower to start up.") } @@ -270,6 +274,7 @@ func (f *FSM) openDBFile(dbPath string) error { if err != nil { return err } + elapsed := time.Now().Sub(start) f.logger.Debug("time to open database", "elapsed", elapsed, "path", dbPath) metrics.MeasureSince([]string{"raft_storage", "fsm", "open_db_file"}, start) From ef870726904276cd74af36303ec29deb62be90be Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Thu, 26 Sep 2024 12:47:07 -0700 Subject: [PATCH 4/7] Add test, make logic more consistent --- physical/raft/bolt_64bit_test.go | 27 +++++++++++++++++++++++++++ physical/raft/fsm.go | 10 ++++------ physical/raft/raft.go | 7 +++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/physical/raft/bolt_64bit_test.go b/physical/raft/bolt_64bit_test.go index f6f1bbd2dee3..283551c547c7 100644 --- a/physical/raft/bolt_64bit_test.go +++ b/physical/raft/bolt_64bit_test.go @@ -43,3 +43,30 @@ func Test_BoltOptions(t *testing.T) { }) } } + +func TestMmapFlags(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + disableMapPopulate bool + }{ + {"MAP_POPULATE is enabled", false}, + {"MAP_POPULATE disabled by env var", true}, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + if tc.disableMapPopulate { + os.Setenv("VAULT_RAFT_DISABLE_MAP_POPULATE", "true") + } + + isEnabled := usingMapPopulate(getMmapFlags("")) + if tc.disableMapPopulate && isEnabled { + t.Error("expected MAP_POPULATE to be disabled but it was enabled") + } + }) + } +} diff --git a/physical/raft/fsm.go b/physical/raft/fsm.go index b353313f84d3..cfbe8374aaff 100644 --- a/physical/raft/fsm.go +++ b/physical/raft/fsm.go @@ -243,16 +243,15 @@ func (r *RaftBackend) SetFSMApplyCallback(f func()) { } func (f *FSM) openDBFile(dbPath string) error { - var isNewDB bool - if len(dbPath) == 0 { return errors.New("can not open empty filename") } + vaultDbExists := true st, err := os.Stat(dbPath) switch { case err != nil && os.IsNotExist(err): - isNewDB = true + vaultDbExists = false case err != nil: return fmt.Errorf("error checking raft FSM db file %q: %v", dbPath, err) default: @@ -264,9 +263,8 @@ func (f *FSM) openDBFile(dbPath string) error { } opts := boltOptions(dbPath) - if runtime.GOOS == "linux" && !isNewDB && !usingMapPopulate(opts.MmapFlags) { - // we're on linux, doing a snapshot restore, and MAP_POPULATE is not set - f.logger.Warn("the MAP_POPULATE mmap flag was not used. This may be due to Vault's database file being larger than the available memory on the system, or due to the VAULT_RAFT_DISABLE_MAP_POPULATE environment variable being set. As a result, Vault may be slower to start up.") + if runtime.GOOS == "linux" && vaultDbExists && !usingMapPopulate(opts.MmapFlags) { + f.logger.Warn("the MAP_POPULATE mmap flag has not been set before opening the FSM database. This may be due to the database file being larger than the available memory on the system, or due to the VAULT_RAFT_DISABLE_MAP_POPULATE environment variable being set. As a result, Vault may be slower to start up.") } start := time.Now() diff --git a/physical/raft/raft.go b/physical/raft/raft.go index e6fc4ab397b6..63af2ffa012a 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -14,6 +14,7 @@ import ( "net/url" "os" "path/filepath" + "runtime" "strconv" "sync" "sync/atomic" @@ -450,6 +451,7 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend } } + // Create the log store. // Build an all in-memory setup for dev mode, otherwise prepare a full // disk-based setup. var logStore raft.LogStore @@ -476,6 +478,7 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend if err != nil { return nil, fmt.Errorf("failed to check if raft.db already exists: %w", err) } + if backendConfig.RaftWal && raftDbExists { logger.Warn("raft is configured to use raft-wal for storage but existing raft.db detected. raft-wal config will be ignored.") backendConfig.RaftWal = false @@ -507,6 +510,10 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend MsgpackUseNewTimeFormat: true, } + if runtime.GOOS == "linux" && raftDbExists && !usingMapPopulate(opts.MmapFlags) { + logger.Warn("the MAP_POPULATE mmap flag has not been set before opening the log store database. This may be due to the database file being larger than the available memory on the system, or due to the VAULT_RAFT_DISABLE_MAP_POPULATE environment variable being set. As a result, Vault may be slower to start up.") + } + store, err := raftboltdb.New(raftOptions) if err != nil { return nil, err From 8f31393576bbbe8b2279d7eb8f29ac199f16b664 Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Thu, 26 Sep 2024 15:00:05 -0700 Subject: [PATCH 5/7] Add changelog --- changelog/28526.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/28526.txt diff --git a/changelog/28526.txt b/changelog/28526.txt new file mode 100644 index 000000000000..b22b093efc9c --- /dev/null +++ b/changelog/28526.txt @@ -0,0 +1,3 @@ +```release-note:improvement +physical/raft: Log when the MAP_POPULATE mmap flag gets disabled before opening the database. +``` \ No newline at end of file From a950db8ff527b278e441e2da0aaec257ad7189c8 Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Thu, 26 Sep 2024 15:40:52 -0700 Subject: [PATCH 6/7] Add godoc for test --- physical/raft/bolt_64bit_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/physical/raft/bolt_64bit_test.go b/physical/raft/bolt_64bit_test.go index 283551c547c7..9683b4ef0946 100644 --- a/physical/raft/bolt_64bit_test.go +++ b/physical/raft/bolt_64bit_test.go @@ -44,6 +44,7 @@ func Test_BoltOptions(t *testing.T) { } } +// TestMmapFlags tests the getMmapFlags function, ensuring it returns the appropriate integer representing the desired mmap flag. func TestMmapFlags(t *testing.T) { t.Parallel() From 6fb7349ab2334ce05916301f25d1aa07323993fa Mon Sep 17 00:00:00 2001 From: Val Conklin Date: Tue, 1 Oct 2024 14:26:51 -0700 Subject: [PATCH 7/7] Make test less dangerous --- physical/raft/bolt_64bit_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/physical/raft/bolt_64bit_test.go b/physical/raft/bolt_64bit_test.go index 9683b4ef0946..3305d450dd6d 100644 --- a/physical/raft/bolt_64bit_test.go +++ b/physical/raft/bolt_64bit_test.go @@ -46,8 +46,6 @@ func Test_BoltOptions(t *testing.T) { // TestMmapFlags tests the getMmapFlags function, ensuring it returns the appropriate integer representing the desired mmap flag. func TestMmapFlags(t *testing.T) { - t.Parallel() - testCases := []struct { name string disableMapPopulate bool @@ -61,7 +59,7 @@ func TestMmapFlags(t *testing.T) { t.Run(tc.name, func(t *testing.T) { if tc.disableMapPopulate { - os.Setenv("VAULT_RAFT_DISABLE_MAP_POPULATE", "true") + t.Setenv("VAULT_RAFT_DISABLE_MAP_POPULATE", "true") } isEnabled := usingMapPopulate(getMmapFlags(""))