MM-31396: Fix a possible deadlock with SidebarCategories (#17109)

This is a deadlock due to reversed locking order of the SidebarChannels
and SidebarCategories table.

I could not find the exact culprit query from the deadlock output
because it only shows the last query a transaction is running. And from looking
at the code, the only query that runs "UPDATE SidebarCategories SET DisplayName = ?, Sorting = ? WHERE Id = ?"
is UpdateSidebarCategories. But for the deadlock to happen, it has to lock
SidebarChannels _first_, and then _then_ lock SidebarCategories.

Looking a bit more throughly, I found that DeleteSidebarCategory does indeed
lock the tables in an inverse way and if DeleteSidebarCategory runs concurrently with
UpdateSidebarCategories, they will deadlock.

Here's how it will happen.

```
tx1
DELETE FROM SidebarChannels WHERE CategoryId = 'xx';

tx2
UPDATE SidebarCategories SET DisplayName='dn' WHERE Id='xx';

tx2
DELETE FROM SidebarChannels WHERE (ChannelId IN ('yy') AND CategoryId = 'xx');

tx1
DELETE FROM SidebarCategories WHERE Id = 'xx';
```

And then we see:
ERROR 1213 (40001): Deadlock found when trying to get lock; try restarting transaction

To fix this, we simply reorder the Delete query to lock the SidebarCategories first,
and then SidebarChannels.

In fact, any transaction updating/deleting rows from those two tables should always operate on that order
if possible.

https://mattermost.atlassian.net/browse/MM-31396

```release-note
Fixed a database deadlock that can happen if a sidebar category is updated and deleted at the same time.
```

store/sqlstore/channel_store_categories.go

@@ -640,6 +640,11 @@ func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categori
 			updatedCategory.Muted = category.Muted
 		}
 
+		// The order in which the queries are executed in the transaction is important.
+		// SidebarCategories need to be update first, and then SidebarChannels should be deleted.
+		// The net effect remains the same, but it prevents deadlocks from other transactions
+		// operating on the tables in reverse order.
+
 		updateQuery, updateParams, _ := s.getQueryBuilder().
 			Update("SidebarCategories").
 			Set("DisplayName", updatedCategory.DisplayName).
@@ -1002,30 +1007,33 @@ func (s SqlChannelStore) DeleteSidebarCategory(categoryId string) error {
 		return store.NewErrInvalidInput("SidebarCategory", "id", categoryId)
 	}
 
-	// Delete the channels in the category
-	query, args, err := s.getQueryBuilder().
-		Delete("SidebarChannels").
-		Where(sq.Eq{"CategoryId": categoryId}).ToSql()
-	if err != nil {
-		return errors.Wrap(err, "delete_sidebar_cateory_tosql")
-	}
-
-	if _, err = transaction.Exec(query, args...); err != nil {
-		return errors.Wrap(err, "failed to delete SidebarChannel")
-	}
+	// The order in which the queries are executed in the transaction is important.
+	// SidebarCategories need to be deleted first, and then SidebarChannels.
+	// The net effect remains the same, but it prevents deadlocks from other transactions
+	// operating on the tables in reverse order.
 
 	// Delete the category itself
-	query, args, err = s.getQueryBuilder().
+	query, args, err := s.getQueryBuilder().
 		Delete("SidebarCategories").
 		Where(sq.Eq{"Id": categoryId}).ToSql()
 	if err != nil {
 		return errors.Wrap(err, "delete_sidebar_cateory_tosql")
 	}
-
 	if _, err = transaction.Exec(query, args...); err != nil {
 		return errors.Wrap(err, "failed to delete SidebarCategory")
 	}
 
