-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a test to verify we respect the overall query timeout #16800
Conversation
Signed-off-by: Manan Gupta <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16800 +/- ##
==========================================
- Coverage 68.94% 68.93% -0.02%
==========================================
Files 1566 1566
Lines 201983 201983
==========================================
- Hits 139252 139229 -23
- Misses 62731 62754 +23 ☔ View full report in Codecov by Sentry. |
// TestOverallQueryTimeout tests that the query timeout is applied to the overall execution of a query | ||
// and not just individual routes. | ||
func TestOverallQueryTimeout(t *testing.T) { | ||
utils.SkipIfBinaryIsBelowVersion(t, 21, "vtgate") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we changed our downgrade tests, I don't think we need to include the skip if binary is below version command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would still need this. For upgrade tests running in a PR against release-20.0, we would end up using the code in main
(upgrade target) to run the tests with both vtgate N+1, vttablet N
and vtgate N, vttablet N+1
. For the latter case we don't want to run this test. So, we need this skip line.
Description
As pointed out in #16624, we weren't respecting overall query timeout. An attempt to fix it was made in #16619. An alternate change in #16735 has already fixed the issue. This PR adds a test to our test suite to check that is indeed the case.
Related Issue(s)
Checklist
Deployment Notes