[MM-46554] Add load test functionality for global drafts #527

mylonsuren · 2022-12-02T15:59:21Z

Summary

Adds load test coverage for drafts.

Ticket Link

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

agnivade · 2022-12-06T05:57:23Z

@mylonsuren - just checking on the status for this. What else is pending here?

mylonsuren · 2022-12-06T15:21:47Z

@mylonsuren - just checking on the status for this. What else is pending here?

@agnivade Not much, I just need to add coverage for deleting drafts and update the frequencies. Currently have some other high priority tasks in front of this, will update this PR in the coming days and get some reviews then 😄

agarciamontoro · 2023-01-17T10:09:34Z

Hey, @mylonsuren, anything else we can help with to get this ready for review?

mylonsuren · 2023-01-30T16:10:48Z

@agarciamontoro @agnivade Sorry for the delay on this, I think it's at a point now where it can get some reviews 😄

agnivade · 2023-01-31T05:21:50Z

deployment/terraform/strings.go

@@ -17,6 +17,7 @@ WorkingDirectory=/opt/mattermost
 User=ubuntu
 Group=ubuntu
 LimitNOFILE=49152
+Environment=MM_FEATUREFLAGS_GlobalDrafts


We need to append =true at the end

agnivade · 2023-01-31T05:28:37Z

loadtest/control/simulcontroller/actions.go

+	numFiles := len(fileIds)
+	for i := 0; i < numFiles; i++ {
+		draft.FileIds = append(draft.FileIds, <-fileIds)
+	}
+


This is a complicated way to achieve this. We can just have a slice of fileIds and let each goroutine set the fileId at separate indices of that slice. That's perfectly valid and not a race condition. And then just append that slice to draft.FileIds

agnivade · 2023-01-31T05:48:42Z

loadtest/control/simulcontroller/actions.go

