roachtest: set acceptance/version-upgrade timeout to 30m #41636
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When adding aggressive consistency checks, I noticed that acceptance/version-upgrade got pushed over the limit. Release note: None
|
This change is |
andreimatei
reviewed
Oct 16, 2019
There was a problem hiding this comment.
LGTM
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)
|
bors r=andreimatei |
craig bot
pushed a commit
that referenced
this pull request
Oct 16, 2019
41509: client/kv: keep transaction UserPriority in sync across client.Txn and TxnCoordSender r=nvanbenschoten a=nvanbenschoten Informs #32508. Before this change, a transaction's UserPriority could diverge between the transaction's client.Txn and its TxnCoordSender. This resulted in the need for ugly code (https://github.com/cockroachdb/cockroach/blob/57b02ddc74004e39a411084e05a20dccd93c3a22/pkg/kv/txn_coord_sender.go#L786), had bugs where a transaction could be restarted with the wrong user priority, and resulted in redundant calls to `roachpb.MakePriority`. This last issue had a runtime cost. We saw the redundant calls to `roachpb.MakePriority` show up in profiles. For instance, it was responsible for **0.32%** of a CPU profile when running Sysbench's `oltp_point_select` workload, mainly due to how slow `rand.ExpFloat64` is on the global rand object. This commit fixes this by keeping the priority in-sync between the `client.Txn` and the `kv.TxnCoordSender`. A side-effect of this is that we no longer perform redundant calls to `roachpb.MakePriority`. Release note: None 41576: build: Upgrade to go 1.12.10 r=petermattis a=bobvawter build: Upgrade to go 1.12.10 This change updates the golang version to 1.12.10 to pick up a security fix in the net/http package. This also picks up a URL parsing security fix in 1.12.8 that eliminates the use of named ports. See the relevant discussion in golang/go@3226f2d The change in URL parsing necessitates a test fix. Any customer impact should be minimal, given the rarity of named ports in common practice, and can be fixed by simply switching to numeric ports. Per the [checklist](build/README.md): * [X] Adjust version in Docker image * [X] Rebuild the Docker image and bump the version in builder.sh accordingly * [ ] ~Bump the version in go-version-check.sh~ (Patch release, not necessary) * [X] Bump the default installed version of Go in bootstrap-debian.sh Fixes: #40939 X-Ref: #23481 Release note (build change): The golang runtime has been upgraded to 1.12.10. Release note (general change): The use of named ports in URLs has been eliminated due to URL parsing ambiguities that were fixed in go 1.12.8. Any impact can be mitigated with the use of numeric port numbers. 41632: builtins: allow full consistency check w/o nuking cluster r=andreimatei a=tbg Previously, triggering a full consistency check would have potential to harm the cluster due to triggering checks with full DistSender parallelism. Limit to one check at a time for a (much slower, but) less harmful check (that we can potentially emit in roachtests). Release note: None 41636: roachtest: set acceptance/version-upgrade timeout to 30m r=andreimatei a=tbg When adding aggressive consistency checks, I noticed that acceptance/version-upgrade got pushed over the limit. Release note: None Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Bob Vawter <bob@cockroachlabs.com> Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When adding aggressive consistency checks, I noticed that
acceptance/version-upgrade got pushed over the limit.
Release note: None