Skip to content

Commit

Permalink
sql: retry when reading pg_catalog tables with an old timestamp
Browse files Browse the repository at this point in the history
This changes the logic so we will try to use the cacheTS if the current
txn has an older table version than the cache.

Also, if there is a timestamp error, we will now release the lease and
try to acquire a new one, which will cause another timestamp to be used.

Release note (bug fix): Fixed a bug that could cause SHOW TABLES and
other introspection operations to encounter a "batch timestamp
must be after replica GC threshold" error.
  • Loading branch information
rafiss committed Jan 25, 2025
1 parent 93180aa commit d564a71
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
1 change: 1 addition & 0 deletions pkg/sql/rolemembershipcache/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/keys",
"//pkg/kv/kvpb",
"//pkg/roachpb",
"//pkg/security/username",
"//pkg/sql/catalog/descpb",
Expand Down
24 changes: 18 additions & 6 deletions pkg/sql/rolemembershipcache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -83,10 +84,6 @@ func (m *MembershipCache) RunAtCacheReadTS(
if tableDesc.GetVersion() > m.tableVersion {
return
}
if tableDesc.GetVersion() < m.tableVersion {
readTS = tableDesc.GetModificationTime()
return
}
// The cached ts could be from long ago, so use the table modification
// if it's more recent.
if m.readTS.Less(tableDesc.GetModificationTime()) {
Expand All @@ -102,12 +99,27 @@ func (m *MembershipCache) RunAtCacheReadTS(
}

// If we found a historical timestamp to use, run in a different transaction.
return db.DescsTxn(ctx, func(ctx context.Context, newTxn descs.Txn) error {
if err := db.DescsTxn(ctx, func(ctx context.Context, newTxn descs.Txn) error {
if err := newTxn.KV().SetFixedTimestamp(ctx, readTS); err != nil {
return err
}
return f(ctx, newTxn)
})
}); err != nil {
if errors.HasType(err, (*kvpb.BatchTimestampBeforeGCError)(nil)) {
// If we picked a timestamp that has already been GC'd, it means we must
// have an older lease. Release the lease and retry.
txn.Descriptors().ReleaseSpecifiedLeases(ctx, []lease.IDVersion{
{
Name: tableDesc.GetName(),
ID: tableDesc.GetID(),
Version: tableDesc.GetVersion(),
},
})
return m.RunAtCacheReadTS(ctx, db, txn, f)
}
return err
}
return nil
}

// GetRolesForMember looks up all the roles 'member' belongs to (direct and
Expand Down

0 comments on commit d564a71

Please sign in to comment.