remove bench job from extended ci
#136470
Assignees
Labels
A-ci
Continuous Integration
branch-release-23.1
Used to mark GA and release blockers, technical advisories, and bugs for 23.1
branch-release-23.2
Used to mark GA and release blockers, technical advisories, and bugs for 23.2
branch-release-24.1
Used to mark GA and release blockers, technical advisories, and bugs for 24.1
branch-release-24.2
Used to mark GA and release blockers, technical advisories, and bugs for 24.2
branch-release-24.3
Used to mark GA and release blockers, technical advisories, and bugs for 24.3
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-dev-inf
Comments
msbutler
added
C-enhancement
craig bot
pushed a commit
that referenced
this issue
Dec 9, 2024
136780: stats: use no-op DatumAlloc when decoding EncDatums r=yuzefovich a=yuzefovich
**tree: make nil DatumAlloc act as a no-op**
This commit makes so that `nil` `DatumAlloc` value makes the alloc struct a no-op, i.e. each datum type passed by value results in a separate allocation. This can be useful when the callers will keep only a subset of all allocated datums.
Release note: None
**stats: use no-op DatumAlloc when decoding EncDatums**
This commit fixes a bounded memory leak that could previously occur due to usage of the DatumAlloc when decoding EncDatums into `tree.Datum`s during table stats collection. We have two use cases for decoded `tree.Datum`s:
- we need it when adding _all_ rows into the sketch (we compute the datum's fingerprint ("hash") to use in the distinct count estimation). This usage is very brief and we don't need the decoded datum afterwards.
- we also need it when sampling _some_ rows when we decide to keep a particular row. In this case, the datum is needed throughout the whole stats collection job (or until it's replaced by another sample) for the histogram bucket computation.
The main observation is that the DatumAlloc allocates datums in batches of 16 objects, and even if at least one of the objects is kept by the sample, then all others from the same batch are as well. We only perform memory accounting for the ones we explicitly keep, yet others would be considered live by the Go runtime, resulting in a bounded memory leak. This behavior has been present since forever, but it became more problematic in 24.2 release with the introduction of dynamically computing the sample size. To go around this problematic behavior this commit uses `nil` DatumAlloc which makes it so that every decoded datum incurs a separate allocation. This will have a minor performance hit and increase in total number of allocations, but at least most of them should be short-lived. Alternatively, we could use an "operating" DatumAlloc during fingerprinting and a no-op during the sampling, but we'd need to explicitly nil out the decoded datum after fingerprinting which would result in decoding some datums twice.
Fixes: #136394.
Release note: None
136901: kvserver: deflake TestStoreRangeMergeConcurrentRequests r=iskettaneh a=iskettaneh
Fixes: #136774
Release note: None
136994: kvserver: deflake TestLeaseRequestFromExpirationToEpochOrLeaderDoesNotRegressExpiration r=iskettaneh a=iskettaneh
This commit deflakes
TestLeaseRequestFromExpirationToEpochOrLeaderDoesNotRegressExpiration by establishing the node liveness record before pausing node liveness heartbeats.
Fixes: #136549
Release Note: None
137039: sql/tests: add `ANALYZE` to sysbench microbenchmark r=mgartner a=mgartner
Collecting table statistics manually during the benchmark setup makes
the performance more representative of a real-world workload without the
variance in results that automatic stats collection would introduce.
Epic: None
Release note: None
137049: teamcity: remove `bench` scripts r=jlinder a=rickystewart
These jobs are very flaky and extremely time-consuming at about an hour per run. Simply delete the scripts.
Closes: #136470
Epic: none
Release note: None
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Ibrahim Kettaneh <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
Based on the specified backports for linked PR #137049, I applied the following new label(s) to this issue: branch-release-23.1, branch-release-23.2, branch-release-24.1, branch-release-24.2, branch-release-24.3. Please adjust the labels as needed to match the branches actually affected by this issue, including adding any known older branches. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
blathers-crl
bot
added
branch-release-23.1
blathers-crl bot
pushed a commit
that referenced
this issue
Dec 9, 2024
These jobs are very flaky and extremely time-consuming at about an hour per run. Simply delete the scripts. Closes: #136470 Epic: none Release note: None
blathers-crl bot
pushed a commit
that referenced
this issue
Dec 9, 2024
These jobs are very flaky and extremely time-consuming at about an hour per run. Simply delete the scripts. Closes: #136470 Epic: none Release note: None
blathers-crl bot
pushed a commit
that referenced
this issue
Dec 9, 2024
These jobs are very flaky and extremely time-consuming at about an hour per run. Simply delete the scripts. Closes: #136470 Epic: none Release note: None
blathers-crl bot
pushed a commit
that referenced
this issue
Dec 9, 2024
These jobs are very flaky and extremely time-consuming at about an hour per run. Simply delete the scripts. Closes: #136470 Epic: none Release note: None
blathers-crl bot
pushed a commit
that referenced
this issue
Dec 9, 2024
These jobs are very flaky and extremely time-consuming at about an hour per run. Simply delete the scripts. Closes: #136470 Epic: none Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While I haven't polled the org, I don't think db engineers investigate and take action if the
benchjob fails because you cant see the benchmark results and because it's incredibly flakey. Furthermore, this job implicitly increases a PRs's time to merge because 1) many engineers wait for extended CI to complete before merging and 2) this job is the longest running extended ci job. I don't know the details, but I imagine we're also spending significant money running this job.Also, @herkolategan, crdb's micro bench infra expert in residence, signed off on this idea here.
Jira issue: CRDB-45088
The text was updated successfully, but these errors were encountered: