diff --git a/pkg/server/application_api/sql_stats_test.go b/pkg/server/application_api/sql_stats_test.go index cbf3a895cd6e..1f930e9d4c70 100644 --- a/pkg/server/application_api/sql_stats_test.go +++ b/pkg/server/application_api/sql_stats_test.go @@ -13,6 +13,7 @@ package application_api_test import ( "context" gosql "database/sql" + "encoding/json" "fmt" "net/url" "reflect" @@ -48,8 +49,11 @@ func TestStatusAPICombinedTransactions(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - // Skip under stress until we extend the timeout for the http client. - skip.UnderStressWithIssue(t, 109184) + // Increase the timeout for the http client if under stress. + additionalTimeout := 0 * time.Second + if skip.Stress() { + additionalTimeout = 20 * time.Second + } var params base.TestServerArgs params.Knobs.SpanConfig = &spanconfig.TestingKnobs{ManagerDisableJobCreation: true} // TODO(irfansharif): #74919. @@ -128,7 +132,7 @@ func TestStatusAPICombinedTransactions(t *testing.T) { // Hit query endpoint. var resp serverpb.StatementsResponse - if err := srvtestutils.GetStatusJSONProto(firstServerProto, "combinedstmts", &resp); err != nil { + if err := srvtestutils.GetStatusJSONProtoWithAdminAndTimeoutOption(firstServerProto, "combinedstmts", &resp, true, additionalTimeout); err != nil { t.Fatal(err) } @@ -190,6 +194,12 @@ func TestStatusAPITransactions(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) + // Increase the timeout for the http client if under stress. + additionalTimeout := 0 * time.Second + if skip.Stress() { + additionalTimeout = 20 * time.Second + } + testCluster := serverutils.StartCluster(t, 3, base.TestClusterArgs{}) ctx := context.Background() defer testCluster.Stopper().Stop(ctx) @@ -263,7 +273,7 @@ func TestStatusAPITransactions(t *testing.T) { // Hit query endpoint. var resp serverpb.StatementsResponse - if err := srvtestutils.GetStatusJSONProto(firstServerProto, "statements", &resp); err != nil { + if err := srvtestutils.GetStatusJSONProtoWithAdminAndTimeoutOption(firstServerProto, "statements", &resp, true, additionalTimeout); err != nil { t.Fatal(err) } @@ -383,8 +393,11 @@ func TestStatusAPIStatements(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - // Skip under stress until we extend the timeout for the http client. - skip.UnderStressWithIssue(t, 109184) + // Increase the timeout for the http client if under stress. + additionalTimeout := 0 * time.Second + if skip.Stress() { + additionalTimeout = 20 * time.Second + } // Aug 30 2021 19:50:00 GMT+0000 aggregatedTs := int64(1630353000) @@ -432,7 +445,7 @@ func TestStatusAPIStatements(t *testing.T) { testPath := func(path string, expectedStmts []string) { // Hit query endpoint. - if err := srvtestutils.GetStatusJSONProtoWithAdminOption(firstServerProto, path, &resp, false); err != nil { + if err := srvtestutils.GetStatusJSONProtoWithAdminAndTimeoutOption(firstServerProto, path, &resp, false, additionalTimeout); err != nil { t.Fatal(err) } @@ -497,8 +510,11 @@ func TestStatusAPICombinedStatementsWithFullScans(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - // Skip under stress until we extend the timeout for the http client. - skip.UnderStressWithIssue(t, 109184) + // Increase the timeout for the http client if under stress. + additionalTimeout := 0 * time.Second + if skip.Stress() { + additionalTimeout = 20 * time.Second + } // Aug 30 2021 19:50:00 GMT+0000 aggregatedTs := int64(1630353000) @@ -591,7 +607,7 @@ func TestStatusAPICombinedStatementsWithFullScans(t *testing.T) { } verifyCombinedStmtStats := func() { - err := srvtestutils.GetStatusJSONProtoWithAdminOption(firstServerProto, endpoint, &resp, false) + err := srvtestutils.GetStatusJSONProtoWithAdminAndTimeoutOption(firstServerProto, endpoint, &resp, false, additionalTimeout) require.NoError(t, err) for _, respStatement := range resp.Statements { @@ -607,10 +623,16 @@ func TestStatusAPICombinedStatementsWithFullScans(t *testing.T) { continue } - require.Equal(t, expectedData.fullScan, actualFullScan) - require.Equal(t, expectedData.distSQL, actualDistSQL) - require.Equal(t, expectedData.failed, actualFailed) - require.Equal(t, expectedData.count, int(actualCount)) + bytes, err := json.Marshal(respStatement) + if err != nil { + t.Fatal(err) + } + respString := (string(bytes)) + + require.Equal(t, expectedData.fullScan, actualFullScan, "failed for respStatement: %v", respString) + require.Equal(t, expectedData.distSQL, actualDistSQL, "failed for respStatement: %v", respString) + require.Equal(t, expectedData.failed, actualFailed, "failed for respStatement: %v", respString) + require.Equal(t, expectedData.count, int(actualCount), "failed for respStatement: %v", respString) } } @@ -822,8 +844,11 @@ func TestStatusAPICombinedStatements(t *testing.T) { func TestStatusAPIStatementDetails(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - // The liveness session might expire before the stress race can finish. - skip.UnderStressRace(t, "expensive tests") + // Increase the timeout for the http client if under stress. + additionalTimeout := 0 * time.Second + if skip.Stress() { + additionalTimeout = 20 * time.Second + } // Aug 30 2021 19:50:00 GMT+0000 aggregatedTs := int64(1630353000) @@ -882,7 +907,7 @@ func TestStatusAPIStatementDetails(t *testing.T) { } testPath := func(path string, expected resultValues) { - err := srvtestutils.GetStatusJSONProtoWithAdminOption(firstServerProto, path, &resp, false) + err := srvtestutils.GetStatusJSONProtoWithAdminAndTimeoutOption(firstServerProto, path, &resp, false, additionalTimeout) require.NoError(t, err) require.Equal(t, int64(expected.totalCount), resp.Statement.Stats.Count) require.Equal(t, expected.aggregatedTsCount, len(resp.StatementStatisticsPerAggregatedTs)) diff --git a/pkg/server/srvtestutils/testutils.go b/pkg/server/srvtestutils/testutils.go index 5beab3e79a39..5801088e9579 100644 --- a/pkg/server/srvtestutils/testutils.go +++ b/pkg/server/srvtestutils/testutils.go @@ -13,6 +13,7 @@ package srvtestutils import ( "encoding/json" "io" + "time" "github.com/cockroachdb/cockroach/pkg/server/apiconstants" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -85,6 +86,18 @@ func GetStatusJSONProtoWithAdminOption( return serverutils.GetJSONProtoWithAdminOption(ts, apiconstants.StatusPrefix+path, response, isAdmin) } +// GetStatusJSONProtoWithAdminAndTimeoutOption is similar to GetStatusJSONProtoWithAdminOption, but +// the caller can specify an additional timeout duration for the request. +func GetStatusJSONProtoWithAdminAndTimeoutOption( + ts serverutils.ApplicationLayerInterface, + path string, + response protoutil.Message, + isAdmin bool, + additionalTimeout time.Duration, +) error { + return serverutils.GetJSONProtoWithAdminAndTimeoutOption(ts, apiconstants.StatusPrefix+path, response, isAdmin, additionalTimeout) +} + // PostStatusJSONProtoWithAdminOption performs a RPC-over-HTTP request to // the status endpoint and unmarshals the response into the specified // proto message. It allows the caller to control whether the request diff --git a/pkg/testutils/serverutils/test_server_shim.go b/pkg/testutils/serverutils/test_server_shim.go index 6ab76fbc7653..10c8f9b0c96e 100644 --- a/pkg/testutils/serverutils/test_server_shim.go +++ b/pkg/testutils/serverutils/test_server_shim.go @@ -23,6 +23,7 @@ import ( "net/url" "strconv" "strings" + "time" "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/kv" @@ -332,6 +333,28 @@ func GetJSONProtoWithAdminOption( return httputil.GetJSON(httpClient, fullURL, response) } +// GetJSONProtoWithAdminAndTimeoutOption is like GetJSONProtoWithAdminOption but +// the caller can specify an additional timeout duration for the request. The +// default is 10s. +func GetJSONProtoWithAdminAndTimeoutOption( + ts ApplicationLayerInterface, + path string, + response protoutil.Message, + isAdmin bool, + additionalTimeout time.Duration, +) error { + httpClient, err := ts.GetAuthenticatedHTTPClient(isAdmin, SingleTenantSession) + if err != nil { + return err + } + httpClient.Timeout += additionalTimeout + u := ts.AdminURL().String() + fullURL := u + path + log.Infof(context.Background(), "test retrieving protobuf over HTTP: %s", fullURL) + log.Infof(context.Background(), "set HTTP client timeout to: %s", httpClient.Timeout) + return httputil.GetJSON(httpClient, fullURL, response) +} + // PostJSONProto uses the supplied client to POST the URL specified by the parameters // and unmarshals the result into response. func PostJSONProto(