Skip to content

Commit

Permalink
fix: expireStaleLeases can't be singleton (#1357)
Browse files Browse the repository at this point in the history
Because a lease is required to run it, and if the lease is stale it will
never be expired. This is a quick workaround by making the task
parallel, but we should fix this probably by making the task use the
hash ring.
  • Loading branch information
alecthomas authored Apr 29, 2024
1 parent c0e18a3 commit 87c5913
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
5 changes: 4 additions & 1 deletion backend/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,17 @@ func New(ctx context.Context, db *dal.DAL, config Config, runnerScaling scaling.
svc.tasks.Parallel(maybeDevelBackoff(time.Second, time.Second*5), svc.syncRoutes)
svc.tasks.Parallel(maybeDevelBackoff(time.Second*3, time.Second*5, makeBackoff(time.Second*2, time.Second*2)), svc.heartbeatController)
svc.tasks.Parallel(maybeDevelBackoff(time.Second*5, time.Second*5), svc.updateControllersList)
// This should be a singleton task, but because this is the task that
// actually expires the leases used to run singleton tasks, it must be
// parallel.
svc.tasks.Parallel(maybeDevelBackoff(time.Second, time.Second*5), svc.expireStaleLeases)

// Singleton tasks use leases to only run on a single controller.
svc.tasks.Singleton(maybeDevelBackoff(time.Second*20, time.Second*20), svc.reapStaleControllers)
svc.tasks.Singleton(maybeDevelBackoff(time.Second, time.Second*10), svc.reapStaleRunners)
svc.tasks.Singleton(maybeDevelBackoff(time.Second, time.Second*20), svc.releaseExpiredReservations)
svc.tasks.Singleton(maybeDevelBackoff(time.Second, time.Second*5), svc.reconcileDeployments)
svc.tasks.Singleton(maybeDevelBackoff(time.Second, time.Second*5), svc.reconcileRunners)
svc.tasks.Singleton(maybeDevelBackoff(time.Second, time.Second*5), svc.expireStaleLeases)
return svc, nil
}

Expand Down
6 changes: 6 additions & 0 deletions backend/controller/dal/lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ func leaseExists(t *testing.T, conn sql.DBI, idempotencyKey uuid.UUID, key lease
}

func TestLease(t *testing.T) {
if testing.Short() {
t.Skip("skipping test in short mode")
}
ctx := log.ContextWithNewDefaultLogger(context.Background())
conn := sqltest.OpenForTesting(ctx, t)
dal, err := New(ctx, conn)
Expand Down Expand Up @@ -57,6 +60,9 @@ func TestLease(t *testing.T) {
}

func TestExpireLeases(t *testing.T) {
if testing.Short() {
t.Skip("skipping test in short mode")
}
ctx := log.ContextWithNewDefaultLogger(context.Background())
conn := sqltest.OpenForTesting(ctx, t)
dal, err := New(ctx, conn)
Expand Down

0 comments on commit 87c5913

Please sign in to comment.