Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

walletdb/bdb: attempt to set initial memory map size when opening #697

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/kkdai/bstream v0.0.0-20181106074824-b3251f7901ec // indirect
github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf
github.com/lightninglabs/neutrino v0.11.0
go.etcd.io/bbolt v1.3.3
golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67
golang.org/x/net v0.0.0-20190206173232-65e2d4e15006
google.golang.org/genproto v0.0.0-20190201180003-4b09977fb922 // indirect
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 h1:R8vQdOQdZ9Y3
github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY=
github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs=
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/coreos/bbolt v1.3.3 h1:n6AiVyVRKQFNb6mJlwESEvvLoDyiTzXX7ORAUlkeBdY=
github.com/coreos/bbolt v1.3.3/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495 h1:6IyqGr3fnd0tM3YxipK27TUskaOVUjU2nG45yzwcQKY=
github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
Expand Down Expand Up @@ -89,8 +87,6 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190904154756-749cb33beabd h1:DBH9mDw0zluJT/R+nGuV3jWFWLFaHyYZWD4tOT+cjn0=
Expand Down
11 changes: 9 additions & 2 deletions waddrmgr/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/btcsuite/btcd/chaincfg"
"github.com/btcsuite/btcwallet/walletdb"
_ "github.com/btcsuite/btcwallet/walletdb/bdb"
"go.etcd.io/bbolt"
)

