From 8070a9aa6624d2b48ede4d039e39c894aa6c9a77 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Fri, 2 Feb 2024 09:17:35 +0100 Subject: [PATCH 1/3] libnet: drop TestMultipleControllersWithSameStore This test is non-representative of what we now do in libnetwork. Since the ability of opening the same boltdb database multiple times in parallel will be dropped in the next commit, just remove this test. Signed-off-by: Albin Kerouanton --- libnetwork/store_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/libnetwork/store_test.go b/libnetwork/store_test.go index e23b0a188a69b..358c148d65ae9 100644 --- a/libnetwork/store_test.go +++ b/libnetwork/store_test.go @@ -80,18 +80,3 @@ func OptionBoltdbWithRandomDBFile(t *testing.T) config.Option { c.Scope.Client.Config = &store.Config{Bucket: "testBackend"} } } - -func TestMultipleControllersWithSameStore(t *testing.T) { - cfgOptions := OptionBoltdbWithRandomDBFile(t) - ctrl1, err := New(cfgOptions) - if err != nil { - t.Fatalf("Error new controller: %v", err) - } - defer ctrl1.Stop() - // Use the same boltdb file without closing the previous controller - ctrl2, err := New(cfgOptions) - if err != nil { - t.Fatalf("Local store must support concurrent controllers") - } - ctrl2.Stop() -} From 4d7c11c208597f1ce1ba64b06eefffa9ecf58e22 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Fri, 2 Feb 2024 08:13:41 +0100 Subject: [PATCH 2/3] libnet: boltdb: remove PersistConnection This parameter was used to tell the boltdb kvstore not to open/close the underlying boltdb db file before/after each get/put operation. Since d21d0884ae, we've a single datastore instance shared by all components that need it. That commit set `PersistConnection=true`. We can now safely remove this param altogether, and remove all the code that was opening and closing the db file before and after each operation -- it's dead code! Signed-off-by: Albin Kerouanton --- libnetwork/datastore/datastore.go | 1 - libnetwork/internal/kvstore/boltdb/boltdb.go | 58 ++++---------------- libnetwork/internal/kvstore/kvstore.go | 1 - 3 files changed, 10 insertions(+), 50 deletions(-) diff --git a/libnetwork/datastore/datastore.go b/libnetwork/datastore/datastore.go index 6df1287c533af..f9d1bb536d555 100644 --- a/libnetwork/datastore/datastore.go +++ b/libnetwork/datastore/datastore.go @@ -92,7 +92,6 @@ func DefaultScope(dataDir string) ScopeCfg { Config: &store.Config{ Bucket: "libnetwork", ConnectionTimeout: time.Minute, - PersistConnection: true, }, }, } diff --git a/libnetwork/internal/kvstore/boltdb/boltdb.go b/libnetwork/internal/kvstore/boltdb/boltdb.go index 7d3ed24836b9d..76169152e39be 100644 --- a/libnetwork/internal/kvstore/boltdb/boltdb.go +++ b/libnetwork/internal/kvstore/boltdb/boltdb.go @@ -29,12 +29,6 @@ type BoltDB struct { dbIndex uint64 path string timeout time.Duration - // By default libkv opens and closes the bolt DB connection for every - // get/put operation. This allows multiple apps to use a Bolt DB at the - // same time. - // PersistConnection flag provides an option to override ths behavior. - // ie: open the connection in New and use it till Close is called. - PersistConnection bool } const ( @@ -53,15 +47,11 @@ func New(endpoint string, options *store.Config) (store.Store, error) { return nil, err } - var db *bolt.DB - if options.PersistConnection { - var err error - db, err = bolt.Open(endpoint, filePerm, &bolt.Options{ - Timeout: options.ConnectionTimeout, - }) - if err != nil { - return nil, err - } + db, err := bolt.Open(endpoint, filePerm, &bolt.Options{ + Timeout: options.ConnectionTimeout, + }) + if err != nil { + return nil, err } timeout := transientTimeout @@ -70,38 +60,19 @@ func New(endpoint string, options *store.Config) (store.Store, error) { } b := &BoltDB{ - client: db, - path: endpoint, - boltBucket: []byte(options.Bucket), - timeout: timeout, - PersistConnection: options.PersistConnection, + client: db, + path: endpoint, + boltBucket: []byte(options.Bucket), + timeout: timeout, } return b, nil } -func (b *BoltDB) reset() { - b.path = "" - b.boltBucket = []byte{} -} - func (b *BoltDB) getDBhandle() (*bolt.DB, error) { - if !b.PersistConnection { - db, err := bolt.Open(b.path, filePerm, &bolt.Options{Timeout: b.timeout}) - if err != nil { - return nil, err - } - b.client = db - } return b.client, nil } -func (b *BoltDB) releaseDBhandle() { - if !b.PersistConnection { - b.client.Close() - } -} - // Put the key, value pair. index number metadata is prepended to the value func (b *BoltDB) Put(key string, value []byte) error { b.mu.Lock() @@ -111,7 +82,6 @@ func (b *BoltDB) Put(key string, value []byte) error { if err != nil { return err } - defer b.releaseDBhandle() return db.Update(func(tx *bolt.Tx) error { bucket, err := tx.CreateBucketIfNotExists(b.boltBucket) @@ -137,7 +107,6 @@ func (b *BoltDB) Exists(key string) (bool, error) { if err != nil { return false, err } - defer b.releaseDBhandle() var exists bool err = db.View(func(tx *bolt.Tx) error { @@ -167,7 +136,6 @@ func (b *BoltDB) List(keyPrefix string) ([]*store.KVPair, error) { if err != nil { return nil, err } - defer b.releaseDBhandle() var kv []*store.KVPair err = db.View(func(tx *bolt.Tx) error { @@ -216,7 +184,6 @@ func (b *BoltDB) AtomicDelete(key string, previous *store.KVPair) error { if err != nil { return err } - defer b.releaseDBhandle() return db.Update(func(tx *bolt.Tx) error { bucket := tx.Bucket(b.boltBucket) @@ -246,7 +213,6 @@ func (b *BoltDB) AtomicPut(key string, value []byte, previous *store.KVPair) (*s if err != nil { return nil, err } - defer b.releaseDBhandle() var dbIndex uint64 dbval := make([]byte, libkvmetadatalen) @@ -293,9 +259,5 @@ func (b *BoltDB) Close() { b.mu.Lock() defer b.mu.Unlock() - if !b.PersistConnection { - b.reset() - } else { - b.client.Close() - } + b.client.Close() } diff --git a/libnetwork/internal/kvstore/kvstore.go b/libnetwork/internal/kvstore/kvstore.go index 4566ca9690aeb..918ad2b508aa9 100644 --- a/libnetwork/internal/kvstore/kvstore.go +++ b/libnetwork/internal/kvstore/kvstore.go @@ -28,7 +28,6 @@ var ( type Config struct { ConnectionTimeout time.Duration Bucket string - PersistConnection bool } // Store represents the backend K/V storage From 83af50aee36d70ce350980462d5aee94f6ad05f8 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Fri, 2 Feb 2024 08:26:06 +0100 Subject: [PATCH 3/3] libnet: boltdb: inline getDBhandle() Previous commit made getDBhandle a one-liner returning a struct member -- making it useless. Inline it. Signed-off-by: Albin Kerouanton --- libnetwork/internal/kvstore/boltdb/boltdb.go | 38 +++----------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/libnetwork/internal/kvstore/boltdb/boltdb.go b/libnetwork/internal/kvstore/boltdb/boltdb.go index 76169152e39be..b2051aca2c9f0 100644 --- a/libnetwork/internal/kvstore/boltdb/boltdb.go +++ b/libnetwork/internal/kvstore/boltdb/boltdb.go @@ -69,21 +69,12 @@ func New(endpoint string, options *store.Config) (store.Store, error) { return b, nil } -func (b *BoltDB) getDBhandle() (*bolt.DB, error) { - return b.client, nil -} - // Put the key, value pair. index number metadata is prepended to the value func (b *BoltDB) Put(key string, value []byte) error { b.mu.Lock() defer b.mu.Unlock() - db, err := b.getDBhandle() - if err != nil { - return err - } - - return db.Update(func(tx *bolt.Tx) error { + return b.client.Update(func(tx *bolt.Tx) error { bucket, err := tx.CreateBucketIfNotExists(b.boltBucket) if err != nil { return err @@ -103,13 +94,8 @@ func (b *BoltDB) Exists(key string) (bool, error) { b.mu.Lock() defer b.mu.Unlock() - db, err := b.getDBhandle() - if err != nil { - return false, err - } - var exists bool - err = db.View(func(tx *bolt.Tx) error { + err := b.client.View(func(tx *bolt.Tx) error { bucket := tx.Bucket(b.boltBucket) if bucket == nil { return store.ErrKeyNotFound @@ -132,13 +118,8 @@ func (b *BoltDB) List(keyPrefix string) ([]*store.KVPair, error) { b.mu.Lock() defer b.mu.Unlock() - db, err := b.getDBhandle() - if err != nil { - return nil, err - } - var kv []*store.KVPair - err = db.View(func(tx *bolt.Tx) error { + err := b.client.View(func(tx *bolt.Tx) error { bucket := tx.Bucket(b.boltBucket) if bucket == nil { return store.ErrKeyNotFound @@ -180,12 +161,8 @@ func (b *BoltDB) AtomicDelete(key string, previous *store.KVPair) error { if previous == nil { return store.ErrPreviousNotSpecified } - db, err := b.getDBhandle() - if err != nil { - return err - } - return db.Update(func(tx *bolt.Tx) error { + return b.client.Update(func(tx *bolt.Tx) error { bucket := tx.Bucket(b.boltBucket) if bucket == nil { return store.ErrKeyNotFound @@ -209,14 +186,9 @@ func (b *BoltDB) AtomicPut(key string, value []byte, previous *store.KVPair) (*s b.mu.Lock() defer b.mu.Unlock() - db, err := b.getDBhandle() - if err != nil { - return nil, err - } - var dbIndex uint64 dbval := make([]byte, libkvmetadatalen) - err = db.Update(func(tx *bolt.Tx) error { + err := b.client.Update(func(tx *bolt.Tx) error { bucket := tx.Bucket(b.boltBucket) if bucket == nil { if previous != nil {