* 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 5deb102e782bcc1da103b1b10715c066d9cc6130.
* 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>
* MM-19628: Fix race in (*registeredPlugin).State access
We used a pointer to an integer in the State variable, and therefore
setting a pointer value directly caused a race condition.
Since the registeredPlugin is a private struct, changing it to a normal int
creates fewer pointers and lets us to fix the race condition by simply setting
the integer value and then doing a registeredPlugins.Store.
The alternative to keeping the pointer would be to create a copy of each
member of a registeredPlugin and do a store again, which I feel is a roundabout
way to achieve this.
* Address review comments
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
While invalidating a channel, we send out a cluster message
from the app layer as well as from the localcachelayer.
This causes multiple redundant messages to be sent out.
And additionally multiple cluster handlers were being registered
from the app layer as well as the localcachelayer.
We fix this by removing the cluster broadcast from the app layer
and directly calling the store method. We also remove
the duplicate event handler registration.
* MM-21336: Improve channel rename restrictions
Non-direct channels should not have __ in them.
* Fix order of i18n extraction
* Incorporate review comments
Use a more relaxed check by actually checking for valid userIds.
* Improve error message
When push notification setting was selected to "All Activity", we were
just sending the unread count for the channel from where the message was sent
and not for all channels.
After discussion with @enahum and @migbot, we decided to keep the badge count
to only refer to mentions and not unread posts.
Therefore, we only take the unread count for the user irrespective of the
notify_props settings.
Updated the tests to reflect that.
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* MM-21209: Use the LRU cache for UserStore.Get call too
We already have userProfileByIdsCache to store the user profiles by Id.
It just wasn't being used for the (*UserStore).Get method. We add a wrapper
method in LocalCacheUserStore to intercept that call and check for the
presence of the user Id in the cache.
There is no need to add any code for invalidation as all of that is already
present.
* Fix nil check for rootstore
* Fix TestUserStore test
Added an invalidate call. The invalidation was being done
from the app level. Hence we have to do it manually here for the test.
Display LHS bot Icon in web app. As part of mentioned task, Added LastIconUpdate variable in model.bot to store last update time of icon. Also added code to update/delete value of the mentioned variable when setting/deleting bot icon.
* Make WebSocketEvent type immutable
* Update code to use updated immutable WebSocketEvent type
* Export WebSocketEvent fields and mark them as deprecated
* migrated termsOfServiceCache from sqlstore to localcachelayer, and mocked store tests
* revert gitignore local change
* fixed caching in termsOfService Get, and added basic tests for termsOfServiceCache
* added a test for cache save, and fixed call to Store tests
* fixed GeLatest termsOfService from cache test
* added license headers to terms_of_service cache files
* using doStandardReadFromCache and doStandardAddToCache when reading and writing to cache
* removed unused variable, termsOfServiceCacheName
* added special key for the latest termsOfService value in termsOfServiceCache
* updated license information on localcachelayer termsOfServiceCache files
* fixed not updating latest termsOfServiceCache on Get by ID, and invalidating cache cluster on termsOfServiceCache save
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
The `channelByNameCache` in the `GetByNames` query checks each teamId+channelName
combination in the cache and if any of the keys is not present, it queries the DB.
In this case, this is effectively a cache miss from the point of view of user.
But since the cache is at a teamId+channelName level, there may be several hits and some misses.
This is misleading as it does not generate proper metrics and might lead to a
false assumption of how effective the cache is.
We create separate metrics which account for all or none cache hits/misses.
This removes the old metrics which are used by other queries. So they are now separated,
and it will improve both the old and new metric. Because the old metric is not contaminated,
and the new metric is also separate.
* Migrate channelCache cache from store/sqlstore/channel_store.go to the new store/localcachelayer
* fix import for new path
* remove unnecessary code, refactor cache constant
* resolve conflicts
* fix missing variable and import after merging
* refactor code, fix current issue
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
