Postgres errors after upgrading from 1.6.0 to 1.7.0

[laurNtiu]

Postgres errors after upgrading from 1.6.0 to 1.7.0

[laurNtiu]

Since I'm a very early adopter, I thought this was worth reporting, indeed.

We're not running HA mode, it's just a normal Mattermost server running in a container. I should have led with this:

Mattermost Version: 7.10.5
Database Schema Version: 108
Database: postgres

The plugin seems to be working, I just got a MR assigned to me and I see the unread message count changing. Clicking on it opens the expected side panel etc.

[laurNtiu]

@laurNtiu Just to double check: You are not running into any function issues with the GitLab plugins? It's "just" an annoying postgres error that get's logged.

Just took a bit of extra time to check with my users, nobody mentions issues with this release. Apparently the plugin works just fine.

That being said, I would still love to get rid of the Postgres error if possible.

[laurNtiu]

No further questions, guess we'll learn to live with this. At least we got rid of the invalid_token errors. So thanks for that, @mickmister, I'd say on balance we're ahead on points :)

When I initially repoted this, to be honest, I was afraid it would be something nastier. Thanks also to @hanzei for looking into it.

[mickmister]

Hi @laurNtiu, thanks for reporting this.

Is the plugin functioning correctly for you in general? I would expect this error to make it so things like fetching the counts in the left side of the UI to not work.

Looking at the error, the duplicate key is the combination of the pluginid and pkey fields. So this means that there existed a pkey mutex_jgojo79ipifwjqxchz5ga7mcih-oauth-token when the error happens, but it's not present now. The entries in the PluginKeyValueStore are unrelated because they do not have a mutex_ key.

As far as what's causing the issue, I think there may be an issue with cluster.Mutex that doesn't properly take into account if there is already one in the database, though that's definitely something it should be taking into account for the purpose of that utility. We may want to revert the feature that introduced this. Not sure yet, but I'll come back with some more info on next steps when I have them.

mattermost-plugin-gitlab/server/plugin.go

Line 883 in 835b440

mutex, err := cluster.NewMutex(p.API, info.UserID+TokenMutexKey)

cc @hanzei

[mickmister]

@laurNtiu So I can understand more about your environment, are you running your Mattermost server in High Availability mode?

[mickmister]

Since I'm a very early adopter, I thought this was worth reporting, indeed.

@laurNtiu I'm curious, what's you method of monitoring the releases to install them from GitHub?

[laurNtiu]

Since I'm a very early adopter, I thought this was worth reporting, indeed.

@laurNtiu I'm curious, what's you method of monitoring the releases to install them from GitHub?

It's just this time. We were very keen on getting rid of the "invalid_token" errors in our monitoring systems, so when I got back from holiday and saw that 1.7.0 came out with #308 fixed, I thought I'd give it a spin. I don't usually install plugins 13h after release :)

[hanzei]

@laurNtiu Just to double check: You are not running into any function issues with the GitLab plugins? It's "just" an annoying postgres error that get's logged.

[hanzei]

@mickmister The postgres error logged here is nothing new. It exists since CompareAndSet got introduced.

https://github.com/mattermost/mattermost/blob/9aba21e291dbfc44b1b715a1f24259ae937725db/store/sqlstore/plugin_store.go#L95-L112

The code tries to insert the new value but fails as another mutex already got the lock. It checks for that error and returns return false, nil.

[mickmister]

That being said, I would still love to get rid of the Postgres error if possible.

@laurNtiu I'm not sure if this can go away. Unique key constraints are something we use across the codebase (this case is in the code linked in the comment above), to allow the database itself to perform uniqueness checks when possible. You should see this same sort of "error" in other places, like opening a "new" DM channel with a user that you already have a DM channel with.

Since this is being logged in the database logs themselves, I don't think Mattermost itself can avoid this log. I'm not sure if there's a way to selectively avoid it on the database logging side either.

I'm closing this issue, as this behavior is expected and likely won't change.

@laurNtiu Thanks again for reporting this, and for using Mattermost and this plugin. Please let me know if you have any questions on this.