Skip to content

Commit daab511

Browse files
committed
rowexec: fix TestUncertaintyErrorIsReturned under race
We just saw a case when `TestUncertaintyErrorIsReturned` failed under race because we got a different DistSQL plan. This seems plausible in case the range cache population (which the test does explicitly) isn't quick enough for some reason, so this commit allows for the DistSQL plan to match the expectation via `SucceedsSoon` (if we happen to get a bad plan, then the following query execution should have the up-to-date range cache). Release note: None
1 parent 895b195 commit daab511

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

pkg/sql/rowexec/processors_test.go

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ import (
4141
"github.com/cockroachdb/cockroach/pkg/testutils"
4242
"github.com/cockroachdb/cockroach/pkg/testutils/distsqlutils"
4343
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
44-
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
4544
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
4645
"github.com/cockroachdb/cockroach/pkg/util/hlc"
4746
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
4847
"github.com/cockroachdb/cockroach/pkg/util/log"
4948
"github.com/cockroachdb/cockroach/pkg/util/randutil"
5049
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
50+
"github.com/cockroachdb/errors"
5151
"github.com/jackc/pgx/v4"
5252
"github.com/stretchr/testify/require"
5353
)
@@ -714,8 +714,6 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
714714
// The default behavior is to enable uncertainty errors for a single random
715715
// node.
716716
overrideErrorOrigin []int
717-
// if non-empty, this test will be skipped.
718-
skip string
719717
}{
720718
{
721719
query: "SELECT * FROM t AS t1 JOIN t AS t2 ON t1.x = t2.x",
@@ -726,8 +724,8 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
726724
expectedPlanURL: "Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzElV1v2jwUgO_fX2Ed6VW3yZTY4TPSJKqSqWlpwgjTJlWoyojLvIY4sx0VVPHfpwBdIW0iwkWbCyTHh8ePfY5PHkH9icAC3x7Y52PE4zuBvoy8a3Rj_xgOzhwXfeg7_tj_OsDIvzgb2h_RNvTTJk6jMx9pghzXtUdo4HlX34bo0nPc7QxFnos0OV2gz0jT0-UEfb-wR_ZmpYFzZaOTPg9mMphb_58AhliEzA3mTIF1AwQwUMBgwgRDIsWUKSVkNvW4DnTCBVgGBh4nqc5eTzBMhWRgPYLmOmJgwTj4GbERC0Im6wZgCJkOeLTG656-Te7ZEjCciyidx8pCC4yysZ8E2ahWJwZMVhhEqp-XUDqYMbDIjpPTB8tY4cO1LgWPt1bmS6vlLQ8XgGEgxH2aoN-Cx0jEFuqRXdclRlI88LDQkOYMzSMNW4Xn9kKwsXeYgMFLdeaNexT3mrhnFsqaOdlWoeyzYxoLGTLJwj3ByeqV7biiJpJ6Nxf4ukojp9LdUyGHFxypWnB1YtTq9PCaI1XMdjLaeLOaaxxp2H6PmmvvydLDE00rJ5oatYOzTKto7Zxh882y3DzSsPMeWe5UaYMjphIRK3ZQ4zByK9VI1opYOGObvqVEKqdsKMV0HbsZemvQ-kXIlN7Mks3AiZ-mlJYsmP_7zuySSCmJFpPMPImWksw9EtkltfIks3x3RoXtNUpRzWISyZOapaRWMamRJ7WOPah2ntQuJXWKnWie1CkldYtJzType-zuOlm530Xi4ZaHYIGxfWqv_Dw9kP0hmKnszvm_xMMaO14m2Y25CyLFMFwH96zPNJNzHnOl-RQsLVO2Wv33NwAA___OinSA",
727725
},
728726
{
729-
// This test reproduces 51458 and should be enabled once that issue is
730-
// fixed.
727+
// Reproduction of not propagating errors to all outputs of the
728+
// hash router (#51458).
731729
query: "SELECT * FROM t JOIN onerow ON t.x = onerow.x",
732730
expectedPlanURL: "Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJy8lW9P4koUxt_fTzE5yY16M0inFPA2McEIN-JFcIFkNzHEjPQAjaXDzkwjxvDdN21xpYV2Kf7pC2WY098855knhxdQPz2wYdDqtC6HxPUngvzX792Qu9aP285Fu0uOm-3BcPCtQ8ng6uK2dULWpf_EdZpc99pdInyU4on0ukSfLsn5en26HJHvV61-KwZ32v-3yFHT5VPJ5_bfR0DBFw52-RwV2HfAgIIJFCoworCQYoxKCRluvUSFbWcJtkHB9ReBDr8eURgLiWC_gHa1h2DDkD942EfuoCwbQMFBzV0vwuuGvl884jNQuBReMPeVTZaUhOvBgoerUpkZMFpREIFeH_FGfngmM65mSWaDwWg1oqA0nyLYbEN3uwm2saIZ0t-4gS-kgxKdBHkUvvmnkh39X3E1uxauj7JsJaV6ONHHDXZyLt3pLPoEFHqBtkmD0YZJG5VU629tVVJtWe9oa4fmriiJRbme7n-nFCslpZ6QwvYPBysajjIzSmXzY_PBMtV_QT6qn5aPaqItc_9LMQtfimmUPvRGzEOlV5LHxAOwEf_bbmJj5rxLvpmSX8mU_wWBqn1aoGqZA2eHoj6qhfAV7jVPjNRJJRY2ic4UY9OUCOQYb6UYR7XxsheBovA4qHS8u160_dctpSXy-e-fgf1JVjbJKkaqZ5PO0iSWJhmbJDObxMw0ysxFnSVQRq5RlUMtZ8VIOZZXi5FyLP83TbIOtrySRlUPNWrr8vJJOUbVipFyjGJbOagVaM_cRG0ZlU-yskn1YqR6NolthbN-cBCscFxNPPF07zpgg7F-Sjv-vD4QvsCnKpyZg5l4irjD50U48SbcU0jhhj9iEzXKueu7SrtjsLUMcLX661cAAAD__3vS9aQ=",
733731
overrideErrorOrigin: []int{0},
@@ -754,14 +752,13 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
754752
}
755753
for _, testCase := range testCases {
756754
t.Run(testCase.query, func(t *testing.T) {
757-
if testCase.skip != "" {
758-
skip.IgnoreLint(t, testCase.skip)
759-
}
760755
func() {
761756
_, err := defaultConn.Exec(ctx, fmt.Sprintf("set vectorize=%s", vectorizeOpt))
762757
require.NoError(t, err)
763-
func() {
764-
// Check distsql plan.
758+
// We allow for the DistSQL plan to be different for some
759+
// time in case the range cache wasn't populated as we
760+
// expected (we've seen this under race in #108250).
761+
testutils.SucceedsSoon(t, func() error {
765762
rows, err := defaultConn.Query(
766763
ctx,
767764
fmt.Sprintf("SELECT info FROM [EXPLAIN (DISTSQL, SHAPE) %s] WHERE info LIKE 'Diagram:%%'", testCase.query),
@@ -771,8 +768,14 @@ func TestUncertaintyErrorIsReturned(t *testing.T) {
771768
rows.Next()
772769
var actualPlanURL string
773770
require.NoError(t, rows.Scan(&actualPlanURL))
774-
require.Equal(t, testCase.expectedPlanURL, actualPlanURL)
775-
}()
771+
if testCase.expectedPlanURL != actualPlanURL {
772+
return errors.Newf(
773+
"DistSQL plans didn't match:\nexpected:%s\nactual: %s",
774+
testCase.expectedPlanURL, actualPlanURL,
775+
)
776+
}
777+
return nil
778+
})
776779

777780
errorOrigin := []int{allNodeIdxs[rng.Intn(len(allNodeIdxs))]}
778781
if testCase.overrideErrorOrigin != nil {

0 commit comments

Comments
 (0)