Skip to content

Commit

Permalink
Merge pull request juju#18472 from manadart/dqlite-rename-with-lease
Browse files Browse the repository at this point in the history
juju#18472

`WithLease` actually uses the application leadership namespace, and so is not really at the lower "lease" level of abstraction, but the higher "leadership" level. The renaming here represents that fact.

In the future it is anticipated that some `WithSingular` guarantee will need to be sought for service logic requiring execution under the singular controller lease. At that time the lease manager acquisition will be parameterised with the appropriate namespace, but that work is delayed until need arises.
  • Loading branch information
jujubot authored Dec 3, 2024
2 parents 74fafde + ec86ea8 commit 209fcec
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
15 changes: 8 additions & 7 deletions domain/leaseservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ func NewLeaseService(leaseChecker lease.ModelLeaseManagerGetter) *LeaseService {
}
}

// WithLease executes the closure function if the holder to the lease is
// held. As soon as that isn't the case, the context is cancelled and the
// function returns.
// WithLeader executes the closure function if the input unit is leader of the
// input application.
// As soon as that isn't the case, the context is cancelled and the function
// returns.
// The context must be passed to the closure function to ensure that the
// cancellation is propagated to the closure.
func (s *LeaseService) WithLease(
ctx context.Context, leaseName, holderName string, fn func(context.Context) error,
func (s *LeaseService) WithLeader(
ctx context.Context, appName, unitName string, fn func(context.Context) error,
) error {
// Holding the lease is quite a complex operation, so we need to ensure that
// the context is not cancelled before we start the operation.
Expand Down Expand Up @@ -67,7 +68,7 @@ func (s *LeaseService) WithLease(
go func() {
// This guards against the case that the lease has changed state
// before we run the function.
err := leaseChecker.WaitUntilExpired(waitCtx, leaseName, start)
err := leaseChecker.WaitUntilExpired(waitCtx, appName, start)

// Ensure that the lease context is cancelled when the wait has
// completed. We do this as quick as possible to ensure that the
Expand Down Expand Up @@ -104,7 +105,7 @@ func (s *LeaseService) WithLease(
// Ensure that the lease is held by the holder before proceeding.
// We're guaranteed that the lease is held by the holder, otherwise the
// context will have been cancelled.
token := leaseChecker.Token(leaseName, holderName)
token := leaseChecker.Token(appName, unitName)
if err := token.Check(); err != nil {
return internalerrors.Errorf("checking lease token: %w", err)
}
Expand Down
16 changes: 8 additions & 8 deletions domain/leaseservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type leaseServiceSuite struct {

var _ = gc.Suite(&leaseServiceSuite{})

func (s *leaseServiceSuite) TestWithLease(c *gc.C) {
func (s *leaseServiceSuite) TestWithLeader(c *gc.C) {
defer s.setupMocks(c).Finish()

// Done is triggered when the lease function is done.
Expand Down Expand Up @@ -54,7 +54,7 @@ func (s *leaseServiceSuite) TestWithLease(c *gc.C) {
service := NewLeaseService(s.modelLeaseManager)

var called bool
err := service.WithLease(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
err := service.WithLeader(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
defer close(done)
called = true
return ctx.Err()
Expand All @@ -63,7 +63,7 @@ func (s *leaseServiceSuite) TestWithLease(c *gc.C) {
c.Check(called, jc.IsTrue)
}

func (s *leaseServiceSuite) TestWithLeaseWaitReturnsError(c *gc.C) {
func (s *leaseServiceSuite) TestWithLeaderWaitReturnsError(c *gc.C) {
defer s.setupMocks(c).Finish()

s.leaseChecker.EXPECT().WaitUntilExpired(gomock.Any(), "leaseName", gomock.Any()).DoAndReturn(
Expand All @@ -75,15 +75,15 @@ func (s *leaseServiceSuite) TestWithLeaseWaitReturnsError(c *gc.C) {
service := NewLeaseService(s.modelLeaseManager)

var called bool
err := service.WithLease(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
err := service.WithLeader(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
called = true
return ctx.Err()
})
c.Assert(err, jc.ErrorIs, context.Canceled)
c.Check(called, jc.IsFalse)
}

func (s *leaseServiceSuite) TestWithLeaseWaitHasLeaseChange(c *gc.C) {
func (s *leaseServiceSuite) TestWithLeaderWaitHasLeaseChange(c *gc.C) {
defer s.setupMocks(c).Finish()

done := make(chan struct{})
Expand Down Expand Up @@ -121,7 +121,7 @@ func (s *leaseServiceSuite) TestWithLeaseWaitHasLeaseChange(c *gc.C) {
// The lease function should be a long running function.

var called bool
err := service.WithLease(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
err := service.WithLeader(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
called = true

// Notify to everyone that we're running.
Expand All @@ -143,7 +143,7 @@ func (s *leaseServiceSuite) TestWithLeaseWaitHasLeaseChange(c *gc.C) {
c.Check(called, jc.IsTrue)
}

func (s *leaseServiceSuite) TestWithLeaseFailsOnWaitCheck(c *gc.C) {
func (s *leaseServiceSuite) TestWithLeaderFailsOnWaitCheck(c *gc.C) {
defer s.setupMocks(c).Finish()

done := make(chan struct{})
Expand Down Expand Up @@ -173,7 +173,7 @@ func (s *leaseServiceSuite) TestWithLeaseFailsOnWaitCheck(c *gc.C) {
// The lease function should be a long running function.

var called bool
err := service.WithLease(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
err := service.WithLeader(context.Background(), "leaseName", "holderName", func(ctx context.Context) error {
called = true
return nil
})
Expand Down

0 comments on commit 209fcec

Please sign in to comment.