From 95df27342e6fa8613191a35e249fb1dc04238a41 Mon Sep 17 00:00:00 2001 From: Bergin Dedej Date: Tue, 17 Dec 2024 17:25:45 +0000 Subject: [PATCH] sql: TestReacquireLeaseOnRestart fix Previously we had an extra condition in an if statement which caused this failure. This was the condition `errors.Is(err, errRenewLease)` it is now removed and we now set toRelease to nil when `session != nil` Fixes: #129421 Release note: none --- pkg/sql/catalog/lease/descriptor_state.go | 5 +++-- pkg/sql/catalog/lease/lease.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/sql/catalog/lease/descriptor_state.go b/pkg/sql/catalog/lease/descriptor_state.go index ed1dc290a691..534617f3c7a1 100644 --- a/pkg/sql/catalog/lease/descriptor_state.go +++ b/pkg/sql/catalog/lease/descriptor_state.go @@ -172,6 +172,7 @@ func (t *descriptorState) upsertLeaseLocked( } if session != nil { s.mu.lease.sessionID = session.ID().UnsafeBytes() + toRelease = nil } if log.ExpensiveLogEnabled(ctx, 2) { log.VEventf(ctx, 2, "replaced lease: %s with %s", toRelease, s.mu.lease) @@ -259,8 +260,8 @@ func (t *descriptorState) release(ctx context.Context, s *descriptorVersionState // when the refcount drops to 0). If so, we'll need to mark the lease as // invalid. removeOnceDereferenced := - // Release from the store if the descriptor has been dropped or taken - // offline. + // Release from the store if the descriptor has been dropped or taken + // offline. t.mu.takenOffline || // Release from the store if the lease is not for the latest // version; only leases for the latest version can be acquired. diff --git a/pkg/sql/catalog/lease/lease.go b/pkg/sql/catalog/lease/lease.go index f30ea8b213aa..1cf705ee1a4e 100644 --- a/pkg/sql/catalog/lease/lease.go +++ b/pkg/sql/catalog/lease/lease.go @@ -821,7 +821,7 @@ func purgeOldVersions( // if we hit this case. Note: This scenario is impossible to hit in the real // world since the lease duration is never set to 0. desc, _, err := t.findForTimestamp(ctx, m.storage.clock.Now()) - if isInactive := catalog.HasInactiveDescriptorError(err); err == nil || isInactive || errors.Is(err, errRenewLease) { + if isInactive := catalog.HasInactiveDescriptorError(err); err == nil || isInactive { removeInactives(isInactive) if desc != nil { t.release(ctx, desc)