pkg/ccl/serverccl/statusccl/statusccl_test: TestTenantStatusAPI failed

[cockroach-teamcity]

pkg/ccl/serverccl/statusccl/statusccl_test.TestTenantStatusAPI failed with artifacts on master @ e924a27896b9fbf495609d3965612366de6ebf78:

=== RUN   TestTenantStatusAPI
    test_log_scope.go:170: test logs captured to: /artifacts/tmp/_tmp/7067ad80d000d85ce2eeee021e911c58/logTestTenantStatusAPI1737636433
    tenant_status_test.go:144: -- test log scope end --
CREATE UNIQUE INDEX test_pkey ON test_db1.public.test USING btree (k ASC)test logs left over in: /artifacts/tmp/_tmp/7067ad80d000d85ce2eeee021e911c58/logTestTenantStatusAPI1737636433
--- FAIL: TestTenantStatusAPI (17.28s)
=== RUN   TestTenantStatusAPI/tenant_cancel_session
    tenant_status_test.go:1095: error executing 'SELECT session_id FROM crdb_internal.cluster_sessions WHERE status = 'ACTIVE' OR status = 'IDLE'': read tcp 127.0.0.1:42566->127.0.0.1:35893: read: connection reset by peer
    tenant_status_test.go:1136: 
        	Error Trace:	pkg/ccl/serverccl/statusccl/statusccl_test/pkg/ccl/serverccl/statusccl/tenant_status_test.go:1136
        	            				pkg/ccl/serverccl/statusccl/statusccl_test/pkg/ccl/serverccl/statusccl/tenant_status_test.go:97
        	Error:      	Condition never satisfied
        	Test:       	TestTenantStatusAPI/tenant_cancel_session
    --- FAIL: TestTenantStatusAPI/tenant_cancel_session (5.11s)

Help

See also: How To Investigate a Go Test Failure (internal)

/cc @cockroachdb/obs-prs

This test on roachdash | Improve this report!

Jira issue: CRDB-39439

[cockroach-teamcity]

pkg/ccl/serverccl/statusccl/statusccl_test.TestTenantStatusAPI failed with artifacts on master @ ba7de435128f4d538168c61d749e5a4d2f1496dd:

=== RUN   TestTenantStatusAPI
    test_log_scope.go:170: test logs captured to: /artifacts/tmp/_tmp/7067ad80d000d85ce2eeee021e911c58/logTestTenantStatusAPI3715395111
    tenant_status_test.go:144: -- test log scope end --
CREATE UNIQUE INDEX test_pkey ON test_db1.public.test USING btree (k ASC)test logs left over in: /artifacts/tmp/_tmp/7067ad80d000d85ce2eeee021e911c58/logTestTenantStatusAPI3715395111
--- FAIL: TestTenantStatusAPI (18.19s)
=== RUN   TestTenantStatusAPI/tenant_cancel_session
    tenant_status_test.go:1095: error executing 'SELECT session_id FROM crdb_internal.cluster_sessions WHERE status = 'ACTIVE' OR status = 'IDLE'': read tcp 127.0.0.1:56308->127.0.0.1:39391: read: connection reset by peer
    tenant_status_test.go:1136: 
        	Error Trace:	pkg/ccl/serverccl/statusccl/statusccl_test/pkg/ccl/serverccl/statusccl/tenant_status_test.go:1136
        	            				pkg/ccl/serverccl/statusccl/statusccl_test/pkg/ccl/serverccl/statusccl/tenant_status_test.go:97
        	Error:      	Condition never satisfied
        	Test:       	TestTenantStatusAPI/tenant_cancel_session
    --- FAIL: TestTenantStatusAPI/tenant_cancel_session (5.11s)

Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

[dhartunian]

I've bisected this test failure to #124373

@rafiss is it possible that given your change we can't guarantee that in 5 seconds the session will be closed in our test?

Code in question is here, and it's the require.Eventually on line 1136 that never succeds which is checking crdb_internal.cluster_sessions for the session we've cancelled in the API to get removed. When run under --stress sometimes the session doesn't get removed after 5 seconds. Bumping to 10 seconds still doesn't do it so I think some sort of signal is missing.

cockroach/pkg/ccl/serverccl/statusccl/tenant_status_test.go