var (
Expand Down Expand Up @@ -238,7 +239,10 @@ func emptyDB(t *testing.T) (tearDownFunc func(), db walletdb.DB) {
t.Fatalf("Failed to create db temp dir: %v", err)
}
dbPath := filepath.Join(dirName, "mgrtest.db")
db, err = walletdb.Create("bdb", dbPath, true)
opts := &bbolt.Options{
NoFreelistSync: true,
}
db, err = walletdb.Create("bdb", dbPath, opts)
if err != nil {
_ = os.RemoveAll(dirName)
t.Fatalf("createDbNamespace: unexpected error: %v", err)
Expand All @@ -259,7 +263,10 @@ func setupManager(t *testing.T) (tearDownFunc func(), db walletdb.DB, mgr *Manag
t.Fatalf("Failed to create db temp dir: %v", err)
}
dbPath := filepath.Join(dirName, "mgrtest.db")
db, err = walletdb.Create("bdb", dbPath, true)
opts := &bbolt.Options{
NoFreelistSync: true,
}
db, err = walletdb.Create("bdb", dbPath, opts)
if err != nil {
_ = os.RemoveAll(dirName)
t.Fatalf("createDbNamespace: unexpected error: %v", err)
Expand Down
6 changes: 5 additions & 1 deletion waddrmgr/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/btcsuite/btcwallet/snacl"
"github.com/btcsuite/btcwallet/walletdb"
"github.com/davecgh/go-spew/spew"
"go.etcd.io/bbolt"
)

// failingCryptoKey is an implementation of the EncryptorDecryptor interface
Expand Down Expand Up @@ -1683,7 +1684,10 @@ func testConvertWatchingOnly(tc *testContext) bool {
defer os.Remove(woMgrName)

// Open the new database copy and get the address manager namespace.
db, err := walletdb.Open("bdb", woMgrName, true)
opts := &bbolt.Options{
NoFreelistSync: true,
}
db, err := walletdb.Open("bdb", woMgrName, opts)
if err != nil {
tc.t.Errorf("openDbNamespace: unexpected error: %v", err)
return false
Expand Down
11 changes: 9 additions & 2 deletions wallet/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/btcsuite/btcwallet/internal/prompt"
"github.com/btcsuite/btcwallet/waddrmgr"
"github.com/btcsuite/btcwallet/walletdb"
"go.etcd.io/bbolt"
)

const (
Expand Down Expand Up @@ -140,7 +141,10 @@ func (l *Loader) createNewWallet(pubPassphrase, privPassphrase,
if err != nil {
return nil, err
}
db, err := walletdb.Create("bdb", dbPath, l.noFreelistSync)
opts := &bbolt.Options{
NoFreelistSync: l.noFreelistSync,
}
db, err := walletdb.Create("bdb", dbPath, opts)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -196,7 +200,10 @@ func (l *Loader) OpenExistingWallet(pubPassphrase []byte, canConsolePrompt bool)

// Open the database using the boltdb backend.
dbPath := filepath.Join(l.dbDirPath, walletDbName)
db, err := walletdb.Open("bdb", dbPath, l.noFreelistSync)
opts := &bbolt.Options{
NoFreelistSync: l.noFreelistSync,
}
db, err := walletdb.Open("bdb", dbPath, opts)
if err != nil {
log.Errorf("Failed to open database: %v", err)
return nil, err
Expand Down
44 changes: 39 additions & 5 deletions walletdb/bdb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"os"

"github.com/btcsuite/btcwallet/walletdb"
"github.com/coreos/bbolt"
"go.etcd.io/bbolt"
)

// convertErr converts some bolt errors to the equivalent walletdb error.
Expand Down Expand Up @@ -365,18 +365,52 @@ func fileExists(name string) bool {
return true
}

const (
// is64Bit tells us if we're running on a 64-bit system or not. If uintptr is
// 32 bits, then its complement will be 0xffffffff rather than
// 0xffffffffffffffff. The complement of uint64(0) will always be
// 0xffffffffffffffff.
is64Bit = uint64(^uintptr(0)) == ^uint64(0)

// max32BitMapSize is the largest mmap size we can support on 32-bit
// systems. (2^31)-1 == 0x7FFFFFFF.
max32BitMapSize = 0x7FFFFFFF
)

// openDB opens the database at the provided path. walletdb.ErrDbDoesNotExist
// is returned if the database doesn't exist and the create flag is not set.
func openDB(dbPath string, noFreelistSync bool, create bool) (walletdb.DB, error) {
func openDB(dbPath string, create bool, options *bbolt.Options) (walletdb.DB, error) {
if !create && !fileExists(dbPath) {
return nil, walletdb.ErrDbDoesNotExist
}

// Specify bbolt freelist options to reduce heap pressure in case the
// freelist grows to be very large.
options := &bbolt.Options{
NoFreelistSync: noFreelistSync,
FreelistType: bbolt.FreelistMapType,
options.FreelistType = bbolt.FreelistMapType

if !create {
// The other value that we want to set is the initial memory
// map size. When bolt expands the mmap, it actually copies
// over the entire database. By setting this to a large value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value larger than zero

// than zero, then we can avoid some of that heap pressure due
// to the copies.
dbFileInfo, err := os.Stat(dbPath)
if err != nil {
return nil, err
}

// We'll aim to set the initial memory map to 2x the size of
// the database as it exists on disk, meaning the DB size can
// double without us having to remap everything.
mmapSize := dbFileInfo.Size() * 2

// If we're on a 32-bit system, then we'll need to limit this
// value to ensure we don't run into errors down the line.
if !is64Bit {
mmapSize = max32BitMapSize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we always set this for 32-bit systems, this will fail bc the process may be using other virtual memory. Also the mmap is a contiguous block of vmem, so even if this value is lower, this problem can still occur. Part of the reason why I think 32-bit support should be dropped

}

options.InitialMmapSize = int(mmapSize)
}

boltDB, err := bbolt.Open(dbPath, 0600, options)
Expand Down
25 changes: 13 additions & 12 deletions walletdb/bdb/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,57 +8,58 @@ import (
"fmt"

"github.com/btcsuite/btcwallet/walletdb"
"go.etcd.io/bbolt"
)

const (
dbType = "bdb"
)

// parseArgs parses the arguments from the walletdb Open/Create methods.
func parseArgs(funcName string, args ...interface{}) (string, bool, error) {
func parseArgs(funcName string, args ...interface{}) (string, *bbolt.Options, error) {
if len(args) != 2 {
return "", false, fmt.Errorf("invalid arguments to %s.%s -- "+
"expected database path and no-freelist-sync option",
return "", nil, fmt.Errorf("invalid arguments to %s.%s -- "+
"expected database path and *bbolt.Option option",
dbType, funcName)
}

dbPath, ok := args[0].(string)
if !ok {
return "", false, fmt.Errorf("first argument to %s.%s is "+
return "", nil, fmt.Errorf("first argument to %s.%s is "+
"invalid -- expected database path string", dbType,
funcName)
}

noFreelistSync, ok := args[1].(bool)
options, ok := args[1].(*bbolt.Options)
if !ok {
return "", false, fmt.Errorf("second argument to %s.%s is "+
"invalid -- expected no-freelist-sync bool", dbType,
return "", nil, fmt.Errorf("second argument to %s.%s is "+
"invalid -- expected *bbolt.Option", dbType,
funcName)
}

return dbPath, noFreelistSync, nil
return dbPath, options, nil
}

// openDBDriver is the callback provided during driver registration that opens
// an existing database for use.
func openDBDriver(args ...interface{}) (walletdb.DB, error) {
dbPath, noFreelistSync, err := parseArgs("Open", args...)
dbPath, options, err := parseArgs("Open", args...)
if err != nil {
return nil, err
}

return openDB(dbPath, noFreelistSync, false)
return openDB(dbPath, false, options)
}

// createDBDriver is the callback provided during driver registration that
// creates, initializes, and opens a database for use.
func createDBDriver(args ...interface{}) (walletdb.DB, error) {
dbPath, noFreelistSync, err := parseArgs("Create", args...)
dbPath, options, err := parseArgs("Create", args...)
if err != nil {
return nil, err
}

return openDB(dbPath, noFreelistSync, true)
return openDB(dbPath, true, options)
}

func init() {
Expand Down
21 changes: 15 additions & 6 deletions walletdb/bdb/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/btcsuite/btcwallet/walletdb"
_ "github.com/btcsuite/btcwallet/walletdb/bdb"
"go.etcd.io/bbolt"
)

// dbType is the database type name for this driver.
Expand All @@ -22,10 +23,14 @@ const dbType = "bdb"
// TestCreateOpenFail ensures that errors related to creating and opening a
// database are handled properly.
func TestCreateOpenFail(t *testing.T) {
opts := &bbolt.Options{
NoFreelistSync: true,
}

// Ensure that attempting to open a database that doesn't exist returns
// the expected error.
wantErr := walletdb.ErrDbDoesNotExist
if _, err := walletdb.Open(dbType, "noexist.db", true); err != wantErr {
if _, err := walletdb.Open(dbType, "noexist.db", opts); err != wantErr {
t.Errorf("Open: did not receive expected error - got %v, "+
"want %v", err, wantErr)
return
Expand All @@ -34,7 +39,7 @@ func TestCreateOpenFail(t *testing.T) {
// Ensure that attempting to open a database with the wrong number of
// parameters returns the expected error.
wantErr = fmt.Errorf("invalid arguments to %s.Open -- expected "+
"database path and no-freelist-sync option", dbType)
"database path and *bbolt.Option option", dbType)
if _, err := walletdb.Open(dbType, 1, 2, 3); err.Error() != wantErr.Error() {
t.Errorf("Open: did not receive expected error - got %v, "+
"want %v", err, wantErr)
Expand All @@ -54,7 +59,7 @@ func TestCreateOpenFail(t *testing.T) {
// Ensure that attempting to create a database with the wrong number of
// parameters returns the expected error.
wantErr = fmt.Errorf("invalid arguments to %s.Create -- expected "+
"database path and no-freelist-sync option", dbType)
"database path and *bbolt.Option option", dbType)
if _, err := walletdb.Create(dbType, 1, 2, 3); err.Error() != wantErr.Error() {
t.Errorf("Create: did not receive expected error - got %v, "+
"want %v", err, wantErr)
Expand All @@ -81,7 +86,7 @@ func TestCreateOpenFail(t *testing.T) {
defer os.Remove(tempDir)

dbPath := filepath.Join(tempDir, "db")
db, err := walletdb.Create(dbType, dbPath, true)
db, err := walletdb.Create(dbType, dbPath, opts)
if err != nil {
t.Errorf("Create: unexpected error: %v", err)
return
Expand All @@ -99,6 +104,10 @@ func TestCreateOpenFail(t *testing.T) {
// TestPersistence ensures that values stored are still valid after closing and
// reopening the database.
func TestPersistence(t *testing.T) {
opts := &bbolt.Options{
NoFreelistSync: true,
}

// Create a new database to run tests against.
tempDir, err := ioutil.TempDir("", "persistencetest")
if err != nil {
Expand All @@ -108,7 +117,7 @@ func TestPersistence(t *testing.T) {
defer os.Remove(tempDir)

dbPath := filepath.Join(tempDir, "db")
db, err := walletdb.Create(dbType, dbPath, true)
db, err := walletdb.Create(dbType, dbPath, opts)
if err != nil {
t.Errorf("Failed to create test database (%s) %v", dbType, err)
return
Expand Down Expand Up @@ -144,7 +153,7 @@ func TestPersistence(t *testing.T) {

// Close and reopen the database to ensure the values persist.
db.Close()
db, err = walletdb.Open(dbType, dbPath, true)
db, err = walletdb.Open(dbType, dbPath, opts)
if err != nil {
t.Errorf("Failed to open test database (%s) %v", dbType, err)
return
Expand Down
7 changes: 6 additions & 1 deletion walletdb/bdb/interface_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"testing"

"github.com/btcsuite/btcwallet/walletdb/walletdbtest"
"go.etcd.io/bbolt"
)

// TestInterface performs all interfaces tests for this database driver.
Expand All @@ -32,5 +33,9 @@ func TestInterface(t *testing.T) {

dbPath := filepath.Join(tempDir, "db")
defer os.RemoveAll(dbPath)
walletdbtest.TestInterface(t, dbType, dbPath, true)

opts := &bbolt.Options{
NoFreelistSync: true,
}
walletdbtest.TestInterface(t, dbType, dbPath, opts)
}
6 changes: 5 additions & 1 deletion walletdb/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/btcsuite/btcwallet/walletdb"
_ "github.com/btcsuite/btcwallet/walletdb/bdb"
"go.etcd.io/bbolt"
)

var (
Expand Down Expand Up @@ -64,7 +65,10 @@ func TestAddDuplicateDriver(t *testing.T) {
defer os.Remove(tempDir)

dbPath := filepath.Join(tempDir, "db")
db, err := walletdb.Create(dbType, dbPath, true)
opts := &bbolt.Options{
NoFreelistSync: true,
}
db, err := walletdb.Create(dbType, dbPath, opts)
if err != nil {
t.Errorf("failed to create database: %v", err)
return
Expand Down
Loading