-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
kvserver: relax resolved ts push interval under testrace #50294
kvserver: relax resolved ts push interval under testrace #50294
Conversation
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.
Could you also go through the tests that use these and pepper them with SET statement_timeout='30s'
so that when this fails again (as I'm sure it will when we change $something) we don't time out the whole package?
Fixed cockroachdb#50091 Some rangefeed tests had a really bad time under stressrace on TC machines (presumably overloaded machines because of the tests of 4 packages running concurrently). These tests were setting very low thresholds for the resolved timestamp transaction pusher, and so transactions in these tests were constantly retrying because the resolved timestamp sweeper kept pushing them and, by the time they refreshed, they got pushed again. This patch bumps the thresholds when running under race. On a GPC n1-standard-8 machine, running TestClosedTimestampCanServeWhConflictingIntent under stressrace with concurrency 8 made the test never complete. Now it completes reliably. Release note: None
ca2bb69
to
b79462a
Compare
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.
done
I've also noticed that a bunch of tests calling setupTestClusterForClosedTimestampTesting()
were skipped under race even before your extra skipping the other day. I intend to see if the can now be unskipped, but separately.
Reviewable status: complete! 0 of 0 LGTMs obtained
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.
bors r+
Reviewable status: complete! 0 of 0 LGTMs obtained
Build succeeded |
A bunch of related tests using setupTestClusterForClosedTimestampTesting() were skipped under testrace because the transaction limits set for these TestClusters don't go well with testrace and loaded machines. This was done inconsistently (not all such tests were skipped). The relevant limits have been tweaked in cockroachdb#50294, so these tests should be good now. Release note: None
Fixed #50091
Some rangefeed tests had a really bad time under stressrace on TC
machines (presumably overloaded machines because of the tests of 4
packages running concurrently). These tests were setting very low
thresholds for the resolved timestamp transaction pusher, and so
transactions in these tests were constantly retrying because the
resolved timestamp sweeper kept pushing them and, by the time they
refreshed, they got pushed again. This patch bumps the thresholds when
running under race.
On a GPC n1-standard-8 machine, running
TestClosedTimestampCanServeWhConflictingIntent under stressrace with
concurrency 8 made the test never complete. Now it completes reliably.
Release note: None