Skip to content

Commit

Permalink
Merge #134665 #134682
Browse files Browse the repository at this point in the history 
134665: catalog/lease: Improve reliability of TestLeaseRenewedAutomatically r=spilchen a=spilchen

The TestLeaseRenewedAutomatically test is designed to verify that an expired lease is reacquired automatically. However, background processes acquiring leases caused occasional test flakiness. Efforts to limit this interference, such as disabling autostats, were only partially effective, as other background activity still sometimes caused issues. Rather than attempting to track every background process, I modified the test to focus lease checks exclusively on the two tables under test. This improved reliability in my local runs since it was the database descriptor lease, not the leases for the two tables, that was being acquired. This adjustment preserves the test’s intent while enhancing its stability.

Epic: none
Closes #133030
Release note: none

134682: go.mod: bump Pebble to 353a0367f335 r=RaduBerinde a=RaduBerinde

Changes:

 * [`353a0367`](cockroachdb/pebble@353a0367) metamorphic: increase per-operation timeouts
 * [`501eb167`](cockroachdb/pebble@501eb167) block: improve FlushGovernor logic
 * [`2da617a0`](cockroachdb/pebble@2da617a0) sstable: fix LazyFetcher lifetime
 * [`d65b21a2`](cockroachdb/pebble@d65b21a2) tool: print sstable format in properties

Informs #134420:
```
RefreshRange/mixed-case/refresh_window=[0.00,0.00]-8     122µs ± 0%      82µs ± 1%  -32.46%  (p=0.000 n=7+8)
```

Release note: none.
Epic: none.

Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
  • Loading branch information
@spilchen @RaduBerinde
3 people committed Nov 8, 2024
3 parents 351bc0d + 7a5c04e + 993533b commit b1901b5
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 10 deletions.
6 changes: 3 additions & 3 deletions DEPS.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1858,10 +1858,10 @@ def go_deps():
patches = [
"@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
],
sha256 = "60c0ff92702935c3382230e5fff9cb0494ed05a8cc58fe971b6d3ed69efccedf",
strip_prefix = "github.com/cockroachdb/[email protected]20241104213112-1af22dc322eb",
sha256 = "469c3e059d4edd2907956850b051c923841fa05fd17a8b390c06c11d376e937f",
strip_prefix = "github.com/cockroachdb/[email protected]20241108172819-353a0367f335",
urls = [
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20241104213112-1af22dc322eb.zip",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20241108172819-353a0367f335.zip",
],
)
go_repository(
Expand Down
2 changes: 1 addition & 1 deletion build/bazelutil/distdir_files.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ DISTDIR_FILES = {
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/metamorphic/com_github_cockroachdb_metamorphic-v0.0.0-20231108215700-4ba948b56895.zip": "28c8cf42192951b69378cf537be5a9a43f2aeb35542908cc4fe5f689505853ea",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20241104213112-1af22dc322eb.zip": "60c0ff92702935c3382230e5fff9cb0494ed05a8cc58fe971b6d3ed69efccedf",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20241108172819-353a0367f335.zip": "469c3e059d4edd2907956850b051c923841fa05fd17a8b390c06c11d376e937f",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20220803192808-1806698b1b7b.zip": "3fda531795c600daf25532a4f98be2a1335cd1e5e182c72789bca79f5f69fcc1",
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ require (
github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
github.com/cockroachdb/gostdlib v1.19.0
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
github.com/cockroachdb/pebble v0.0.0-20241104213112-1af22dc322eb
github.com/cockroachdb/pebble v0.0.0-20241108172819-353a0367f335
github.com/cockroachdb/redact v1.1.5
github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895 h1:XANOgPYtvELQ/h4IrmPAohXqe2pWA8Bwhejr3VQoZsA=
github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895/go.mod h1:aPd7gM9ov9M8v32Yy5NJrDyOcD8z642dqs+F0CeNXfA=
github.com/cockroachdb/pebble v0.0.0-20241104213112-1af22dc322eb h1:v2hka/wH+A3g7oIyjzKVvp8WWIdfhRUGbtH9fPQLJas=
github.com/cockroachdb/pebble v0.0.0-20241104213112-1af22dc322eb/go.mod h1:6/y5AMSI64bnNjTmDbxhWG8Hm2E/NF0KCElcQxUYrvo=
github.com/cockroachdb/pebble v0.0.0-20241108172819-353a0367f335 h1:XNfzUxsCx4TwWbOkX8hf09x9bfsXfypU4tfeBZKr1vY=
github.com/cockroachdb/pebble v0.0.0-20241108172819-353a0367f335/go.mod h1:6/y5AMSI64bnNjTmDbxhWG8Hm2E/NF0KCElcQxUYrvo=
github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30=
github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_refresh_range_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func setupData(
if opts.rwMode == fs.ReadOnly {
readOnlyStr = "_readonly"
}
name := fmt.Sprintf("refresh_range_bench_data_%s_%s%s_%d_%d_%d",
name := fmt.Sprintf("refresh_range_bench_data_%s_%s%s_%d_%d_%d_v2",
verStr, orderStr, readOnlyStr, opts.numKeys, opts.valueBytes, opts.lBaseMaxBytes)

dir := testfixtures.ReuseOrGenerate(b, name, func(dir string) {
Expand Down
17 changes: 15 additions & 2 deletions pkg/sql/catalog/lease/lease_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,9 @@ func TestLeaseRenewedAutomatically(testingT *testing.T) {

var testAcquiredCount int32
var testAcquisitionBlockCount int32
// Descriptor IDs for the two tables under test
var test1ID int32
var test2ID int32
var minimumDescID descpb.ID
var params base.TestClusterArgs
params.ServerArgs.DefaultTestTenant = base.TestDoesNotWorkWithSharedProcessModeButWeDontKnowWhyYet(
Expand Down Expand Up @@ -1310,7 +1313,15 @@ func TestLeaseRenewedAutomatically(testingT *testing.T) {
if id < minimumDescID || typ == lease.AcquireBackground {
return
}
if int32(id) != atomic.LoadInt32(&test1ID) && int32(id) != atomic.LoadInt32(&test2ID) {
return
}
atomic.AddInt32(&testAcquisitionBlockCount, 1)
// The test sets the IDs of the two tables only when we shouldn't block. So if we
// see a block event dump a stack to aid in debugging.
log.Infof(ctx,
"Lease acquisition of ID %d resulted in a block event. Stack trace to follow:\n%s",
id, debug.Stack())
},
},
}
Expand Down Expand Up @@ -1363,8 +1374,10 @@ CREATE TABLE t.test2 ();
}
eo2 := ts2.Expiration(ctx)

// Reset testAcquisitionBlockCount as the first acqusition will always block.
atomic.StoreInt32(&testAcquisitionBlockCount, 0)
// Save off the IDs of the two tables so that we increment testAcquisitionBlockCount
// if we ever block waiting for those leases to expire.
atomic.StoreInt32(&test1ID, int32(test1Desc.GetID()))
atomic.StoreInt32(&test2ID, int32(test2Desc.GetID()))

testutils.SucceedsSoon(t, func() error {
// Acquire another lease by name on test1. At first this will be the
Expand Down

0 comments on commit b1901b5

Please sign in to comment.