Skip to content

Commit

Permalink
Merge #40996
Browse files Browse the repository at this point in the history
40996: sql: fix lease interaction with offline tables r=dt a=pbardea

This updates the lease management code to handle offline tables the
same way dropped tables are handled. We are not supposed to be able
to acquire a lease on offline tables (as well as dropped tables).

Fixes #40951 and therefore #40361.

Release justification: Fixes test flakes where a table descriptor lease
is being taken on an offline table.

Release note: None

Co-authored-by: Paul Bardea <pbardea@gmail.com>
  • Loading branch information
craig[bot] and pbardea committed Sep 23, 2019
2 parents 9d81b08 + 573e349 commit bf42553
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/import_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1597,7 +1597,7 @@ func TestImportIntoCSV(t *testing.T) {
<-importBodyFinished

err := sqlDB.DB.QueryRowContext(ctx, `SELECT 1 FROM t`).Scan(&unused)
if !testutils.IsError(err, "table \"t\" is offline: importing") {
if !testutils.IsError(err, "relation \"t\" does not exist") {
return err
}
return nil
Expand Down
9 changes: 5 additions & 4 deletions pkg/sql/lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (s LeaseStore) jitteredLeaseDuration() time.Duration {

// acquire a lease on the most recent version of a table descriptor.
// If the lease cannot be obtained because the descriptor is in the process of
// being dropped, the error will be errTableDropped.
// being dropped or offline, the error will be of type inactiveTableError.
// The expiration time set for the lease > minExpiration.
func (s LeaseStore) acquire(
ctx context.Context, minExpiration hlc.Timestamp, tableID sqlbase.ID,
Expand Down Expand Up @@ -980,7 +980,7 @@ func (t *tableState) removeInactiveVersions() []*storedTableLease {
}

// If the lease cannot be obtained because the descriptor is in the process of
// being dropped, the error will be errTableDropped.
// being dropped or offline, the error will be of type inactiveTableError.
// The boolean returned is true if this call was actually responsible for the
// lease acquisition.
func acquireNodeLease(ctx context.Context, m *LeaseManager, id sqlbase.ID) (bool, error) {
Expand Down Expand Up @@ -1138,8 +1138,9 @@ func purgeOldVersions(
// active lease, so that it doesn't get released when removeInactives()
// is called below. Release this lease after calling removeInactives().
table, _, err := t.findForTimestamp(ctx, m.clock.Now())
if dropped := err == errTableDropped; dropped || err == nil {
removeInactives(dropped)
if _, ok := err.(*inactiveTableError); ok || err == nil {
isInactive := ok
removeInactives(isInactive)
if table != nil {
s, err := t.release(&table.ImmutableTableDescriptor, m.removeOnceDereferenced())
if err != nil {
Expand Down
18 changes: 11 additions & 7 deletions pkg/sql/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,24 @@ func (p *planner) getVirtualTabler() VirtualTabler {
return p.extendedEvalCtx.VirtualSchemas
}

var errTableDropped = errors.New("table is being dropped")
var errTableAdding = errors.New("table is being added")

type inactiveTableError struct {
error
}

func filterTableState(tableDesc *sqlbase.TableDescriptor) error {
switch tableDesc.State {
case sqlbase.TableDescriptor_DROP:
return errTableDropped
case sqlbase.TableDescriptor_ADD:
return errTableAdding
return inactiveTableError{errors.New("table is being dropped")}
case sqlbase.TableDescriptor_OFFLINE:
err := errors.Errorf("table %q is offline", tableDesc.Name)
if tableDesc.OfflineReason != "" {
return errors.Errorf("table %q is offline: %s", tableDesc.Name, tableDesc.OfflineReason)
err = errors.Errorf("table %q is offline: %s", tableDesc.Name, tableDesc.OfflineReason)
}
return errors.Errorf("table %q is offline", tableDesc.Name)
return inactiveTableError{err}
case sqlbase.TableDescriptor_ADD:
return errTableAdding
case sqlbase.TableDescriptor_PUBLIC:
return nil
default:
Expand Down Expand Up @@ -284,7 +288,7 @@ func (tc *TableCollection) getTableVersion(
// Read the descriptor from the store in the face of some specific errors
// because of a known limitation of AcquireByName. See the known
// limitations of AcquireByName for details.
if err == errTableDropped || err == sqlbase.ErrDescriptorNotFound {
if _, ok := err.(inactiveTableError); ok || err == sqlbase.ErrDescriptorNotFound {
return readTableFromStore()
}
// Lease acquisition failed with some other error. This we don't
Expand Down

0 comments on commit bf42553

Please sign in to comment.