Lines 1093 to 1144 in 3962ffc

	func selectClusterSessionIDs(t *testing.T, conn *sqlutils.SQLRunner) []string {
	var sessionIDs []string
	rows := conn.QueryStr(t,
	"SELECT session_id FROM crdb_internal.cluster_sessions WHERE status = 'ACTIVE' OR status = 'IDLE'")
	for _, row := range rows {
	sessionIDs = append(sessionIDs, row[0])
	}
	return sessionIDs
	}
	func testTenantStatusCancelSession(t *testing.T, helper serverccl.TenantTestHelper) {
	// Open a SQL session on tenant SQL pod 0.
	sqlPod0 := helper.TestCluster().TenantConn(0)
	sqlPod0.Exec(t, "SELECT 1")
	// See the session over HTTP on tenant SQL pod 1.
	httpPod1 := helper.TestCluster().TenantAdminHTTPClient(t, 1)
	defer httpPod1.Close()
	listSessionsResp := serverpb.ListSessionsResponse{}
	httpPod1.GetJSON("/_status/sessions?exclude_closed_sessions=true", &listSessionsResp)
	var session serverpb.Session
	for _, s := range listSessionsResp.Sessions {
	if s.LastActiveQuery == "SELECT 1" {
	session = s
	break
	}
	}
	require.NotNil(t, session.ID, "session not found")
	// See the session over SQL on tenant SQL pod 0.
	sessionID := hex.EncodeToString(session.ID)
	require.Eventually(t, func() bool {
	return strings.Contains(strings.Join(selectClusterSessionIDs(t, sqlPod0), ","), sessionID)
	}, 5*time.Second, 100*time.Millisecond)
	// Cancel the session over HTTP from tenant SQL pod 1.
	cancelSessionReq := serverpb.CancelSessionRequest{SessionID: session.ID}
	cancelSessionResp := serverpb.CancelSessionResponse{}
	httpPod1.PostJSON("/_status/cancel_session/"+session.NodeID.String(), &cancelSessionReq, &cancelSessionResp)
	require.Equal(t, true, cancelSessionResp.Canceled, cancelSessionResp.Error)
	// No longer see the session over SQL from tenant SQL pod 0.
	// (The SQL client maintains an internal connection pool and automatically reconnects.)
	require.Eventually(t, func() bool {
	return !strings.Contains(strings.Join(selectClusterSessionIDs(t, sqlPod0), ","), sessionID)
	}, 5*time.Second, 100*time.Millisecond)
	// Attempt to cancel the session again over HTTP from tenant SQL pod 1, so that we can see the error message.
	httpPod1.PostJSON("/_status/cancel_session/"+session.NodeID.String(), &cancelSessionReq, &cancelSessionResp)
	require.Equal(t, false, cancelSessionResp.Canceled)
	require.Equal(t, fmt.Sprintf("session ID %s not found", sessionID), cancelSessionResp.Error)
	}

[rafiss]

It looks like the failure is read: connection reset by peer

    tenant_status_test.go:1095: error executing 'SELECT session_id FROM crdb_internal.cluster_sessions WHERE status = 'ACTIVE' OR status = 'IDLE'': read tcp 127.0.0.1:56308->127.0.0.1:39391: read: connection reset by peer

So I don't think the issue is that the session is never cancelled. Rather, the issue appears to be that the test is trying to use a connection that was closed by the server. It appears to be a test bug -- the test is using the same SQL session to run this query as the SQL session that was cancelled. As such, I'll remove the blocker label. Prior to #124373, the test was probably getting away with this, since it took longer for the session to react to the cancel signal, which might affect how the connection pool automatically reconnects.

Here's an example of how it could be fixed. I've used overly verbose variable names for clarity.

diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go
index 2b8f857e774..6db66436a63 100644
--- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go
+++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go
@@ -1101,9 +1101,16 @@ func selectClusterSessionIDs(t *testing.T, conn *sqlutils.SQLRunner) []string {
 }
 
 func testTenantStatusCancelSession(t *testing.T, helper serverccl.TenantTestHelper) {
-	// Open a SQL session on tenant SQL pod 0.
-	sqlPod0 := helper.TestCluster().TenantConn(0)
-	sqlPod0.Exec(t, "SELECT 1")
+	ctx := context.TODO()
+	// Open two different SQL sessions on tenant SQL pod 0.
+	sqlPod0 := helper.TestCluster().TenantDB(0)
+	sqlPod0SessionToCancel, err := sqlPod0.Conn(ctx)
+	require.NoError(t, err)
+	sqlPod0SessionForIntrospection, err := sqlPod0.Conn(ctx)
+	require.NoError(t, err)
+	_, err = sqlPod0SessionToCancel.ExecContext(ctx, "SELECT 1")
+	require.NoError(t, err)
+	introspectionRunner := sqlutils.MakeSQLRunner(sqlPod0SessionForIntrospection)
 
 	// See the session over HTTP on tenant SQL pod 1.
 	httpPod1 := helper.TestCluster().TenantAdminHTTPClient(t, 1)
@@ -1122,7 +1129,7 @@ func testTenantStatusCancelSession(t *testing.T, helper serverccl.TenantTestHelp
 	// See the session over SQL on tenant SQL pod 0.
 	sessionID := hex.EncodeToString(session.ID)
 	require.Eventually(t, func() bool {
-		return strings.Contains(strings.Join(selectClusterSessionIDs(t, sqlPod0), ","), sessionID)
+		return strings.Contains(strings.Join(selectClusterSessionIDs(t, introspectionRunner), ","), sessionID)
 	}, 5*time.Second, 100*time.Millisecond)
 
 	// Cancel the session over HTTP from tenant SQL pod 1.
@@ -1134,7 +1141,7 @@ func testTenantStatusCancelSession(t *testing.T, helper serverccl.TenantTestHelp
 	// No longer see the session over SQL from tenant SQL pod 0.
 	// (The SQL client maintains an internal connection pool and automatically reconnects.)
 	require.Eventually(t, func() bool {
-		return !strings.Contains(strings.Join(selectClusterSessionIDs(t, sqlPod0), ","), sessionID)
+		return !strings.Contains(strings.Join(selectClusterSessionIDs(t, introspectionRunner), ","), sessionID)
 	}, 5*time.Second, 100*time.Millisecond)
 
 	// Attempt to cancel the session again over HTTP from tenant SQL pod 1, so that we can see the error message.