Skip to content

[sql-25] firewalldb: start cleaning up RulesDB/KVStores code #1023

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 3, 2025
Merged

[sql-25] firewalldb: start cleaning up RulesDB/KVStores code #1023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5026e66
firewalldb: rename DB to BoltDB
ellemouton Apr 1, 2025
69a2512
firewalldb: move bolt db logic to kvdb_store.go
ellemouton Apr 1, 2025
261c547
firewalldb: add DB struct and Start/Stop methods
ellemouton Apr 1, 2025
5440c19
firewalldb: move generic DB setup code to the DB.Start method
ellemouton Apr 1, 2025
8bc7dd3
firewalldb: kvdb test file
ellemouton Mar 12, 2025
5dd0479
firewalldb: move bbolt kvstore logic to specific file
ellemouton Apr 3, 2025
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
51 changes: 32 additions & 19 deletions config_dev.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
package terminal

import (
"fmt"
"path/filepath"

"github.com/lightninglabs/lightning-terminal/accounts"
"github.com/lightninglabs/lightning-terminal/db"
"github.com/lightninglabs/lightning-terminal/firewalldb"
"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/clock"
)
Expand Down Expand Up @@ -87,7 +89,7 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
networkDir = filepath.Join(cfg.LitDir, cfg.Network)
acctStore accounts.Store
sessStore session.Store
closeFn func() error
closeFns = make(map[string]func() error)
)

switch cfg.DatabaseBackend {
Expand All @@ -106,7 +108,7 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {

acctStore = accounts.NewSQLStore(sqlStore.BaseDB, clock)
sessStore = session.NewSQLStore(sqlStore.BaseDB, clock)
closeFn = sqlStore.BaseDB.Close
closeFns["sqlite"] = sqlStore.BaseDB.Close

case DatabaseBackendPostgres:
sqlStore, err := db.NewPostgresStore(cfg.Postgres)
Expand All @@ -116,7 +118,7 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {

acctStore = accounts.NewSQLStore(sqlStore.BaseDB, clock)
sessStore = session.NewSQLStore(sqlStore.BaseDB, clock)
closeFn = sqlStore.BaseDB.Close
closeFns["postgres"] = sqlStore.BaseDB.Close

default:
accountStore, err := accounts.NewBoltStore(
Expand All @@ -126,35 +128,46 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
if err != nil {
return nil, err
}
closeFns["bbolt-accounts"] = accountStore.Close

sessionStore, err := session.NewDB(
networkDir, session.DBFilename, clock, accountStore,
)
if err != nil {
return nil, err
}
closeFns["bbolt-sessions"] = sessionStore.Close

acctStore = accountStore
sessStore = sessionStore
closeFn = func() error {
var returnErr error
err = accountStore.Close()
if err != nil {
returnErr = err
}

err = sessionStore.Close()
if err != nil {
returnErr = err
}
}

return returnErr
}
firewallBoltDB, err := firewalldb.NewBoltDB(
networkDir, firewalldb.DBFilename, sessStore,
)
if err != nil {
return nil, fmt.Errorf("error creating firewall BoltDB: %v",
err)
}
closeFns["bbolt-firewalldb"] = firewallBoltDB.Close

return &stores{
accounts: acctStore,
sessions: sessStore,
close: closeFn,
accounts: acctStore,
sessions: sessStore,
firewall: firewalldb.NewDB(firewallBoltDB),
firewallBolt: firewallBoltDB,
close: func() error {
var returnErr error
for storeName, fn := range closeFns {
err := fn()
if err != nil {
log.Errorf("error closing %s store: %v",
storeName, err)
returnErr = err
}
}

return returnErr
},
}, nil
}
24 changes: 22 additions & 2 deletions config_prod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"

"github.com/lightninglabs/lightning-terminal/accounts"
"github.com/lightninglabs/lightning-terminal/firewalldb"
"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/clock"
)
Expand Down Expand Up @@ -46,18 +47,37 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
err)
}

firewallDB, err := firewalldb.NewBoltDB(
networkDir, firewalldb.DBFilename, sessStore,
)
if err != nil {
return nil, fmt.Errorf("error creating firewall DB: %v", err)
}
Comment on lines +50 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmmm this is pre-existing, so I'll let you decide if you'd want to address this or not:
One issue that I realized we have here, is that if this firewalldb.NewBoltDB call, or the session.NewDB function call above returns an err, we will never Close the previously initialized dbs. That is because we'll return nil for the *stores return param, and therefore not run the close function in terminal.go.

Not sure if that's something we'd want to address or not, so will let you decide on that :)!

Copy link
Member Author

Choose a reason for hiding this comment

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

great catch! yes this absolutely should be addressed. But I will do it in a follow up since these commits have already been approved and since the issue was introduced before (my bad!).


