search

mattermost-community / mattermost-plugin-welcomebot

Apache License 2.0
66 stars 41 forks source link

Duplicate categories are created if new members are assigned to channels and channels are assigned to categories #120

Open christianhueserhzdr opened 1 year ago

christianhueserhzdr commented 1 year ago

The automatic assignment of new team members to channels and the triggered automatic assignment of this channels to the new member's categories creates duplicate categories.

I assume that the assignment of members to channels triggers the assignment of channels to categories. If this member to channel assignments are not finished when the channel to category assignments start there will be duplicate categories because when the check occurs if the category is already present or needs to be created there are several triggered channel to category assignments which all think that no category has yet been created, thus they all create duplicate categories. Which is an issue of concurrency.

I would suggest

  1. to not trigger the channel to category assignments before all the member to channel assignments are done, or
  2. to resolve this issue of concurrency adequately.

Please let me know if this an issue that the Mattermost server can only deal with and I create an issue there.

Thank you very much for looking into this issue and for your valuable feedback! :slightly_smiling_face:

christianhueserhzdr commented 11 months ago

Is there any update on this issue?

christianhueserhzdr commented 8 months ago

Do you need any additional information about this issue from my side? Thanks for letting me know!

mickmister commented 8 months ago

HI @christianhueserhzdr, thank you for filing this issue. I'm thinking we can solve this in the welcomebot plugin, by debouncing the operations of adding the user to multiple channels.

We currently do not do any "waiting" in between adding the user to different channels. If we add some wait period (~3seconds) I think that would be enough to avoid this issue. What are your thoughts on this?

Here are the two places we are doing this operation in a loop:

https://github.com/mattermost/mattermost-plugin-welcomebot/blob/502eb34dc47db99c6b0b96e630fba4ee3516ad2e/server/welcomebot.go#L201-L203

https://github.com/mattermost/mattermost-plugin-welcomebot/blob/502eb34dc47db99c6b0b96e630fba4ee3516ad2e/server/welcomebot.go#L114-L116

mickmister commented 8 months ago

@raghavaggarwal2308 For context, there is a feature called "Channel Actions" in the playbooks plugin https://github.com/mattermost/mattermost-plugin-playbooks, that allows an admin to configure the channel to make it so whenever a new user joins the channel, they will have that channel added to a specific channel category.

It seems there is a race condition happening with the welcomebot and playbooks plugins, where the welcomebot plugin is adding users to the channels too quickly, so the "channel category add" process can end up creating new categories instead of using existing ones, which results in duplicate channel categories. If we slow down the welcomebot's channel member modifications to avoid this race condition, I think the issue will be avoided.

christianhueserhzdr commented 8 months ago

Thank you very much, Michael @mickmister, for looking into this issue report and writing down your opinion on how to resolve it.

Regarding your suggestion, adding some wait period is a quick fix in some cases, but personally I do not consider waiting times to be very sustainable. This issue will recur from time to time when the waiting period has not been long enough. On the other hand, introducing a waiting period of 3 seconds or more will result in a longer processing times, if several tens of channels and multiple categories are involved as already seen in some cases.

I would very much prefer to see that these race conditions are solved, if this is possible. I can imaging that this needs to be solved on both sides, Mattermost and the plugin.

Anyhow, thank you very much for your feedback and thoughts! I am glad that someone is looking after this issue. :slightly_smiling_face:

Please let me know how you would like to solve it. Thanks!

Christian

mickmister commented 8 months ago

@christianhueserhzdr We've decided to handle this issue in both places (waiting in the plugin, and avoiding race conditions in the platform). I'm thinking the plugin fix will arrive sooner. We'll keep you updated here. Thanks @christianhueserhzdr!