Skip to content

fix: prevent db deadlock when workspaces go dormant #10618

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 2 commits into from
Nov 13, 2023
Merged

fix: prevent db deadlock when workspaces go dormant #10618

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efb87ed
add test to prevent deadlock regression
sreya Nov 9, 2023
7532d60
fix: prevent db deadlock when workspaces go dormant
sreya Nov 9, 2023
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
Next Next commit
add test to prevent deadlock regression
  • Loading branch information
@sreya
sreya committed Nov 9, 2023
commit efb87edaf82fd39657d309fee2a9e794a941fe39
81 changes: 81 additions & 0 deletions enterprise/coderd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ import (
"github.com/coder/coder/v2/coderd/autobuild"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/rbac"
agplschedule "github.com/coder/coder/v2/coderd/schedule"
"github.com/coder/coder/v2/coderd/schedule/cron"
"github.com/coder/coder/v2/coderd/util/ptr"
"github.com/coder/coder/v2/codersdk"
entaudit "github.com/coder/coder/v2/enterprise/audit"
"github.com/coder/coder/v2/enterprise/audit/backends"
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
"github.com/coder/coder/v2/enterprise/coderd/license"
"github.com/coder/coder/v2/enterprise/coderd/schedule"
Expand Down Expand Up @@ -309,6 +312,84 @@ func TestWorkspaceAutobuild(t *testing.T) {
require.True(t, ws.LastUsedAt.After(lastUsedAt))
})

// This test serves as a regression prevention for generating
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍 We could rewrite using the new shiny dbfake but this can be a follow-up.

// audit logs in the same transaction the transition workspaces to
// the dormant state. The auditor that is passed to autobuild does
// not use the transaction when inserting an audit log which can
// cause a deadlock.
t.Run("NoDeadlock", func(t *testing.T) {
t.Parallel()

if !dbtestutil.WillUsePostgres() {
t.Skipf("Skipping non-postgres run")
}

var (
ticker = make(chan time.Time)
statCh = make(chan autobuild.Stats)
inactiveTTL = time.Minute
)

const (
maxConns = 3
numWorkspaces = maxConns * 5
)
// This is a bit bizarre but necessary so that we can
// initialize our coderd with a real auditor and limit DB connections
// to simulate deadlock conditions.
db, pubsub, sdb := dbtestutil.NewDBWithSQLDB(t)
// Set MaxOpenConns so we can ensure we aren't inadvertently acquiring
// another connection from within a transaction.
sdb.SetMaxOpenConns(maxConns)
auditor := entaudit.NewAuditor(db, entaudit.DefaultFilter, backends.NewPostgres(db, true))

client, user := coderdenttest.New(t, &coderdenttest.Options{
Options: &coderdtest.Options{
AutobuildTicker: ticker,
AutobuildStats: statCh,
TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()),
Database: db,
Pubsub: pubsub,
Auditor: auditor,
IncludeProvisionerDaemon: true,
},
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1},
},
})

version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
ProvisionPlan: echo.PlanComplete,
ProvisionApply: echo.ApplyComplete,
})
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
ctr.TimeTilDormantMillis = ptr.Ref[int64](inactiveTTL.Milliseconds())
})
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)

workspaces := make([]codersdk.Workspace, 0, numWorkspaces)
for i := 0; i < numWorkspaces; i++ {
ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
require.Equal(t, codersdk.WorkspaceStatusRunning, build.Status)
workspaces = append(workspaces, ws)
}

// Simulate being inactive.
ticker <- time.Now().Add(time.Hour)
stats := <-statCh

// Expect workspace to transition to stopped state for breaching
// failure TTL.
require.Len(t, stats.Transitions, numWorkspaces)
for _, ws := range workspaces {
// The workspace should be dormant.
ws = coderdtest.MustWorkspace(t, client, ws.ID)
require.NotNil(t, ws.DormantAt)
}
})

t.Run("InactiveTTLTooEarly", func(t *testing.T) {
t.Parallel()

Expand Down