mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

[AI assisted] MM-64298: Process setting status offline in batches (#31065)

Просмотр исходного кода 
When a user disconnects from the hub, we would spawn off a goroutine
which would make a cluster request, and then update the user status
as offline in the DB.

This was another case of unbounded concurrency where the number of
goroutines spawned was user controlled. Therefore, we would see
a clear spike in DB connections on master when a lot of users
would suddenly disconnect.

To fix this, we implement concurrency control in two areas:
1. In making the cluster request. We implement a counting semaphore
per-hub to avoid making unbounded cluster requests.
2. We use a buffered channel with a periodic flusher to process
status updates.

We also add a new store method to upsert multiple statuses
in a single query. The statusUpdateThreshold is set to 32, which means
no more than 32 rows will be upserted at one time, keeping the
SQL query load reasonable.

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

```release-note
We improve DB connection spikes on user disconnect
by processing status updates in batches.
```
Этот коммит содержится в:
Agniva De Sarker
2025-06-17 09:20:34 +05:30
коммит произвёл GitHub
родитель b99a22f175
Коммит 761bc7549b
11 изменённых файлов: 415 добавлений и 24 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

21
server/channels/store/retrylayer/retrylayer.go
Просмотреть файл

@@ -12230,6 +12230,27 @@ func (s *RetryLayerStatusStore) SaveOrUpdate(status *model.Status) error {
}
func (s *RetryLayerStatusStore) SaveOrUpdateMany(statuses map[string]*model.Status) error {
tries := 0
for {
err := s.StatusStore.SaveOrUpdateMany(statuses)
if err == nil {
return nil
}
if !isRepeatableError(err) {
return err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return err
}
timepkg.Sleep(100 * timepkg.Millisecond)
}
}
func (s *RetryLayerStatusStore) UpdateExpiredDNDStatuses() ([]*model.Status, error) {
tries := 0

41
server/channels/store/sqlstore/status_store.go
Просмотреть файл

@@ -48,11 +48,9 @@ func (s SqlStatusStore) SaveOrUpdate(st *model.Status) error {
Values(st.UserId, st.Status, st.Manual, st.LastActivityAt, st.DNDEndTime, st.PrevStatus)
if s.DriverName() == model.DatabaseDriverMysql {
query = query.SuffixExpr(sq.Expr("ON DUPLICATE KEY UPDATE Status = ?, `Manual` = ?, LastActivityAt = ?, DNDEndTime = ?, PrevStatus = ?",
st.Status, st.Manual, st.LastActivityAt, st.DNDEndTime, st.PrevStatus))
query = query.SuffixExpr(sq.Expr("ON DUPLICATE KEY UPDATE Status = VALUES(Status), `Manual` = VALUES(`Manual`), LastActivityAt = VALUES(LastActivityAt), DNDEndTime = VALUES(DNDEndTime), PrevStatus = VALUES(PrevStatus)"))
} else {
query = query.SuffixExpr(sq.Expr("ON CONFLICT (userid) DO UPDATE SET Status = ?, Manual = ?, LastActivityAt = ?, DNDEndTime = ?, PrevStatus = ?",
st.Status, st.Manual, st.LastActivityAt, st.DNDEndTime, st.PrevStatus))
query = query.SuffixExpr(sq.Expr("ON CONFLICT (userid) DO UPDATE SET Status = EXCLUDED.Status, Manual = EXCLUDED.Manual, LastActivityAt = EXCLUDED.LastActivityAt, DNDEndTime = EXCLUDED.DNDEndTime, PrevStatus = EXCLUDED.PrevStatus"))
}
if _, err := s.GetMaster().ExecBuilder(query); err != nil {
@@ -62,6 +60,41 @@ func (s SqlStatusStore) SaveOrUpdate(st *model.Status) error {
return nil
}
func (s SqlStatusStore) SaveOrUpdateMany(statuses map[string]*model.Status) error {
if len(statuses) == 0 {
return nil
}
// If there's only one status, use the existing method
if len(statuses) == 1 {
for _, st := range statuses {
return s.SaveOrUpdate(st)
}
}
query := s.getQueryBuilder().
Insert("Status").
Columns("UserId", "Status", quoteColumnName(s.DriverName(), "Manual"), "LastActivityAt", "DNDEndTime", "PrevStatus")
// Add values for each unique status
for _, st := range statuses {
query = query.Values(st.UserId, st.Status, st.Manual, st.LastActivityAt, st.DNDEndTime, st.PrevStatus)
}
// Handle different databases
if s.DriverName() == model.DatabaseDriverMysql {
query = query.SuffixExpr(sq.Expr("ON DUPLICATE KEY UPDATE Status = VALUES(Status), `Manual` = VALUES(`Manual`), LastActivityAt = VALUES(LastActivityAt), DNDEndTime = VALUES(DNDEndTime), PrevStatus = VALUES(PrevStatus)"))
} else {
query = query.SuffixExpr(sq.Expr("ON CONFLICT (userid) DO UPDATE SET Status = EXCLUDED.Status, Manual = EXCLUDED.Manual, LastActivityAt = EXCLUDED.LastActivityAt, DNDEndTime = EXCLUDED.DNDEndTime, PrevStatus = EXCLUDED.PrevStatus"))
}
if _, err := s.GetMaster().ExecBuilder(query); err != nil {
return errors.Wrap(err, "failed to upsert multiple Status records")
}
return nil
}
func (s SqlStatusStore) Get(userId string) (*model.Status, error) {
query := s.statusSelectQuery.Where(sq.Eq{"UserId": userId})

1
server/channels/store/store.go
Просмотреть файл

@@ -717,6 +717,7 @@ type EmojiStore interface {
type StatusStore interface {
SaveOrUpdate(status *model.Status) error
SaveOrUpdateMany(statuses map[string]*model.Status) error
Get(userID string) (*model.Status, error)
GetByIds(userIds []string) ([]*model.Status, error)
ResetAll() error

18
server/channels/store/storetest/mocks/StatusStore.go
Просмотреть файл

@@ -138,6 +138,24 @@ func (_m *StatusStore) SaveOrUpdate(status *model.Status) error {
return r0
}
// SaveOrUpdateMany provides a mock function with given fields: statuses
func (_m *StatusStore) SaveOrUpdateMany(statuses map[string]*model.Status) error {
ret := _m.Called(statuses)
if len(ret) == 0 {
panic("no return value specified for SaveOrUpdateMany")
}
var r0 error
if rf, ok := ret.Get(0).(func(map[string]*model.Status) error); ok {
r0 = rf(statuses)
} else {
r0 = ret.Error(0)
}
return r0
}
// UpdateExpiredDNDStatuses provides a mock function with no fields
func (_m *StatusStore) UpdateExpiredDNDStatuses() ([]*model.Status, error) {
ret := _m.Called()

82
server/channels/store/storetest/status_store.go
Просмотреть файл

@@ -17,19 +17,75 @@ import (
)
func TestStatusStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {
t.Run("", func(t *testing.T) { testStatusStore(t, rctx, ss) })
t.Run("Basic", func(t *testing.T) { testStatusStore(t, rctx, ss) })
t.Run("ActiveUserCount", func(t *testing.T) { testActiveUserCount(t, rctx, ss) })
t.Run("UpdateExpiredDNDStatuses", func(t *testing.T) { testUpdateExpiredDNDStatuses(t, rctx, ss) })
t.Run("Get", func(t *testing.T) { testStatusGet(t, rctx, ss, s) })
t.Run("GetByIds", func(t *testing.T) { testStatusGetByIds(t, rctx, ss, s) })
t.Run("SaveOrUpdateMany", func(t *testing.T) { testSaveOrUpdateMany(t, rctx, ss) })
}
func testSaveOrUpdateMany(t *testing.T, _ request.CTX, ss store.Store) {
// Test with empty map
err := ss.Status().SaveOrUpdateMany(map[string]*model.Status{})
require.NoError(t, err, "SaveOrUpdateMany with empty map should succeed")
// Test with single status
status1 := &model.Status{UserId: model.NewId(), Status: model.StatusOnline, Manual: false, LastActivityAt: 10, ActiveChannel: ""}
err = ss.Status().SaveOrUpdateMany(map[string]*model.Status{
status1.UserId: status1,
})
require.NoError(t, err, "SaveOrUpdateMany with single status should succeed")
// Verify the status was saved
retrieved, err := ss.Status().Get(status1.UserId)
require.NoError(t, err)
assert.Equal(t, status1.UserId, retrieved.UserId)
assert.Equal(t, status1.Status, retrieved.Status)
// Test with multiple statuses
status2 := &model.Status{UserId: model.NewId(), Status: model.StatusAway, Manual: true, LastActivityAt: 20, ActiveChannel: ""}
status3 := &model.Status{UserId: model.NewId(), Status: model.StatusDnd, Manual: true, LastActivityAt: 30, ActiveChannel: ""}
err = ss.Status().SaveOrUpdateMany(map[string]*model.Status{
status2.UserId: status2,
status3.UserId: status3,
})
require.NoError(t, err, "SaveOrUpdateMany with multiple statuses should succeed")
// Verify all statuses were saved
statuses, err := ss.Status().GetByIds([]string{status2.UserId, status3.UserId})
require.NoError(t, err)
require.Len(t, statuses, 2, "should have retrieved both statuses")
// Verify the retrieved statuses are actually the ones from status2 and status3
statusMap := make(map[string]*model.Status)
for _, status := range statuses {
statusMap[status.UserId] = status
}
assert.Equal(t, status2.Status, statusMap[status2.UserId].Status)
assert.Equal(t, status3.Status, statusMap[status3.UserId].Status)
// Test with duplicate userIds (last one should win)
status4 := &model.Status{UserId: status1.UserId, Status: model.StatusOffline, Manual: true, LastActivityAt: 40, ActiveChannel: ""}
status5 := &model.Status{UserId: status1.UserId, Status: model.StatusDnd, Manual: false, LastActivityAt: 50, ActiveChannel: ""}
err = ss.Status().SaveOrUpdateMany(map[string]*model.Status{
status4.UserId: status4,
status5.UserId: status5,
})
require.NoError(t, err, "SaveOrUpdateMany with duplicate userIds should succeed")
// Verify the last status was saved
retrieved, err = ss.Status().Get(status1.UserId)
require.NoError(t, err)
assert.Equal(t, status1.UserId, retrieved.UserId)
assert.Equal(t, status5.Status, retrieved.Status)
assert.Equal(t, int64(50), retrieved.LastActivityAt)
}
func testStatusStore(t *testing.T, _ request.CTX, ss store.Store) {
status := &model.Status{UserId: model.NewId(), Status: model.StatusOnline, Manual: false, LastActivityAt: 0, ActiveChannel: ""}
require.NoError(t, ss.Status().SaveOrUpdate(status))
status.LastActivityAt = 10
_, err := ss.Status().Get(status.UserId)
require.NoError(t, err)
@@ -42,13 +98,31 @@ func testStatusStore(t *testing.T, _ request.CTX, ss store.Store) {
statuses, err := ss.Status().GetByIds([]string{status.UserId, "junk"})
require.NoError(t, err)
require.Len(t, statuses, 1, "should only have 1 status")
assert.Equal(t, status, statuses[0])
// Test updating an existing status
updatedStatus := &model.Status{
UserId: status.UserId,
Status: model.StatusDnd,
Manual: false,
LastActivityAt: 1234,
DNDEndTime: 5678,
PrevStatus: model.StatusOnline,
ActiveChannel: "", // This field won't be stored, so set it to match what we'll get back
}
require.NoError(t, ss.Status().SaveOrUpdate(updatedStatus))
// Verify status was updated
retrievedStatus, err := ss.Status().Get(status.UserId)
require.NoError(t, err)
assert.Equal(t, updatedStatus, retrievedStatus)
err = ss.Status().ResetAll()
require.NoError(t, err)
statusParameter, err := ss.Status().Get(status.UserId)
require.NoError(t, err)
require.Equal(t, statusParameter.Status, model.StatusOffline, "should be offline")
require.Equal(t, model.StatusOffline, statusParameter.Status, "should be offline")
err = ss.Status().UpdateLastActivityAt(status.UserId, 10)
require.NoError(t, err)

16
server/channels/store/timerlayer/timerlayer.go
Просмотреть файл

@@ -9621,6 +9621,22 @@ func (s *TimerLayerStatusStore) SaveOrUpdate(status *model.Status) error {
return err
}
func (s *TimerLayerStatusStore) SaveOrUpdateMany(statuses map[string]*model.Status) error {
start := time.Now()
err := s.StatusStore.SaveOrUpdateMany(statuses)
elapsed := float64(time.Since(start)) / float64(time.Second)
if s.Root.Metrics != nil {
success := "false"
if err == nil {
success = "true"
}
s.Root.Metrics.ObserveStoreMethodDuration("StatusStore.SaveOrUpdateMany", success, elapsed)
}
return err
}
func (s *TimerLayerStatusStore) UpdateExpiredDNDStatuses() ([]*model.Status, error) {
start := time.Now()