Skip to content

Commit c0d05db

Browse files
EmyrkEmyrk
committed
fixups
1 parent 746203d commit c0d05db

File tree

7 files changed

+36
-15
lines changed

7 files changed

+36
-15
lines changed

‎coderd/database/db.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,20 @@ type DBTX interface {
4848
GetContext(ctx context.Context, dest interface{}, query string, args ...interface{}) error
4949
}
5050

51+
func WithSerialRetryCount(count int) func(*sqlQuerier) {
52+
return func(q *sqlQuerier) {
53+
q.serialRetryCount = count
54+
}
55+
}
56+
5157
// New creates a new database store using a SQL database connection.
52-
func New(sdb *sql.DB) Store {
58+
func New(sdb *sql.DB, opts ...func(*sqlQuerier)) Store {
5359
dbx := sqlx.NewDb(sdb, "postgres")
5460
return &sqlQuerier{
5561
db: dbx,
5662
sdb: dbx,
63+
// This is an arbitrary number.
64+
serialRetryCount: 3,
5765
}
5866
}
5967

@@ -104,6 +112,10 @@ type querier interface {
104112
type sqlQuerier struct {
105113
sdb *sqlx.DB
106114
db DBTX
115+
116+
// serialRetryCount is the number of times to retry a transaction
117+
// if it fails with a serialization error.
118+
serialRetryCount int
107119
}
108120

109121
func (*sqlQuerier) Wrappers() []string {
@@ -143,11 +155,9 @@ func (q *sqlQuerier) InTx(function func(Store) error, txOpts *TxOptions) error {
143155
// If we are in a transaction already, the parent InTx call will handle the retry.
144156
// We do not want to duplicate those retries.
145157
if !inTx && sqlOpts.Isolation == sql.LevelSerializable {
146-
// This is an arbitrarily chosen number.
147-
const retryAmount = 1
148158
var err error
149159
attempts := 0
150-
for attempts = 0; attempts < retryAmount; attempts++ {
160+
for attempts = 0; attempts < q.serialRetryCount; attempts++ {
151161
txOpts.executionCount++
152162
err = q.runTx(function, sqlOpts)
153163
if err == nil {

‎coderd/database/dbfake/builder.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func (b OrganizationBuilder) Do() OrganizationResponse {
6767
org := dbgen.Organization(b.t, b.db, b.seed)
6868

6969
ctx := testutil.Context(b.t, testutil.WaitShort)
70+
//nolint:gocritic // builder code needs perms
7071
ctx = dbauthz.AsSystemRestricted(ctx)
7172
everyone, err := b.db.InsertAllUsersGroup(ctx, org.ID)
7273
require.NoError(b.t, err)

‎coderd/database/dbtestutil/db.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) {
135135
if o.dumpOnFailure {
136136
t.Cleanup(func() { DumpOnFailure(t, connectionURL) })
137137
}
138-
db = database.New(sqlDB)
138+
// Unit tests should not retry serial transaction failures.
139+
db = database.New(sqlDB, database.WithSerialRetryCount(1))
139140

140141
ps, err = pubsub.New(context.Background(), o.logger, sqlDB, connectionURL)
141142
require.NoError(t, err)

‎coderd/database/dbtestutil/tx.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ type DBTx struct {
2626
//
2727
// require.NoError(t, a.Done()
2828
func StartTx(t *testing.T, db database.Store, opts *database.TxOptions) *DBTx {
29-
errC := make(chan error)
29+
done := make(chan error)
3030
finalErr := make(chan error)
3131
txC := make(chan database.Store)
3232

@@ -47,9 +47,7 @@ func StartTx(t *testing.T, db database.Store, opts *database.TxOptions) *DBTx {
4747
t.Fatal("InTx called more than once, this is not allowed with the StartTx helper")
4848
}
4949

50-
select {
51-
case _, _ = <-errC:
52-
}
50+
<-done
5351
// Just return nil. The caller should be checking their own errors.
5452
return nil
5553
}, opts)
@@ -59,7 +57,7 @@ func StartTx(t *testing.T, db database.Store, opts *database.TxOptions) *DBTx {
5957
txStore := <-txC
6058
close(txC)
6159

62-
return &DBTx{Store: txStore, done: errC, finalErr: finalErr}
60+
return &DBTx{Store: txStore, done: done, finalErr: finalErr}
6361
}
6462

6563
// Done can only be called once. If you call it twice, it will panic.

‎coderd/database/queries.sql.go

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/quotas.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ JOIN latest_builds ON
4343
latest_builds.workspace_id = workspaces.id
4444
WHERE
4545
NOT deleted AND
46+
-- We can likely remove these conditions since we check above.
47+
-- But it does not hurt to be defensive and make sure future query changes
48+
-- do not break anything.
4649
workspaces.owner_id = @owner_id AND
4750
workspaces.organization_id = @organization_id
4851
;

‎enterprise/coderd/workspacequota_test.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -306,12 +306,12 @@ func TestWorkspaceQuota(t *testing.T) {
306306
})
307307
}
308308

309-
// DB=ci DB_FROM=cikggwjxbths
309+
// nolint:paralleltest // Tests must run serially
310310
func TestWorkspaceSerialization(t *testing.T) {
311311
t.Parallel()
312312

313313
if !dbtestutil.WillUsePostgres() {
314-
panic("We should only run this test with postgres")
314+
t.Skip("Serialization errors only occur in postgres")
315315
}
316316

317317
db, ps := dbtestutil.NewDB(t)
@@ -342,8 +342,6 @@ func TestWorkspaceSerialization(t *testing.T) {
342342
}, user).
343343
Do()
344344

345-
var _ = otherOrg
346-
347345
// TX mixing tests. **DO NOT** run these in parallel.
348346
// The goal here is to mess around with different ordering of
349347
// transactions and queries.
@@ -373,6 +371,7 @@ func TestWorkspaceSerialization(t *testing.T) {
373371
// +------------------------------+------------------+
374372
// pq: could not serialize access due to concurrent update
375373
ctx := testutil.Context(t, testutil.WaitLong)
374+
//nolint:gocritic // testing
376375
ctx = dbauthz.AsSystemRestricted(ctx)
377376

378377
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -426,6 +425,7 @@ func TestWorkspaceSerialization(t *testing.T) {
426425
// +------------------------------+------------------+
427426
// Works!
428427
ctx := testutil.Context(t, testutil.WaitLong)
428+
//nolint:gocritic // testing
429429
ctx = dbauthz.AsSystemRestricted(ctx)
430430

431431
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -456,7 +456,6 @@ func TestWorkspaceSerialization(t *testing.T) {
456456
Isolation: sql.LevelSerializable,
457457
})
458458
assert.NoError(t, err)
459-
460459
}
461460

462461
// Start TX
@@ -491,6 +490,7 @@ func TestWorkspaceSerialization(t *testing.T) {
491490
// +---------------------+----------------------------------+
492491
// pq: could not serialize access due to concurrent update
493492
ctx := testutil.Context(t, testutil.WaitShort)
493+
//nolint:gocritic // testing
494494
ctx = dbauthz.AsSystemRestricted(ctx)
495495

496496
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -540,6 +540,7 @@ func TestWorkspaceSerialization(t *testing.T) {
540540
// +---------------------+----------------------------------+
541541
// pq: could not serialize access due to concurrent update
542542
ctx := testutil.Context(t, testutil.WaitShort)
543+
//nolint:gocritic // testing
543544
ctx = dbauthz.AsSystemRestricted(ctx)
544545

545546
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -583,6 +584,7 @@ func TestWorkspaceSerialization(t *testing.T) {
583584
// +---------------------+----------------------------------+
584585
// Works!
585586
ctx := testutil.Context(t, testutil.WaitShort)
587+
//nolint:gocritic // testing
586588
ctx = dbauthz.AsSystemRestricted(ctx)
587589
var err error
588590

@@ -637,6 +639,7 @@ func TestWorkspaceSerialization(t *testing.T) {
637639
// | | CommitTx() |
638640
// +---------------------+---------------------+
639641
ctx := testutil.Context(t, testutil.WaitLong)
642+
//nolint:gocritic // testing
640643
ctx = dbauthz.AsSystemRestricted(ctx)
641644

642645
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -695,6 +698,7 @@ func TestWorkspaceSerialization(t *testing.T) {
695698
// | | CommitTx() |
696699
// +---------------------+---------------------+
697700
ctx := testutil.Context(t, testutil.WaitLong)
701+
//nolint:gocritic // testing
698702
ctx = dbauthz.AsSystemRestricted(ctx)
699703

700704
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -756,6 +760,7 @@ func TestWorkspaceSerialization(t *testing.T) {
756760
// +---------------------+---------------------+
757761
// pq: could not serialize access due to read/write dependencies among transactions
758762
ctx := testutil.Context(t, testutil.WaitLong)
763+
//nolint:gocritic // testing
759764
ctx = dbauthz.AsSystemRestricted(ctx)
760765

761766
myWorkspace := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{

0 commit comments

Comments
 (0)