+	userId := u.Store().Id()
+
+	team, err := u.Store().CurrentTeam()
+	if errors.Is(err, memstore.ErrTeamStoreEmpty) {


This is only returned from Store().RandomTeam(), so no need to check it here

agnivade · 2023-01-31T05:49:34Z

loadtest/control/simulcontroller/actions.go

+		Message:   message,
+		ChannelId: channel.Id,
+		RootId:    rootId,
+		CreateAt:  time.Now().Unix() * 1000,


Use model.GetMillis

agnivade · 2023-01-31T05:49:54Z

loadtest/control/simulcontroller/actions.go

+	var rootId = ""
+
+	// 33% of the time draft will be a thread reply
+	if rand.Float64() < 0.33 {


How was this derived?

agnivade · 2023-01-31T05:50:31Z

loadtest/control/simulcontroller/actions.go

+	}
+
+	team, err := u.Store().CurrentTeam()
+	if errors.Is(err, memstore.ErrTeamStoreEmpty) {


Same as earlier

agnivade · 2023-01-31T05:50:55Z

loadtest/control/simulcontroller/controller.go

+		{
+			run:       c.upsertDraft,
+			frequency: 0.615,
+		},
+		{
+			run:       c.getDrafts,
+			frequency: 0.555,
+		},
+		{
+			run:       c.deleteDraft,
+			frequency: 1.5,
+		},


Are all of these calculated from community metrics?

As per data from last week, the frequencies should be something more like:

upsertDrafts: 1.5 * 0.622 = 0.933 getDrafts: 1.5 * 0.376 = 0.564 deleteDraft: 1.5 * 1.17 = 1.755

Also, can we use the minServerVersion field in these actions? We should add that from now on to all new actions, and I want to start adding them to the current ones as well.

agarciamontoro

Thank you for this, @mylonsuren, it's great :) I left some comments to tackle, but the approach is flawless, thank you!

agarciamontoro · 2023-02-06T15:38:59Z

loadtest/store/memstore/store.go

+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	s.drafts[teamId] = map[string]*model.Draft{}


Here we override the whole team drafts every time. Is that intended? If so, we should specify it in the function documentation.

agarciamontoro · 2023-02-06T15:53:44Z

loadtest/control/simulcontroller/controller.go

+		{
+			run:       c.upsertDraft,
+			frequency: 0.615,
+		},
+		{
+			run:       c.getDrafts,
+			frequency: 0.555,
+		},
+		{
+			run:       c.deleteDraft,
+			frequency: 1.5,
+		},


As per data from last week, the frequencies should be something more like:

upsertDrafts: 1.5 * 0.622 = 0.933 getDrafts: 1.5 * 0.376 = 0.564 deleteDraft: 1.5 * 1.17 = 1.755

agarciamontoro · 2023-02-06T16:00:21Z

loadtest/control/simulcontroller/actions.go

+		return control.UserActionResponse{Info: fmt.Sprintf("no drafts found in team %v", team.Id)}
+	}
+
+	err = u.DeleteDraft(channel.Id, draftId)


Here, channel.Id and draftId` may be the same ID if the draft is a root post, right? I understand that is correct, but I wanted to double-check, this rootid-channelid for drafts was a bit confusing for me at first.

agarciamontoro · 2023-02-06T16:07:09Z

loadtest/store/memstore/store_test.go

+		draftId, err := s.RandomDraftForTeam(teamId)
+		require.NoError(t, err)
+		require.Equal(t, draft.ChannelId, draftId)


We can actually check the store directly, with s.drafts[teamId]

agarciamontoro · 2023-02-06T16:07:18Z

loadtest/store/memstore/store_test.go

+		draftId2, err := s.RandomDraftForTeam(teamId2)
+		require.NoError(t, err)
+		require.Equal(t, draft2.RootId, draftId2)


Same comment.

agarciamontoro · 2023-02-06T16:07:45Z

loadtest/store/memstore/store_test.go

+		draftId, err := s.RandomDraftForTeam(teamId)
+		require.NoError(t, err)
+		require.Equal(t, draft.ChannelId, draftId)


Same comment. And here it'd actually let you to add more than one draft to check that the whole set is added.

agarciamontoro · 2023-02-06T16:07:52Z

loadtest/store/memstore/store_test.go

+		draftId2, err := s.RandomDraftForTeam(teamId2)
+		require.NoError(t, err)
+		require.Equal(t, draft2.RootId, draftId2)


And same here.

agarciamontoro · 2023-02-06T16:09:57Z

loadtest/control/simulcontroller/controller.go

+		{
+			run:       c.upsertDraft,
+			frequency: 0.615,
+		},
+		{
+			run:       c.getDrafts,
+			frequency: 0.555,
+		},
+		{
+			run:       c.deleteDraft,
+			frequency: 1.5,
+		},


Also, can we use the minServerVersion field in these actions? We should add that from now on to all new actions, and I want to start adding them to the current ones as well.

mattermost-build · 2023-02-17T01:01:07Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

agarciamontoro · 2023-02-21T12:52:02Z

:( I will address all comments here so we can merge this PR.

agnivade · 2023-03-30T08:50:49Z

Ah this is indeed unfortunate, hopefully you'll have the time to get this merged. No rush, this is not a priority.

agnivade · 2024-03-08T04:01:54Z

Sad that it's been lying for a year. Let's see if I can find some time to clean this up.

agarciamontoro · 2024-03-08T11:14:51Z

Sad that it's been lying for a year. Let's see if I can find some time to clean this up.

I've been wanting to get this merged since last year, but there was always something more urgent in the queue. Very glad if you want to tackle it!

Add draft actions

831c998

mylonsuren added the Work In Progress Not yet ready for review label Dec 2, 2022

mylonsuren self-assigned this Dec 2, 2022

mylonsuren added 2 commits December 9, 2022 16:40

Update frequencies and add deleteDraft functionality

cc1bdaa

add config setting for AllowSyncedDrafts

480e269

mylonsuren added 5 commits January 25, 2023 01:17

Update to use store

3b8d31a

Remove extra drafts functions

c89b3ef

Merge branch 'master' into MM-46554_load-test-global-drafts

143e7d2

add tests for store

1c3da95

Merge branch 'master' into MM-46554_load-test-global-drafts

4f6739f

mylonsuren marked this pull request as ready for review January 30, 2023 16:09

mylonsuren added 2: Dev Review Requires review by a core committer and removed Work In Progress Not yet ready for review labels Jan 30, 2023

mylonsuren requested review from agarciamontoro and agnivade January 30, 2023 16:09

agnivade reviewed Jan 31, 2023

View reviewed changes

agarciamontoro requested changes Feb 6, 2023

View reviewed changes

mattermost-build added the Lifecycle/1:stale label Feb 17, 2023

agarciamontoro assigned agarciamontoro and unassigned mylonsuren Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-46554] Add load test functionality for global drafts #527

[MM-46554] Add load test functionality for global drafts #527

mylonsuren commented Dec 2, 2022 •

edited

agnivade commented Dec 6, 2022

mylonsuren commented Dec 6, 2022

agarciamontoro commented Jan 17, 2023

mylonsuren commented Jan 30, 2023

agnivade Jan 31, 2023

agnivade Jan 31, 2023

agnivade Jan 31, 2023

agnivade Jan 31, 2023

agnivade Jan 31, 2023

agnivade Jan 31, 2023

agnivade Jan 31, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro left a comment

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

agarciamontoro Feb 6, 2023

mattermost-build commented Feb 17, 2023

agarciamontoro commented Feb 21, 2023

agnivade commented Mar 30, 2023

agnivade commented Mar 8, 2024

agarciamontoro commented Mar 8, 2024 •

edited

[MM-46554] Add load test functionality for global drafts #527

Are you sure you want to change the base?

[MM-46554] Add load test functionality for global drafts #527

Conversation

mylonsuren commented Dec 2, 2022 • edited

Summary

Ticket Link

agnivade commented Dec 6, 2022

mylonsuren commented Dec 6, 2022

agarciamontoro commented Jan 17, 2023

mylonsuren commented Jan 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarciamontoro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattermost-build commented Feb 17, 2023

agarciamontoro commented Feb 21, 2023

agnivade commented Mar 30, 2023

agnivade commented Mar 8, 2024

agarciamontoro commented Mar 8, 2024 • edited

mylonsuren commented Dec 2, 2022 •

edited

agarciamontoro commented Mar 8, 2024 •

edited