Skip to content

Commit 35a1a5a

Browse files
committed
builtins: decrease wait time for await_statement_hints_cache
Previously, `crdb_internal.await_statement_hints_cache` waited until the hint cache frontier timestamp reached `now()` plus the closed timestamp target duration and refresh interval. The frontier timestamp indicates that we've seen all events up to that timestamp, so it's OK to just wait until `now()` instead. This commit also fixes a flake in the `statement_hint_builtins` logic test due to a test flag `UseRangeTombstonesForPointDeletes`. Epic: None Release note: None
1 parent 0375f11 commit 35a1a5a

File tree

8 files changed

+12
-23
lines changed

8 files changed

+12
-23
lines changed

pkg/sql/faketreeeval/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ go_library(
1010
"//pkg/jobs/jobspb",
1111
"//pkg/roachpb",
1212
"//pkg/security/username",
13-
"//pkg/settings/cluster",
1413
"//pkg/sql/catalog/descpb",
1514
"//pkg/sql/hintpb",
1615
"//pkg/sql/pgwire/pgcode",

pkg/sql/faketreeeval/evalctx.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
1515
"github.com/cockroachdb/cockroach/pkg/roachpb"
1616
"github.com/cockroachdb/cockroach/pkg/security/username"
17-
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1817
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
1918
"github.com/cockroachdb/cockroach/pkg/sql/hintpb"
2019
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -583,7 +582,7 @@ func (ep *DummyEvalPlanner) ClearTableStatsCache() {}
583582
func (ep *DummyEvalPlanner) ClearStatementHintsCache() {}
584583

585584
// AwaitStatementHintsCache is part of the eval.Planner interface.
586-
func (ep *DummyEvalPlanner) AwaitStatementHintsCache(ctx context.Context, st *cluster.Settings) {}
585+
func (ep *DummyEvalPlanner) AwaitStatementHintsCache(ctx context.Context) {}
587586

588587
// RetryCounter is part of the eval.Planner interface.
589588
func (ep *DummyEvalPlanner) RetryCounter() int {

pkg/sql/hints/BUILD.bazel

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ go_library(
1414
"//pkg/kv/kvclient/rangefeed/rangefeedbuffer",
1515
"//pkg/kv/kvclient/rangefeed/rangefeedcache",
1616
"//pkg/kv/kvpb",
17-
"//pkg/kv/kvserver",
18-
"//pkg/kv/kvserver/closedts",
1917
"//pkg/roachpb",
2018
"//pkg/settings",
2119
"//pkg/settings/cluster",

pkg/sql/hints/hint_cache.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed/rangefeedbuffer"
1818
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed/rangefeedcache"
1919
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
20-
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
21-
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts"
2220
"github.com/cockroachdb/cockroach/pkg/roachpb"
2321
"github.com/cockroachdb/cockroach/pkg/settings"
2422
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -381,22 +379,18 @@ var _ rangefeedbuffer.Event = &bufferEvent{}
381379
// with the present. After Await returns, MaybeGetStatementHints should
382380
// accurately reflect all hints that were modified before the call to Await
383381
// (assuming the ctx was not canceled).
384-
func (c *StatementHintsCache) Await(ctx context.Context, st *cluster.Settings) {
385-
// The frontier timestamp comes from the rangefeed, and could be up to
386-
// kv.closed_timestamp.target_duration +
387-
// kv.rangefeed.closed_timestamp_refresh_interval behind the present.
388-
targetDuration := closedts.TargetDuration.Get(&st.SV)
389-
refreshInterval := kvserver.RangeFeedRefreshInterval.Get(&st.SV)
390-
const fudge = 10 * time.Millisecond
391-
waitUntil := c.clock.Now().AddDuration(targetDuration + refreshInterval + fudge).WallTime
382+
func (c *StatementHintsCache) Await(ctx context.Context) {
383+
// Wait in intervals of at least 100 milliseconds to avoid busy-waiting.
384+
const minWait = time.Millisecond * 100
385+
waitUntil := c.clock.Now().WallTime
392386

393387
// Await is only used for testing, so we don't need to wake up immediately. We
394388
// can get away with polling the frontier time.
395-
for frontier := c.frontier.Load(); frontier < waitUntil; frontier = c.frontier.Load() {
389+
for frontier := c.frontier.Load(); frontier <= waitUntil; frontier = c.frontier.Load() {
396390
select {
397391
case <-ctx.Done():
398392
return
399-
case <-time.After(time.Duration(waitUntil-frontier) * time.Nanosecond):
393+
case <-time.After(max(time.Duration(waitUntil-frontier), minWait)):
400394
}
401395
}
402396
}

pkg/sql/logictest/testdata/logic_test/statement_hint_builtins

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# LogicTest: !local-mixed-25.2 !local-mixed-25.3 !local-prepared
2+
# cluster-opt: disable-mvcc-range-tombstones-for-point-deletes
23

34
statement ok
45
CREATE TABLE xy (x INT PRIMARY KEY, y INT, INDEX (y));

pkg/sql/planner.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/cockroachdb/cockroach/pkg/roachpb"
2020
"github.com/cockroachdb/cockroach/pkg/security/username"
2121
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
22-
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
2322
"github.com/cockroachdb/cockroach/pkg/spanconfig"
2423
"github.com/cockroachdb/cockroach/pkg/sql/auditlogging"
2524
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -1099,9 +1098,9 @@ func (p *planner) ClearStatementHintsCache() {
10991098
}
11001099

11011100
// AwaitStatementHintsCache is part of the eval.Planner interface.
1102-
func (p *planner) AwaitStatementHintsCache(ctx context.Context, st *cluster.Settings) {
1101+
func (p *planner) AwaitStatementHintsCache(ctx context.Context) {
11031102
if p.execCfg.StatementHintsCache != nil {
1104-
p.execCfg.StatementHintsCache.Await(ctx, st)
1103+
p.execCfg.StatementHintsCache.Await(ctx)
11051104
}
11061105
}
11071106

pkg/sql/sem/builtins/builtins.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9741,7 +9741,7 @@ WHERE object_id = table_descriptor_id
97419741
Types: tree.ParamTypes{},
97429742
ReturnType: tree.FixedReturnType(types.Void),
97439743
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
9744-
evalCtx.Planner.AwaitStatementHintsCache(ctx, evalCtx.Settings)
9744+
evalCtx.Planner.AwaitStatementHintsCache(ctx)
97459745
return tree.DVoidDatum, nil
97469746
},
97479747
Info: `This function is used to await the statement hints cache on the gateway node`,

pkg/sql/sem/eval/deps.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
1414
"github.com/cockroachdb/cockroach/pkg/roachpb"
1515
"github.com/cockroachdb/cockroach/pkg/security/username"
16-
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1716
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
1817
"github.com/cockroachdb/cockroach/pkg/sql/hintpb"
1918
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
@@ -464,7 +463,7 @@ type Planner interface {
464463

465464
// AwaitStatementHintsCache waits for the node's statement hints cache to
466465
// catch up with recent hint injections.
467-
AwaitStatementHintsCache(ctx context.Context, st *cluster.Settings)
466+
AwaitStatementHintsCache(ctx context.Context)
468467

469468
// RetryCounter is the number of times this statement has been retried.
470469
RetryCounter() int

0 commit comments

Comments
 (0)