From 5daef58f54a49e41b6e95d3b43b273095854f3b3 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 27 Oct 2023 11:24:29 -0700 Subject: [PATCH] SQLite allows having 1 set to MaxOpenConnections (#3941) * let sqlite have a single concurrent connection --- internal/clientcache/cmd/doc.go | 2 +- .../clientcache/internal/cache/store_test.go | 28 +++++++++++++++++++ internal/clientcache/internal/db/db.go | 4 ++- internal/db/db.go | 2 +- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/internal/clientcache/cmd/doc.go b/internal/clientcache/cmd/doc.go index d8ca46f88..f844f7d1c 100644 --- a/internal/clientcache/cmd/doc.go +++ b/internal/clientcache/cmd/doc.go @@ -6,4 +6,4 @@ Package cmd contains the command packages to the client cache. Adding these commands to the boundary cli is the only integration between the boundary binary and the client cache. */ -package cmd \ No newline at end of file +package cmd diff --git a/internal/clientcache/internal/cache/store_test.go b/internal/clientcache/internal/cache/store_test.go index a7920de15..a4b01b163 100644 --- a/internal/clientcache/internal/cache/store_test.go +++ b/internal/clientcache/internal/cache/store_test.go @@ -5,6 +5,7 @@ package cache import ( "context" + "sync" "testing" "time" @@ -506,3 +507,30 @@ func TestSession(t *testing.T) { assert.ErrorContains(t, rw.LookupById(ctx, lookSess), "not found") }) } + +func TestRepository_SqliteReadDuringTx(t *testing.T) { + ctx := context.Background() + s, err := cachedb.Open(ctx) + require.NoError(t, err) + rw := db.New(s) + + ret := AuthToken{ + Id: "something", + } + assert.ErrorContains(t, rw.LookupById(ctx, &ret), "not found") + + var startTx sync.WaitGroup + startTx.Add(1) + go func() { + _, err = rw.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, writer db.Writer) error { + startTx.Done() + time.Sleep(500 * time.Millisecond) + + assert.ErrorContains(t, reader.LookupById(ctx, &ret), "not found") + return nil + }) + }() + // setup a read that potentially executes during a tx + startTx.Wait() + assert.ErrorContains(t, rw.LookupById(ctx, &ret), "not found") +} diff --git a/internal/clientcache/internal/db/db.go b/internal/clientcache/internal/db/db.go index 2d44c7146..b24a1dafa 100644 --- a/internal/clientcache/internal/db/db.go +++ b/internal/clientcache/internal/db/db.go @@ -34,7 +34,9 @@ func Open(ctx context.Context, opt ...Option) (*db.DB, error) { default: url = DefaultStoreUrl } - conn, err := db.Open(ctx, db.Sqlite, url) + // sqlite db must be set to 1 max open connection so requests can be serialized + // otherwise reading while a tx is ongoing results in sql logic errors. + conn, err := db.Open(ctx, db.Sqlite, url, db.WithMaxOpenConnections(1)) if err != nil { return nil, errors.Wrap(ctx, err, op) } diff --git a/internal/db/db.go b/internal/db/db.go index 571ecbacb..e2961829f 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -174,7 +174,7 @@ func Open(ctx context.Context, dbType DbType, connectionUrl string, opt ...Optio wrappedOpts = append(wrappedOpts, dbw.WithLogger(opts.withGormFormatter)) } if opts.withMaxOpenConnections > 0 { - if opts.withMaxOpenConnections < 5 { + if opts.withMaxOpenConnections < 5 && dbType != Sqlite { return nil, fmt.Errorf("max_open_connections cannot be below 5") } wrappedOpts = append(wrappedOpts, dbw.WithMaxOpenConnections(opts.withMaxOpenConnections))