`KVCompareAndSet(key, sameValue, sameValue)` can fail spuriously on MySQL if the underlying `UPDATE` requires no actual changes. As per the [MySQL documentation](https://dev.mysql.com/doc/refman/8.0/en/mysql-affected-rows.html), we can't rely on rows affected in this case:
> For UPDATE statements, the affected-rows value by default is the number of rows actually changed. If you specify the CLIENT_FOUND_ROWS flag to mysql_real_connect() when connecting to mysqld, the affected-rows value is the number of rows “found”; that is, matched by the WHERE clause.
It's not tenable to change `CLIENT_FOUND_ROWS` for the all connection, so handle this case in the code instead by running a `SELECT` after the fact. Note that `KVCompareAndSet` has no guarantee of atomicity in this case, but neither would `CompareAndSwap` on which this is method was inspired.
Finally, note that no changes are required for Postgres, which has sane semantics as the default.
Fixes: https://mattermost.atlassian.net/browse/MM-21328
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* MM-12773: Fail if creating ChannelMemberHistory record fails.
* MM-12773: Guard against attempting to create duplicate channel member history record.
* MM-21481: Fixed several issues from user marshalling
- Fixed the root cause for panic by properly converting
the map to a User struct.
- Added a check for type conversion for extra safety.
- Fixed a somewhat unrelated issue of a pointer to pointer reference.
* Fix tests
* MM-19463 - Migrate tests from "web/webhook_test.go" to use testify
* fix shadows declaration
* make test cleaner
* fix wrong test order
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* Change link to redirect email notifications to landing page
* Fix to undefined variable
* Change `vault` to `landing`
* Added a couple tests
Co-authored-by: David Meza <dmeza@users.noreply.github.com>
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* promote user to admin upon login
* Add SAML support for admin accounts
* update en.json
* update i18n
* Add tests as per comments
* change function name
* fix config.go
* invalidate cache so its not checking for cache when roles change
* add enable attribute and filter
* MM-21356: Conditionally set user status online
Check for the set_online query param and do not set the status
if it is set to false.
* Fix some issues
* Add a test
* Log an error if an invalid value was passed
* Logging a warning instead of error
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* MM-20644: Add users to teams as a SchemeAdmin based on a new configuration field on GroupTeams and GroupChannels records.
* MM-20644: Adds SchemeAdmin to mapping of the GroupSyncable struct fields.
* MM-2064: Adds test to ensure SchemeAdmin field value is mapped.
* MM-20644: Adds missing index creation for fresh DBs.
* MM-20644: Duplicates UpdateMembersRole across Team and Channel stores. Adds tests.
* MM-20644: Fixes some old method name references.
* MM-20644: Moves variable declaration; removes Println statement.
* MM-20644: Use a SQL query instead of two to update Team and Channel members.
* MM-20644: Fixes tests; updates query.
* MM-20644: Fix permission check for patching a group syncable.
* MM-20644: Fixes test for change of permissions verification in group patch API request.
* MM-20644: Fix for ORM select vs insert.
* MM-20644: Linting fixes.
* MM-20644: Fixes some tests.
* MM-20644: Skips changing the role of guests.
* MM-20644: Added syncableID filtering
* Ensure both DeleteAt and UpdateAt are set to same value
* fix another point this could happen
* Revert "fix another point this could happen"
This reverts commit 5deb102e78.
* MM-21211: Optimize Channelstore.UpdateLastViewedAt
For postgres, we use CTEs to do both queries at one
and return the data.
For mysql, CTEs aren't available until version 8. So we just optimize the
existing query.
- We use IN instead of multiple OR conditions for the channel membership check.
According to mysql https://dev.mysql.com/doc/refman/5.7/en/comparison-operators.html#operator_in:
If no type conversion is needed for the values in the IN() list,
they are all constants of the same type, and expr can be compared to each of them
as a value of the same type (possibly after type conversion), an optimization takes place.
The values the list are sorted and the search for expr is done using a binary search,
which makes the IN() operation very quick.
- We use the previous column LastViewedAt to replace the value of LastUpdateAt
instead of doing a switch case again.
https://dev.mysql.com/doc/refman/5.6/en/ansi-diff-update.html
If you access a column from the table to be updated in an expression, UPDATE uses the
current value of the column.
Results (ran with getchannel profile):
Postgres:
| Metric | Baseline | Actual | Delta | Delta % |
| --- | --- | --- | --- | --- |
| Hits | 1594 | 1574 | -20 | -1.25%
| Error Rate | 0.00% | 0.00% | 0% | 0% |
| Mean Response Time | 7.08ms | 6.36ms | -0.72ms | -10.20% |
| Median Response Time | 5.00ms | 4.00ms | -1.00ms | -20.00% |
| 95th Percentile | 9.00ms | 8.00ms | -1.00ms | -11.11% |
Mysql:
| Metric | Baseline | Actual | Delta | Delta % |
| --- | --- | --- | --- | --- |
| Hits | 1600 | 1566 | -34 | -2.12%
| Error Rate | 0.00% | 0.00% | 0% | 0% |
| Mean Response Time | 7.39ms | 7.02ms | -0.37ms | -4.94% |
| Median Response Time | 6.00ms | 6.00ms | 0ms | 0% |
| 95th Percentile | 10.00ms | 10.00ms | 0ms | 0% |
* Add comment on using CTEs for mysql8
* incorporate review comments
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* Refactor to use structured logging
* Properly formatted with gofmt
* created interface Cache, but construction of cache is still coupled.
* Implementing cache factories to build caches
* Simple redis implementation without error handling. Keys and values by default are string
* refactor NewLocalCacheLayer to inject cache factory
* Removed redis impl to focus on cache abstraction
* CacheFactory injected on sqlsupplier and saved in Store struct
* remove useless private method
* replace concrete declaration of lru cache to cache abstraction
* discard spaces
* Renamed factory to provider because concrete implementations of factories may hold an state (such as a redis client for example)
* refactor to include all caches in the same package cache and subpackages
* method close cache. This method will be used for concrete implementations that need to release resources (close a connection, etc)
* closing cacheprovider and releasing resources while closing sql store
* fixed merge conflict fail
* gofmt files
* remove unused property from post_store
* naming refactor to avoid stutter. Added godocs on interface
* Store doesnt know anything about the cache and provider. Cache provider will be built after loading config and injected in localCacheLayer
* fixed broken test
* cache provider initialized before RunOldAppInitialization which initializes the localcachelayer
* move statusCache to server to initialize it with the new cache provider
* update terms_service and channel_layer to have new cacheProvider
* gofmt
* Add Connect method to the cache provider
* mock cacheprovider in user_layer_test
Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
While sending the config broadcast message across a cluster, we would
include the environment overrides in the config.
We fix this by exposing a config config store method to remove the overrides,
and then calling that before saving the config across the cluster.
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