+	// Delete the channels in the category
+	query, args, err = s.getQueryBuilder().
+		Delete("SidebarChannels").
+		Where(sq.Eq{"CategoryId": categoryId}).ToSql()
+	if err != nil {
+		return errors.Wrap(err, "delete_sidebar_cateory_tosql")
+	}
+	if _, err = transaction.Exec(query, args...); err != nil {
+		return errors.Wrap(err, "failed to delete SidebarChannel")
+	}
+
 	if err := transaction.Commit(); err != nil {
 		return errors.Wrap(err, "commit_transaction")
 	}

store/storetest/channel_store_categories.go

@@ -23,9 +23,9 @@ func TestChannelStoreCategories(t *testing.T, ss store.Store, s SqlStore) {
 	t.Run("GetSidebarCategories", func(t *testing.T) { testGetSidebarCategories(t, ss) })
 	t.Run("UpdateSidebarCategories", func(t *testing.T) { testUpdateSidebarCategories(t, ss) })
 	t.Run("ClearSidebarOnTeamLeave", func(t *testing.T) { testClearSidebarOnTeamLeave(t, ss, s) })
-	t.Run("UpdateSidebarCategories", func(t *testing.T) { testUpdateSidebarCategories(t, ss) })
 	t.Run("DeleteSidebarCategory", func(t *testing.T) { testDeleteSidebarCategory(t, ss, s) })
 	t.Run("UpdateSidebarChannelsByPreferences", func(t *testing.T) { testUpdateSidebarChannelsByPreferences(t, ss) })
+	t.Run("SidebarCategoryDeadlock", func(t *testing.T) { testSidebarCategoryDeadlock(t, ss) })
 }
 
 func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) {
@@ -2041,3 +2041,68 @@ func testUpdateSidebarChannelsByPreferences(t *testing.T, ss store.Store) {
 		})
 	})
 }
+
+// testSidebarCategoryDeadlock tries to delete and update a category at the same time
+// in the hope of triggering a deadlock. This is a best-effort test case, and is not guaranteed
+// to catch a bug.
+func testSidebarCategoryDeadlock(t *testing.T, ss store.Store) {
+	userID := model.NewId()
+	teamID := model.NewId()
+
+	// Join a channel
+	channel, err := ss.Channel().Save(&model.Channel{
+		Name:   "channel",
+		Type:   model.CHANNEL_OPEN,
+		TeamId: teamID,
+	}, 10)
+	require.NoError(t, err)
+	_, err = ss.Channel().SaveMember(&model.ChannelMember{
+		UserId:      userID,
+		ChannelId:   channel.Id,
+		NotifyProps: model.GetDefaultChannelNotifyProps(),
+	})
+	require.NoError(t, err)
+
+	// And then create the initial categories so that it includes the channel
+	res, err := ss.Channel().CreateInitialSidebarCategories(userID, teamID)
+	require.NoError(t, err)
+	require.NotEmpty(t, res)
+
+	initialCategories, err := ss.Channel().GetSidebarCategories(userID, teamID)
+	require.NoError(t, err)
+
+	channelsCategory := initialCategories.Categories[1]
+	require.Equal(t, []string{channel.Id}, channelsCategory.Channels)
+
+	customCategory, err := ss.Channel().CreateSidebarCategory(userID, teamID, &model.SidebarCategoryWithChannels{})
+	require.NoError(t, err)
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	go func() {
+		defer wg.Done()
+		_, _, err := ss.Channel().UpdateSidebarCategories(userID, teamID, []*model.SidebarCategoryWithChannels{
+			{
+				SidebarCategory: channelsCategory.SidebarCategory,
+				Channels:        []string{},
+			},
+			{
+				SidebarCategory: customCategory.SidebarCategory,
+				Channels:        []string{channel.Id},
+			},
+		})
+		if err != nil {
+			var nfErr *store.ErrNotFound
+			require.True(t, errors.As(err, &nfErr))
+		}
+	}()
+
+	go func() {
+		defer wg.Done()
+		err := ss.Channel().DeleteSidebarCategory(customCategory.Id)
+		require.NoError(t, err)
+	}()
+
+	wg.Wait()
+}