roachtest: separate clearrange roachtest with MVCC range tombstones
#86752
Assignees
Labels
Comments
|
cc @cockroachdb/replication |
|
CC @ajwerner |
blathers-crl
bot
added
the
A-storage
aliher1911
added a commit
to aliher1911/cockroach
that referenced
this issue
Sep 26, 2022
Previously clear range tests were run with default server settings. Range tombstones are disabled by default so new functionality in schema change commands is not execised. This commit adds test variant that enables range tombstones. Release note: None Fixes cockroachdb#86752
nicktrav
added
T-kv-replication
and removed
A-storage
|
cc @cockroachdb/replication |
craig bot
pushed a commit
that referenced
this issue
Sep 28, 2022
88735: roachtest: run clearrange test with range tombstones r=erikgrinaker a=aliher1911 Previously clear range tests were run with default server settings. Range tombstones are disabled by default so new functionality in schema change commands is not execised. This commit adds test variant that enables range tombstones. Release note: None Fixes #86752 88911: hlc: improve `Timestamp.IsEmpty()` performance r=sumeerbhola a=erikgrinaker `Timestamp.IsEmpty()` saw a significant performance regression with Go 1.19, in particular with empty timestamps, apparently due to the compiler's use of `memeqbody` for struct comparisons. On `EncodeMVCCKey` benchmarks, the empty timestamp case showed a ~50% regression. This patch changes `IsEmpty()` to avoid struct comparison, instead comparing the member fields directly. This yields a significant performance improvement (using Go 1.19): ``` name old time/op new time/op delta TimestampIsEmpty/empty-24 6.90ns ± 2% 0.79ns ± 0% -88.56% (p=0.000 n=10+9) TimestampIsEmpty/walltime-24 2.97ns ± 1% 0.75ns ± 1% -74.90% (p=0.000 n=10+10) TimestampIsEmpty/all-24 2.97ns ± 1% 0.75ns ± 1% -74.77% (p=0.000 n=10+10) ``` This in turn significantly improves `EncodeMVCCKey` performance: ``` name old time/op new time/op delta EncodeMVCCKey/key=empty/ts=empty-24 22.0ns ± 0% 15.7ns ± 5% -28.67% (p=0.000 n=9+10) EncodeMVCCKey/key=empty/ts=walltime-24 21.2ns ± 0% 17.8ns ± 0% -16.42% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=walltime+logical-24 21.8ns ± 0% 18.6ns ± 0% -15.02% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=all-24 22.2ns ± 0% 18.8ns ± 0% -15.07% (p=0.000 n=9+9) EncodeMVCCKey/key=short/ts=empty-24 22.3ns ± 0% 15.8ns ± 0% -29.06% (p=0.000 n=8+10) EncodeMVCCKey/key=short/ts=walltime-24 21.2ns ± 0% 18.0ns ± 0% -15.02% (p=0.000 n=9+8) EncodeMVCCKey/key=short/ts=walltime+logical-24 22.6ns ± 0% 18.8ns ± 0% -16.53% (p=0.000 n=9+8) EncodeMVCCKey/key=short/ts=all-24 22.6ns ± 0% 19.1ns ± 0% -15.32% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=empty-24 65.2ns ± 1% 58.6ns ± 0% -10.11% (p=0.000 n=9+9) EncodeMVCCKey/key=long/ts=walltime-24 62.6ns ± 0% 59.6ns ± 1% -4.72% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=walltime+logical-24 63.5ns ± 0% 60.2ns ± 0% -5.16% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=all-24 63.9ns ± 0% 59.6ns ± 0% -6.81% (p=0.000 n=9+9) ``` It also yields a nice improvement over 22.1 with Go 1.18: ``` name old time/op new time/op delta EncodeMVCCKey/key=empty/ts=empty-24 15.7ns ± 0% 15.7ns ± 5% ~ (p=0.497 n=8+10) EncodeMVCCKey/key=empty/ts=walltime-24 19.5ns ± 0% 17.8ns ± 0% -8.99% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=walltime+logical-24 20.2ns ± 0% 18.6ns ± 0% -8.03% (p=0.000 n=10+10) EncodeMVCCKey/key=empty/ts=all-24 20.4ns ± 0% 18.8ns ± 0% -7.67% (p=0.000 n=9+9) EncodeMVCCKey/key=short/ts=empty-24 16.0ns ± 0% 15.8ns ± 0% -1.47% (p=0.000 n=10+10) EncodeMVCCKey/key=short/ts=walltime-24 20.6ns ± 0% 18.0ns ± 0% -12.51% (p=0.000 n=8+8) EncodeMVCCKey/key=short/ts=walltime+logical-24 20.9ns ± 0% 18.8ns ± 0% -9.82% (p=0.000 n=10+8) EncodeMVCCKey/key=short/ts=all-24 20.9ns ± 0% 19.1ns ± 0% -8.79% (p=0.000 n=9+10) EncodeMVCCKey/key=long/ts=walltime-24 60.4ns ± 0% 59.6ns ± 1% -1.30% (p=0.000 n=9+10) EncodeMVCCKey/key=long/ts=walltime+logical-24 61.4ns ± 0% 60.2ns ± 0% -1.84% (p=0.000 n=10+10) EncodeMVCCKey/key=long/ts=all-24 61.4ns ± 0% 59.6ns ± 0% -2.88% (p=0.000 n=10+9) EncodeMVCCKey/key=long/ts=empty-24 59.1ns ± 0% 58.6ns ± 0% -0.77% (p=0.000 n=10+9) ``` Resolves #88818. Release note: None Co-authored-by: Oleg Afanasyev <[email protected]> Co-authored-by: Erik Grinaker <[email protected]>
blathers-crl bot
pushed a commit
that referenced
this issue
Sep 28, 2022
Previously clear range tests were run with default server settings. Range tombstones are disabled by default so new functionality in schema change commands is not execised. This commit adds test variant that enables range tombstones. Release note: None Fixes #86752
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Once schema GC respects
storage.mvcc.range_tombstones.enabled(#86580), we should have two versions of theclearrangeroachtest: one with and one without the setting enabled.We'll remove this setting for 23.1, so we can remove the additional roachtest version after the branch cut, but it would be good to have this coverage on the release branch.
Jira issue: CRDB-18915
Epic CRDB-2624
The text was updated successfully, but these errors were encountered: