From 69f48db0a35f99028170c1e94fdb5b322993c2b0 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Wed, 22 Jan 2025 19:53:20 +0100 Subject: [PATCH] [management] disable prepareStmt for sqlite (#3228) --- go.mod | 6 +- go.sum | 11 ++-- management/server/store/sql_store.go | 14 +++-- management/server/store/sql_store_test.go | 74 ++++++++++++++++++++++- management/server/store/store.go | 2 +- 5 files changed, 92 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index fa573bb9c..895ad5618 100644 --- a/go.mod +++ b/go.mod @@ -55,7 +55,7 @@ require ( github.com/libdns/route53 v1.5.0 github.com/libp2p/go-netroute v0.2.1 github.com/magiconair/properties v1.8.7 - github.com/mattn/go-sqlite3 v1.14.19 + github.com/mattn/go-sqlite3 v1.14.22 github.com/mdlayher/socket v0.5.1 github.com/miekg/dns v1.1.59 github.com/mitchellh/hashstructure/v2 v2.0.2 @@ -100,8 +100,8 @@ require ( gopkg.in/yaml.v3 v3.0.1 gorm.io/driver/mysql v1.5.7 gorm.io/driver/postgres v1.5.7 - gorm.io/driver/sqlite v1.5.3 - gorm.io/gorm v1.25.7 + gorm.io/driver/sqlite v1.5.7 + gorm.io/gorm v1.25.12 nhooyr.io/websocket v1.8.11 ) diff --git a/go.sum b/go.sum index a099498fb..2aae595f0 100644 --- a/go.sum +++ b/go.sum @@ -475,8 +475,8 @@ github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.9/go.mod h1:YNRxwqDuOph6SZLI9vUUz6OYw3QyUt7WiY2yME+cCiQ= -github.com/mattn/go-sqlite3 v1.14.19 h1:fhGleo2h1p8tVChob4I9HpmVFIAkKGpiukdrgQbWfGI= -github.com/mattn/go-sqlite3 v1.14.19/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg= +github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU= +github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/mdlayher/genetlink v1.3.2 h1:KdrNKe+CTu+IbZnm/GVUMXSqBBLqcGpRDa0xkQy56gw= github.com/mdlayher/genetlink v1.3.2/go.mod h1:tcC3pkCrPUGIKKsCsp0B3AdaaKuHtaxoJRz3cc+528o= github.com/mdlayher/netlink v1.7.2 h1:/UtM3ofJap7Vl4QWCPDGXY8d3GIY2UGSDbK+QWmY8/g= @@ -1241,10 +1241,11 @@ gorm.io/driver/mysql v1.5.7 h1:MndhOPYOfEp2rHKgkZIhJ16eVUIRf2HmzgoPmh7FCWo= gorm.io/driver/mysql v1.5.7/go.mod h1:sEtPWMiqiN1N1cMXoXmBbd8C6/l+TESwriotuRRpkDM= gorm.io/driver/postgres v1.5.7 h1:8ptbNJTDbEmhdr62uReG5BGkdQyeasu/FZHxI0IMGnM= gorm.io/driver/postgres v1.5.7/go.mod h1:3e019WlBaYI5o5LIdNV+LyxCMNtLOQETBXL2h4chKpA= -gorm.io/driver/sqlite v1.5.3 h1:7/0dUgX28KAcopdfbRWWl68Rflh6osa4rDh+m51KL2g= -gorm.io/driver/sqlite v1.5.3/go.mod h1:qxAuCol+2r6PannQDpOP1FP6ag3mKi4esLnB/jHed+4= -gorm.io/gorm v1.25.7 h1:VsD6acwRjz2zFxGO50gPO6AkNs7KKnvfzUjHQhZDz/A= +gorm.io/driver/sqlite v1.5.7 h1:8NvsrhP0ifM7LX9G4zPB97NwovUakUxc+2V2uuf3Z1I= +gorm.io/driver/sqlite v1.5.7/go.mod h1:U+J8craQU6Fzkcvu8oLeAQmi50TkwPEhHDEjQZXDah4= gorm.io/gorm v1.25.7/go.mod h1:hbnx/Oo0ChWMn1BIhpy1oYozzpM15i4YPuHDmfYtwg8= +gorm.io/gorm v1.25.12 h1:I0u8i2hWQItBq1WfE0o2+WuL9+8L21K9e2HHSTE/0f8= +gorm.io/gorm v1.25.12/go.mod h1:xh7N7RHfYlNc5EmcI/El95gXusucDrQnHXe0+CgWcLQ= gotest.tools/v3 v3.5.0 h1:Ljk6PdHdOhAb5aDMWXjDLMMhph+BpztA4v1QdqEW2eY= gotest.tools/v3 v3.5.0/go.mod h1:isy3WKz7GK6uNw/sbHzfKBLvlvXwUyV06n6brMxxopU= gvisor.dev/gvisor v0.0.0-20231020174304-db3d49b921f9 h1:sCEaoA7ZmkuFwa2IR61pl4+RYZPwCJOiaSYT0k+BRf8= diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 900d81322..2179f0754 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -956,7 +956,7 @@ func NewSqliteStore(ctx context.Context, dataDir string, metrics telemetry.AppMe } file := filepath.Join(dataDir, storeStr) - db, err := gorm.Open(sqlite.Open(file), getGormConfig()) + db, err := gorm.Open(sqlite.Open(file), getGormConfig(SqliteStoreEngine)) if err != nil { return nil, err } @@ -966,7 +966,7 @@ func NewSqliteStore(ctx context.Context, dataDir string, metrics telemetry.AppMe // NewPostgresqlStore creates a new Postgres store. func NewPostgresqlStore(ctx context.Context, dsn string, metrics telemetry.AppMetrics) (*SqlStore, error) { - db, err := gorm.Open(postgres.Open(dsn), getGormConfig()) + db, err := gorm.Open(postgres.Open(dsn), getGormConfig(PostgresStoreEngine)) if err != nil { return nil, err } @@ -976,7 +976,7 @@ func NewPostgresqlStore(ctx context.Context, dsn string, metrics telemetry.AppMe // NewMysqlStore creates a new MySQL store. func NewMysqlStore(ctx context.Context, dsn string, metrics telemetry.AppMetrics) (*SqlStore, error) { - db, err := gorm.Open(mysql.Open(dsn+"?charset=utf8&parseTime=True&loc=Local"), getGormConfig()) + db, err := gorm.Open(mysql.Open(dsn+"?charset=utf8&parseTime=True&loc=Local"), getGormConfig(MysqlStoreEngine)) if err != nil { return nil, err } @@ -984,11 +984,15 @@ func NewMysqlStore(ctx context.Context, dsn string, metrics telemetry.AppMetrics return NewSqlStore(ctx, db, MysqlStoreEngine, metrics) } -func getGormConfig() *gorm.Config { +func getGormConfig(engine Engine) *gorm.Config { + prepStmt := true + if engine == SqliteStoreEngine { + prepStmt = false + } return &gorm.Config{ Logger: logger.Default.LogMode(logger.Silent), CreateBatchSize: 400, - PrepareStmt: true, + PrepareStmt: prepStmt, } } diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index cb51dab51..9350da1c8 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -10,16 +10,18 @@ import ( "net/netip" "os" "runtime" + "sync" "testing" "time" "github.com/google/uuid" - "github.com/netbirdio/netbird/management/server/util" "github.com/rs/xid" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/management/server/util" + nbdns "github.com/netbirdio/netbird/dns" resourceTypes "github.com/netbirdio/netbird/management/server/networks/resources/types" routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types" @@ -2843,3 +2845,73 @@ func TestSqlStore_DeletePeer(t *testing.T) { require.Error(t, err) require.Nil(t, peer) } + +func TestSqlStore_DatabaseBlocking(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store_with_expired_peers.sql", t.TempDir()) + t.Cleanup(cleanup) + if err != nil { + t.Fatal(err) + } + + concurrentReads := 40 + + testRunSuccessful := false + wgSuccess := sync.WaitGroup{} + wgSuccess.Add(concurrentReads) + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + start := make(chan struct{}) + + for i := 0; i < concurrentReads/2; i++ { + go func() { + t.Logf("Entered routine 1-%d", i) + + <-start + err := store.ExecuteInTransaction(context.Background(), func(tx Store) error { + _, err := tx.GetAccountIDByPeerID(context.Background(), LockingStrengthShare, "cfvprsrlo1hqoo49ohog") + return err + }) + if err != nil { + t.Errorf("Failed, got error: %v", err) + return + } + + t.Log("Got User from routine 1") + wgSuccess.Done() + }() + } + + for i := 0; i < concurrentReads/2; i++ { + go func() { + t.Logf("Entered routine 2-%d", i) + + <-start + _, err := store.GetAccountIDByPeerID(context.Background(), LockingStrengthShare, "cfvprsrlo1hqoo49ohog") + if err != nil { + t.Errorf("Failed, got error: %v", err) + return + } + + t.Log("Got User from routine 2") + wgSuccess.Done() + }() + } + + time.Sleep(200 * time.Millisecond) + close(start) + t.Log("Started routines") + + go func() { + wgSuccess.Wait() + testRunSuccessful = true + }() + + <-ctx.Done() + if !testRunSuccessful { + t.Fatalf("Test failed") + } + + t.Logf("Test completed") +} diff --git a/management/server/store/store.go b/management/server/store/store.go index 245df1c3e..4b4dcfb4f 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -319,7 +319,7 @@ func NewTestStoreFromSQL(ctx context.Context, filename string, dataDir string) ( } file := filepath.Join(dataDir, storeStr) - db, err := gorm.Open(sqlite.Open(file), getGormConfig()) + db, err := gorm.Open(sqlite.Open(file), getGormConfig(kind)) if err != nil { return nil, nil, err }