diff --git a/pkg/ccl/multitenantccl/tenantcostclient/query_ru_estimate_test.go b/pkg/ccl/multitenantccl/tenantcostclient/query_ru_estimate_test.go index a9eca84e8f3a..b9cd7fc138cc 100644 --- a/pkg/ccl/multitenantccl/tenantcostclient/query_ru_estimate_test.go +++ b/pkg/ccl/multitenantccl/tenantcostclient/query_ru_estimate_test.go @@ -195,7 +195,7 @@ func TestEstimateQueryRUConsumption(t *testing.T) { // Check the estimated RU aggregate for all the queries against the actual // measured RU consumption for the tenant. tenantMeasuredRUs = getTenantRUs() - tenantStartRUs - const deltaFraction = 0.05 + const deltaFraction = 0.25 allowedDelta := tenantMeasuredRUs * deltaFraction require.InDeltaf(t, tenantMeasuredRUs, tenantEstimatedRUs, allowedDelta, "estimated RUs (%d) were not within %f RUs of the expected value (%f)", diff --git a/pkg/kv/kvprober/kvprober_integration_test.go b/pkg/kv/kvprober/kvprober_integration_test.go index 4a2a3b6c9c0f..1fb9d30eb6f4 100644 --- a/pkg/kv/kvprober/kvprober_integration_test.go +++ b/pkg/kv/kvprober/kvprober_integration_test.go @@ -42,7 +42,6 @@ func TestProberDoesReadsAndWrites(t *testing.T) { defer log.Scope(t).Close(t) skip.UnderShort(t) - skip.WithIssue(t, 109564) ctx := context.Background() diff --git a/pkg/kv/kvprober/planner.go b/pkg/kv/kvprober/planner.go index f38533b47e8d..9f02283b4b01 100644 --- a/pkg/kv/kvprober/planner.go +++ b/pkg/kv/kvprober/planner.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" ) @@ -34,8 +35,6 @@ type Step struct { // planner abstracts deciding on ranges to probe. It is public to integration // test from kvprober_test. -// -// planner is NOT thread-safe. type planner interface { // Next returns a Step for the prober to execute on. A Step is a decision on // on what range to probe next. Executing on a series of Steps returned by @@ -54,6 +53,14 @@ type planner interface { type meta2Planner struct { db *kv.DB settings *cluster.Settings + + // The production kvprober as of Sep 7th 2023 does not need this mutex, as + // there is one planner per probe loop we run. At the same time, making the + // planner thread-safe makes for easier testing in kvprober_integration_test.go. + // Given that in this context there is no meaningful performance cost to + // synchronizing with a mutex, it makes sense to do it for the test use case + // specifically. + mu syncutil.Mutex // cursor points to a key in meta2 at which scanning should resume when new // plans are needed. cursor roachpb.Key @@ -148,6 +155,10 @@ func newMeta2Planner( // number, we pay a higher CPU cost less often; if it's set to a low number, // we pay a smaller CPU cost more often. func (p *meta2Planner) next(ctx context.Context) (Step, error) { + // See comment where mu is defined. + p.mu.Lock() + defer p.mu.Unlock() + if len(p.plan) == 0 { // Protect CRDB from planning executing too often, due to either issues // with CRDB (meta2 unavailability) or bugs in kvprober.