return &stores{
accounts: acctStore,
sessions: sessStore,
accounts: acctStore,
sessions: sessStore,
firewallBolt: firewallDB,
firewall: firewalldb.NewDB(firewallDB),
close: func() error {
var returnErr error
if err := acctStore.Close(); err != nil {
returnErr = fmt.Errorf("error closing "+
"account store: %v", err)

log.Error(returnErr.Error())
}
if err := sessStore.Close(); err != nil {
returnErr = fmt.Errorf("error closing "+
"session store: %v", err)

log.Error(returnErr.Error())
}
if err := firewallDB.Close(); err != nil {
returnErr = fmt.Errorf("error closing "+
"firewall DB: %v", err)

log.Error(returnErr.Error())
}

return returnErr
Expand Down
16 changes: 9 additions & 7 deletions firewalldb/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ type Action struct {
}

// AddAction serialises and adds an Action to the DB under the given sessionID.
func (db *DB) AddAction(sessionID session.ID, action *Action) (uint64, error) {
func (db *BoltDB) AddAction(sessionID session.ID, action *Action) (uint64,
error) {

var buf bytes.Buffer
if err := SerializeAction(&buf, action); err != nil {
return 0, err
Expand Down Expand Up @@ -231,7 +233,7 @@ func getAction(actionsBkt *bbolt.Bucket, al *ActionLocator) (*Action, error) {

// SetActionState finds the action specified by the ActionLocator and sets its
// state to the given state.
func (db *DB) SetActionState(al *ActionLocator, state ActionState,
func (db *BoltDB) SetActionState(al *ActionLocator, state ActionState,
errorReason string) error {

if errorReason != "" && state != ActionStateError {
Expand Down Expand Up @@ -293,7 +295,7 @@ type ListActionsFilterFn func(a *Action, reversed bool) (bool, bool)
// The indexOffset and maxNum params can be used to control the number of
// actions returned. The return values are the list of actions, the last index
// and the total count (iff query.CountTotal is set).
func (db *DB) ListActions(filterFn ListActionsFilterFn,
func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
query *ListActionsQuery) ([]*Action, uint64, uint64, error) {

var (
Expand Down Expand Up @@ -345,7 +347,7 @@ func (db *DB) ListActions(filterFn ListActionsFilterFn,

// ListSessionActions returns a list of the given session's Actions that pass
// the filterFn requirements.
func (db *DB) ListSessionActions(sessionID session.ID,
func (db *BoltDB) ListSessionActions(sessionID session.ID,
filterFn ListActionsFilterFn, query *ListActionsQuery) ([]*Action,
uint64, uint64, error) {

Expand Down Expand Up @@ -391,7 +393,7 @@ func (db *DB) ListSessionActions(sessionID session.ID,
// pass the filterFn requirements.
//
// TODO: update to allow for pagination.
func (db *DB) ListGroupActions(ctx context.Context, groupID session.ID,
func (db *BoltDB) ListGroupActions(ctx context.Context, groupID session.ID,
filterFn ListActionsFilterFn) ([]*Action, error) {

if filterFn == nil {
Expand Down Expand Up @@ -589,7 +591,7 @@ type ActionReadDBGetter interface {
}

// GetActionsReadDB is a method on DB that constructs an ActionsReadDB.
func (db *DB) GetActionsReadDB(groupID session.ID,
func (db *BoltDB) GetActionsReadDB(groupID session.ID,
featureName string) ActionsReadDB {

return &allActionsReadDB{
Expand All @@ -601,7 +603,7 @@ func (db *DB) GetActionsReadDB(groupID session.ID,

// allActionsReadDb is an implementation of the ActionsReadDB.
type allActionsReadDB struct {
db *DB
db *BoltDB
groupID session.ID
featureName string
}
Expand Down
6 changes: 3 additions & 3 deletions firewalldb/actions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var (
func TestActionStorage(t *testing.T) {
tmpDir := t.TempDir()

db, err := NewDB(tmpDir, "test.db", nil)
db, err := NewBoltDB(tmpDir, "test.db", nil)
require.NoError(t, err)
t.Cleanup(func() {
_ = db.Close()
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestActionStorage(t *testing.T) {
func TestListActions(t *testing.T) {
tmpDir := t.TempDir()

db, err := NewDB(tmpDir, "test.db", nil)
db, err := NewBoltDB(tmpDir, "test.db", nil)
require.NoError(t, err)
t.Cleanup(func() {
_ = db.Close()
Expand Down Expand Up @@ -353,7 +353,7 @@ func TestListGroupActions(t *testing.T) {
index.AddPair(sessionID1, group1)
index.AddPair(sessionID2, group1)

db, err := NewDB(t.TempDir(), "test.db", index)
db, err := NewBoltDB(t.TempDir(), "test.db", index)
require.NoError(t, err)
t.Cleanup(func() {
_ = db.Close()
Expand Down
Loading
Loading