multitenant: Increase timeout for kvtenantccl #98997
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
|
This change is |
ajstorm
force-pushed
the
ajstorm-longer-timeout
branch
from
24765b4
to
b4622ea
Compare
knz
approved these changes
Mar 20, 2023
Yes, my commit record was inaccurate. I think we need this increased timeout for non-stress runs as well. Will update the commit record before merging. I also think that we should investigate tenant rate limiting as a possible cause for this timeout. |
The new TestTenantUpgradeInterlock test takes a long time to run (up to a minute locally) and has started timing (the first instance being in a post-merge arm64 test). On the suspicion that it's timing out strictly because it's long running, increase the timeout to see if that resolves the failures. Release note: None Epic: None Fixes: cockroachdb#98986
ajstorm
force-pushed
the
ajstorm-longer-timeout
branch
from
b4622ea
to
95c90f9
Compare
|
bors r=knz |
|
Build failed: |
|
flake is #98020 bors r+ |
|
Build succeeded: |
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this pull request
Mar 22, 2023
TestTenantUpgradeInterlock was skipped with cockroachdb#99121 because it was timing out regularly. In response to the timeouts however, cockroachdb#98997 was merged to increase the timeout duration for the test (since its run length is proportional to the number of migrations in the release, which has been increasing). Unfortuntely, our wires got crossed and the skip was introduced before the test had a chance to run with the new timeout. cockroachdb#98987 hasn't shown a failure with the new timeout length, so it may be the case that the longer timeout has resolved the problem. Reenabling the test to see if it has better success under the new timeout length. Fixes: cockroachdb#98987 Epic: None Release note: None
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this pull request
Mar 22, 2023
TestTenantUpgradeInterlock was skipped with cockroachdb#99121 because it was timing out regularly. In response to the timeouts however, cockroachdb#98997 was merged to increase the timeout duration for the test (since its run length is proportional to the number of migrations in the release, which has been increasing). Unfortuntely, our wires got crossed and the skip was introduced before the test had a chance to run with the new timeout. cockroachdb#98987 hasn't shown a failure with the new timeout length, so it may be the case that the longer timeout has resolved the problem. Reenabling the test to see if it has better success under the new timeout length. Fixes: cockroachdb#98987 Epic: None Release note: None
craig bot
pushed a commit
that referenced
this pull request
Mar 22, 2023
99216: multitenant: re-enable TestTenantUpgradeInterlock r=knz a=ajstorm TestTenantUpgradeInterlock was skipped with #99121 because it was timing out regularly. In response to the timeouts however, #98997 was merged to increase the timeout duration for the test (since its run length is proportional to the number of migrations in the release, which has been increasing). Unfortuntely, our wires got crossed and the skip was introduced before the test had a chance to run with the new timeout. #98987 hasn't shown a failure with the new timeout length, so it may be the case that the longer timeout has resolved the problem. Reenabling the test to see if it has better success under the new timeout length. Fixes: #98987 Epic: None Release note: None Co-authored-by: Adam Storm <storm@cockroachlabs.com>
blathers-crl bot
pushed a commit
that referenced
this pull request
Mar 23, 2023
TestTenantUpgradeInterlock was skipped with #99121 because it was timing out regularly. In response to the timeouts however, #98997 was merged to increase the timeout duration for the test (since its run length is proportional to the number of migrations in the release, which has been increasing). Unfortuntely, our wires got crossed and the skip was introduced before the test had a chance to run with the new timeout. #98987 hasn't shown a failure with the new timeout length, so it may be the case that the longer timeout has resolved the problem. Reenabling the test to see if it has better success under the new timeout length. Fixes: #98987 Epic: None Release note: None
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.
The new TestTenantUpgradeInterlock test takes a long time to run (up to a minute locally) and has started timing out in nightly runs. On the suspicion that it's timing out strictly because it's long running, increase the timeout to see if that resolves the failures.
Release note: None