Skip to content

Commit d6a3f54

Browse files
committed
lease: remvoe compatibility with pre-24.2 lease table
Release note: None
1 parent 892744f commit d6a3f54

File tree

2 files changed

+46
-58
lines changed

2 files changed

+46
-58
lines changed

pkg/sql/catalog/lease/count.go

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ func countLeasesWithDetail(
9494
) (countDetail, error) {
9595
var whereClause []string
9696
forceMultiRegionQuery := false
97-
useBytesOnRetry := false
9897
for _, t := range versions {
9998
versionClause := ""
10099
if !forAnyVersion {
@@ -120,19 +119,13 @@ func countLeasesWithDetail(
120119
// entire table.
121120
if (cachedDatabaseRegions != nil && cachedDatabaseRegions.IsMultiRegion()) ||
122121
forceMultiRegionQuery {
123-
// If we are injecting a raw leases descriptors, that will not have the enum
124-
// type set, so convert the region to byte equivalent physical representation.
125-
detail, err = countLeasesByRegion(ctx, txn, prober, regionMap, cachedDatabaseRegions,
126-
useBytesOnRetry, at, whereClause)
122+
detail, err = countLeasesByRegion(ctx, txn, prober, regionMap, at, whereClause)
127123
} else {
128124
detail, err = countLeasesNonMultiRegion(ctx, txn, at, whereClause)
129125
}
130126
// If any transient region column errors occur then we should retry the count query.
131127
if isTransientRegionColumnError(err) {
132128
forceMultiRegionQuery = true
133-
// If the query was already multi-region aware, then the system database is MR,
134-
// but our lease descriptor has not been upgraded yet.
135-
useBytesOnRetry = cachedDatabaseRegions != nil && cachedDatabaseRegions.IsMultiRegion()
136129
return txn.KV().GenerateForcedRetryableErr(ctx, "forcing retry once with MR columns")
137130
}
138131

@@ -187,15 +180,10 @@ func countLeasesByRegion(
187180
txn isql.Txn,
188181
prober regionliveness.Prober,
189182
regionMap regionliveness.LiveRegions,
190-
cachedDBRegions regionliveness.CachedDatabaseRegions,
191-
convertRegionsToBytes bool,
192183
at hlc.Timestamp,
193184
whereClauses []string,
194185
) (countDetail, error) {
195186
regionClause := "crdb_region=$2::system.crdb_internal_region"
196-
if convertRegionsToBytes {
197-
regionClause = "crdb_region=$2"
198-
}
199187
stmt := fmt.Sprintf(
200188
`SELECT %[1]s FROM system.public.lease AS OF SYSTEM TIME '%[2]s' WHERE %[3]s `,
201189
getCountLeaseColumns(),
@@ -204,28 +192,13 @@ func countLeasesByRegion(
204192
)
205193
var detail countDetail
206194
if err := regionMap.ForEach(func(region string) error {
207-
regionEnumValue := region
208-
// The leases table descriptor injected does not have the type of the column
209-
// set to the region enum type. So, instead convert the logical value to
210-
// the physical one for comparison.
211-
// TODO(fqazi): In 24.2 when this table format is default we can stop using
212-
// synthetic descriptors and use the first code path.
213-
if convertRegionsToBytes {
214-
regionTypeDesc := cachedDBRegions.GetRegionEnumTypeDesc().AsRegionEnumTypeDescriptor()
215-
for i := 0; i < regionTypeDesc.NumEnumMembers(); i++ {
216-
if regionTypeDesc.GetMemberLogicalRepresentation(i) == region {
217-
regionEnumValue = string(regionTypeDesc.GetMemberPhysicalRepresentation(i))
218-
break
219-
}
220-
}
221-
}
222195
var values tree.Datums
223196
queryRegionRows := func(countCtx context.Context) error {
224197
var err error
225198
values, err = txn.QueryRowEx(
226199
countCtx, "count-leases", txn.KV(),
227200
sessiondata.NodeUserSessionDataOverride,
228-
stmt, at.GoTime(), regionEnumValue,
201+
stmt, at.GoTime(), region,
229202
)
230203
return err
231204
}

pkg/sql/catalog/lease/lease.go

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descbuilder"
3333
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
3434
"github.com/cockroachdb/cockroach/pkg/sql/catalog/internal/catkv"
35-
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
3635
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
3736
"github.com/cockroachdb/cockroach/pkg/sql/enum"
3837
"github.com/cockroachdb/cockroach/pkg/sql/isql"
@@ -1346,7 +1345,7 @@ type Manager struct {
13461345
settings *cluster.Settings
13471346
mu struct {
13481347
syncutil.Mutex
1349-
// TODO(james): Track size of leased descriptors in memory.
1348+
13501349
descriptors map[descpb.ID]*descriptorState
13511350

13521351
// Session based leases that will be removed with expiry, since
@@ -2732,27 +2731,36 @@ func (m *Manager) deleteOrphanedLeasesFromStaleSession(
27322731

27332732
var distinctSessions []tree.Datums
27342733
aostTime := hlc.Timestamp{WallTime: initialTimestamp}
2735-
distinctSessionQuery := `SELECT DISTINCT(session_id) FROM system.lease AS OF SYSTEM TIME %s WHERE crdb_region=$1 AND NOT crdb_internal.sql_liveness_is_alive(session_id, true) LIMIT $2`
2736-
syntheticDescriptors := catalog.Descriptors{systemschema.LeaseTable()}
27372734
const limit = 50
27382735

2736+
// Build the query based on whether the system database is multi-region.
2737+
// For multi-region, we need to cast the region parameter to the enum type.
2738+
// For single-region, we use the bytes representation.
2739+
var distinctSessionQuery string
2740+
var regionParam interface{}
2741+
isMultiRegion := bool(multiRegionSystemDb)
2742+
if isMultiRegion {
2743+
distinctSessionQuery = `SELECT DISTINCT(session_id) FROM system.lease AS OF SYSTEM TIME %s WHERE crdb_region=$1::system.crdb_internal_region AND NOT crdb_internal.sql_liveness_is_alive(session_id, true) LIMIT $2`
2744+
regionParam = region
2745+
} else {
2746+
distinctSessionQuery = `SELECT DISTINCT(session_id) FROM system.lease AS OF SYSTEM TIME %s WHERE crdb_region=$1 AND NOT crdb_internal.sql_liveness_is_alive(session_id, true) LIMIT $2`
2747+
regionParam = enum.One
2748+
}
2749+
27392750
totalSessionsProcessed := 0
27402751
totalLeasesDeleted := 0
27412752

27422753
for r := retry.StartWithCtx(ctx, retryOpts); r.Next(); {
27432754
// Get a list of distinct, dead session IDs that exist in the system.lease
27442755
// table.
2745-
err = ex.WithSyntheticDescriptors(syntheticDescriptors, func() error {
2746-
distinctSessions, err = ex.QueryBufferedEx(ctx,
2747-
"query-lease-table-dead-sessions",
2748-
nil,
2749-
sessiondata.NodeUserSessionDataOverride,
2750-
fmt.Sprintf(distinctSessionQuery, aostTime.AsOfSystemTime()),
2751-
region,
2752-
limit,
2753-
)
2754-
return err
2755-
})
2756+
distinctSessions, err = ex.QueryBufferedEx(ctx,
2757+
"query-lease-table-dead-sessions",
2758+
nil,
2759+
sessiondata.NodeUserSessionDataOverride,
2760+
fmt.Sprintf(distinctSessionQuery, aostTime.AsOfSystemTime()),
2761+
regionParam,
2762+
limit,
2763+
)
27562764
if err != nil {
27572765
if !startup.IsRetryableReplicaError(err) {
27582766
log.Dev.Warningf(ctx, "unable to read session IDs for orphaned leases: %v", err)
@@ -2767,7 +2775,7 @@ func (m *Manager) deleteOrphanedLeasesFromStaleSession(
27672775
// Delete rows in our lease table with orphaned sessions.
27682776
for _, sessionRow := range distinctSessions {
27692777
sessionID := sqlliveness.SessionID(tree.MustBeDBytes(sessionRow[0]))
2770-
sessionLeasesDeleted, err := deleteLeaseWithSessionIDWithBatch(ctx, ex, retryOpts, syntheticDescriptors, sessionID, region, limit)
2778+
sessionLeasesDeleted, err := deleteLeaseWithSessionIDWithBatch(ctx, ex, retryOpts, isMultiRegion, sessionID, regionParam, limit)
27712779
if err != nil {
27722780
log.Dev.Warningf(ctx, "unable to delete orphaned leases for session %s: %v", sessionID, err)
27732781
break
@@ -2802,25 +2810,32 @@ func deleteLeaseWithSessionIDWithBatch(
28022810
ctx context.Context,
28032811
ex isql.Executor,
28042812
retryOpts retry.Options,
2805-
syntheticDescriptors catalog.Descriptors,
2813+
multiRegionSystemDb bool,
28062814
sessionID sqlliveness.SessionID,
2807-
region string,
2815+
regionParam interface{},
28082816
batchSize int,
28092817
) (int, error) {
2818+
// Build the query based on whether the system database is multi-region.
2819+
// For multi-region, we need to cast the region parameter to the enum type.
2820+
// For single-region, we use the bytes representation.
2821+
var deleteOrphanedQuery string
2822+
if multiRegionSystemDb {
2823+
deleteOrphanedQuery = `DELETE FROM system.lease WHERE session_id=$1 AND crdb_region=$2::system.crdb_internal_region LIMIT $3`
2824+
} else {
2825+
deleteOrphanedQuery = `DELETE FROM system.lease WHERE session_id=$1 AND crdb_region=$2 LIMIT $3`
2826+
}
2827+
28102828
totalDeleted := 0
28112829
for r := retry.StartWithCtx(ctx, retryOpts); r.Next(); {
28122830
var rowsDeleted int
2813-
deleteOrphanedQuery := `DELETE FROM system.lease WHERE session_id=$1 AND crdb_region=$2 LIMIT $3`
2814-
if err := ex.WithSyntheticDescriptors(syntheticDescriptors, func() error {
2815-
var err error
2816-
rowsDeleted, err = ex.ExecEx(ctx, "delete-orphaned-leases-by-session", nil,
2817-
sessiondata.NodeUserSessionDataOverride,
2818-
deleteOrphanedQuery,
2819-
sessionID.UnsafeBytes(),
2820-
region,
2821-
batchSize)
2822-
return err
2823-
}); err != nil {
2831+
var err error
2832+
rowsDeleted, err = ex.ExecEx(ctx, "delete-orphaned-leases-by-session", nil,
2833+
sessiondata.NodeUserSessionDataOverride,
2834+
deleteOrphanedQuery,
2835+
sessionID.UnsafeBytes(),
2836+
regionParam,
2837+
batchSize)
2838+
if err != nil {
28242839
if !startup.IsRetryableReplicaError(err) {
28252840
return totalDeleted, err
28262841
}

0 commit comments

Comments
 (0)