* Make UpdateSidebarCategories return the original categories
* MM-20897 Add category muting
* Prevent muting the DMs category
* Fix muted state not being stored in the database
* Address feedback
* Address some feedback
* Fix unit tests
* MM-20897 Mute/unmute channels in the database in bulk
* Satisfy golangci-lint
* MM-28067: Optimize app server startup in tests
For every test, we would wipe out the database and then start
the server again. This would run through all the migrations
and create new rows in Systems and Roles table every time.
As a result, this was consuming a lot of setup time for every test.
We optimize this by preloading the DB with dummy data in the roles
and systems table so that the server code just skips over those migrations.
This is completely forward compatible and adding new migrations does not
need to generate the sql files again. Only in case of schema changes to the
roles or systems table, this would need to be done.
It is unlikely the Systems table schema will get changed. The Roles table
might change in future but it's a comparatively rare event. Given the reduction
in CI runtime we are seeing, it's a worthy optimization.
We also apply some more optimizations:
- Coalesce multiple UpdateConfig calls into a single one. Each UpdateConfig call
has to do a json marshal which would take precious CPU cycles. It's much more
efficient to do everything in a single call.
- Remove unnecessary debug.FreeOSMemory in reload config. This was an artifact
from old days and is no longer required.
Numbers:
Results show a full **2 minutes** shaved off the test runtime. Earlier, tests
would take around 16 minutes. Now they take 14 minutes.
```release-note
NONE
```
* fix app package
* Refactor apply multi role filters and add role filters to get all profiles
* Add some tests
* Fix tests
* Fix lint
* Trigger CI
* Rename param to make more sense
* Tie get filtered user stats to usermanagement read users
* Dont filter out other system roles when searching for team members or team admins only filter out system admins
* add new permissions
* add migration
* fix test
* remove system roles as default permissions
* implement changes discussed with dennis
* add read only and fix i18n
* use model consts instead of strings
* turn the permissions into pseudo constants
* Update read only default permissions
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Hossein Ahmadian-Yazdi <hyazdi1997@gmail.com>
* MM-30026: Use DB master when getting team members from a session
A race condition happens when the read-replica isn't updated yet
by the time a session expiry message reaches another node in the cluster.
Here is the sequence of events that can cause it:
- Server1 gets any request which has to wipe session cache.
- The SQL query is written to DB master, and a cluster message is propagated
to clear the session cache for that user.
- Now before the read-replica is updated with the master’s update,
the cluster message reaches Server2. The session cache is wiped out for that user.
- _Any random_ request for that user hits Server2. Does NOT have to be
the update team name request. The request does not find the value
in session cache, because it’s wiped off, and picks it up from the DB.
Surprise surprise, it gets the stale value. Sticks it into the cache.
By now, the read-replica is updated. But guess what, we aren’t going to
ask the DB anymore, because we have it in the cache. And the cache has the stale value.
We use a temporary approach for now by introducing a context in the DB calls so that
the useMaster information can be easily passed. And this has the added advantage of
reusing the same context for future DB calls in case it happens. And we can also
add more context keys as needed.
A proper approach needs some architectural changes. See the issue for more details.
```release-note
Fixed a bug where a session will hold on to a cached value
in an HA setup with read-replicas configured.
```
* incorporate review comments
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
* MM-30300: Add coalesce to get null content as empty string on FileInfo store
* Making explicit the fields get from the FileInfo table
* Addressing PR review comments
* Fix the team and channel filtering UT to include empty team or channel
* Fix tests that were failing before this change
Once we've activated the team/channels filter tests for PostgreSQL
and MySQL there are some tests failing so this changes fixes them
* Disable team filtering tests for DBs by now
We have a discrepancy between DB search and ES/Bleve on how to filter
teams when you have users in both teams:
- DB when filtering by one team and searching by another returns users that are in both teams
- Bleve and ES returns empty
Co-authored-by: Mario de Frutos <mario@defrutos.org>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
* MM-29980: Optimize profilesInChannels cache to fast path
We add one more message type to the fast path- profiles in channels. There
are 2 primary reasons for this:
- This is not really a new model type, but just a map of users. And users already use
the fast path. So we can get some more gains without really investing much more code.
- A more important reason is that with the upcoming striped mutex changes, we will get
a higher throughput at the cost of a bit more CPU utilization. The reason being that
since less amount of time will be spent in lock-contention, the CPU is free to do more
stuff. So this change is to counter that increase.
As usual, this gives much better performance than the original decoder.
Micro-benchmark results
```
name old time/op new time/op delta
LRU/UserMap=new-8 16.6µs ± 3% 3.9µs ± 4% -76.15% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
LRU/UserMap=new-8 4.78kB ± 0% 2.74kB ± 0% -42.65% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
LRU/UserMap=new-8 38.0 ± 0% 30.0 ± 0% -21.05% (p=0.000 n=10+10)
```
https://mattermost.atlassian.net/browse/MM-29980
Here are some results from a load test. The comparison is done with a 2 node cluster; one running master
and one running with this patch so that it's easier to compare. The total users are 2000.
<See PR>
```release-note
NONE
```
* Fix gofmt
* Trigger CI
* Add the content field to FileInfo
* Fixing the upgrade code
* Trying to fix the text-scheme
* Fixing test-schema
* Fixing test-schema
* Moving the migration to the next version
* MM-28815: Optimizes message export query.
* MM-28815: Removes DeleteAt WHERE condition.
* MM-28815: Adds Id to the ORDER BY clause.
* MM-28815: Concatenates the string to remove the confusing '%%' in the query.
* MM-28115: Remvoes secondary sorting.
* MM-28815: Keeps the alias in the order by.
* MM-28815: Removing forced index.
* MM-28815: Formatting.
* Adding godocs for the 4 functions listed:
SqlTeamStore.UpdateLastTeamIconUpdate
SqlTeamStore.UpdateMembersRole
SqlTeamStore.UserBelongsToTeams
SqlTeamStore.GetTeamMembersForExport
* Apply suggestions from code review
Applying suggestions from the community to help clean up the grammar in the docs.
Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>
* Apply suggestions from code review part 2
Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>
Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
* Migration completed
* Fix tests
* Reduce to one line
* Fix: change to plain error
* Fix imports
* Trigger CI
* Fix i18
* Fix merge with master
* Trigger CI
MM-27744 disable Zap for unit tests.
Zap has no concept of shutdown or close. Zap is only shutdown when the app exits. Not a problem for console logging, but when creating a new Zap logger that outputs to files on every unit test, that leaves no easy way to clean up until process exit. Depending on what else is running this can exhaust all file handles and cause unit tests to fail.
Zap is now disabled unit tests and uses Logr instead, regardless of config settings. `make test-server` peak file handle usage dropped from ~5K to less than 100.
GetPostsSince is used when loading posts for a channel.
An opportunity for optimization is that the primary SQL query
is repeated twice and then a UNION is constructed for the results.
```
SELECT
*
FROM
Posts
WHERE
UpdateAt > :Time AND ChannelId = :ChannelId
LIMIT 1000
```
But we can use a CTE for this which caches the results to be reused later.
This leads to the main query being executed once rather than twice. And from
Postgres 12 onwards, CTEs can be inlined which opens the door to further optimizations.
From the docs (https://www.postgresql.org/docs/10/queries-with.html)
> A useful property of WITH queries is that they are evaluated only once per
execution of the parent query, even if they are referred to more than once
by the parent query or sibling WITH queries. Thus, expensive calculations
that are needed in multiple places can be placed within a WITH query to avoid redundant work.
Another possible application is to prevent unwanted multiple evaluations of functions
with side-effects. However, the other side of this coin is that the optimizer is
less able to push restrictions from the parent query down into a WITH query than an ordinary subquery.
In our case, the caveat does not apply because we are only filtering columns and not rows,
so we can safely use it.
Following are the query plan comparisons:
Old: http://tatiyants.com/pev/#/plans/plan_1599394993105
New: http://tatiyants.com/pev/#/plans/plan_1599395970886
As we can see, in old bitmap index scan+heap scan happens twice, but in the new one,
it happens only once.
This has been load tested with a large dataset and confirmed to exhibit good improvements.
We got hit by yet another random text search issue, but this was
on the more extreme end because this was a string of length 4 and still
it collided.
I am not sure if there is a fool proof solution to this than just
going for a larger string.
https://mattermost.atlassian.net/browse/MM-27953