Merge pull request #100244 from cockroachdb/blathers/backport-release…

…-23.1-100017 release-23.1: upgrades: update system.web_sessions migration to handle orphaned rows
cockroachdb · Mar 31, 2023 · 7e72aae · 7e72aae
2 parents fba51d2 + 715078b
commit 7e72aae
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 10 deletions.
diff --git a/pkg/upgrade/upgrades/web_sessions_table_user_id_migration.go b/pkg/upgrade/upgrades/web_sessions_table_user_id_migration.go
@@ -53,6 +53,12 @@ WHERE w.user_id IS NULL AND w.username = u.username
 LIMIT 1000
 `
 
+const deleteRowsForDroppedUsersWebSessionsTableStmt = `
+DELETE FROM system.web_sessions
+WHERE user_id IS NULL
+LIMIT 1000
+`
+
 const setUserIDColumnToNotNullWebSessionsTableStmt = `
 ALTER TABLE system.web_sessions
 ALTER COLUMN user_id SET NOT NULL
@@ -62,16 +68,27 @@ func backfillWebSessionsTableUserIDColumn(
 	ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps,
 ) error {
 	ie := d.DB.Executor()
-	for {
-		rowsAffected, err := ie.ExecEx(ctx, "backfill-user-id-col-web-sessions-table", nil, /* txn */
-			sessiondata.NodeUserSessionDataOverride,
-			backfillUserIDColumnWebSessionsTableStmt,
-		)
-		if err != nil {
-			return err
-		}
-		if rowsAffected == 0 {
-			break
+	for _, op := range []struct {
+		name  string
+		query string
+	}{
+		{
+			name:  "backfill-user-id-col-web-sessions-table",
+			query: backfillUserIDColumnWebSessionsTableStmt,
+		},
+		{
+			name:  "delete-rows-for-dropped-users-web-sessions-table",
+			query: deleteRowsForDroppedUsersWebSessionsTableStmt,
+		},
+	} {
+		for {
+			rowsAffected, err := ie.ExecEx(ctx, op.name, nil /* txn */, sessiondata.NodeUserSessionDataOverride, op.query)
+			if err != nil {
+				return err
+			}
+			if rowsAffected == 0 {
+				break
+			}
 		}
 	}
 

diff --git a/pkg/upgrade/upgrades/web_sessions_table_user_id_migration_test.go b/pkg/upgrade/upgrades/web_sessions_table_user_id_migration_test.go
@@ -80,6 +80,12 @@ VALUES (
 	})
 	tdb.CheckQueryResults(t, "SELECT count(*) FROM system.web_sessions", [][]string{{strconv.Itoa(numUsers)}})
 
+	// Drop a user to test that migration deletes orphaned rows.
+	if numUsers > 0 {
+		tdb.Exec(t, "DROP USER testuser0")
+		numUsers -= 1
+	}
+
 	// Run migrations.
 	_, err := tc.Conns[0].ExecContext(ctx, `SET CLUSTER SETTING version = $1`,
 		clusterversion.ByKey(clusterversion.V23_1WebSessionsTableHasUserIDColumn).String())