mattermost-cloud
mattermost-cloud copied to clipboard
[MM-44904] Adding installations to group based on group total installations
Summary
Refactor of the group allocation for new installations.
Previously, group allocation was performed randomly based on group annotations. This system is changed with this PR using the group total number of installations, choosing the group with the lowest count.
~~Previously, group allocation was performed randomly based on group annotations. This system is changed with this PR introducing the concept of a Group Allocator, which receives a list of groups and selects one based on it internal logic.~~
~~Two group allocations have been created, one Random, to preserve the original logic, and a new one to select a group based on the lowest count of total or updated installations.~~
Database utilisation is quite heavy (since we need 6 queries to get the status for one group). If we only need the total installation count I could refactor the query or add a new method to the store/filter to retrieve the total installation count directly with the groups. (Which is probably the best way ahead for the time being).
Ticket Link
https://mattermost.atlassian.net/browse/MM-44904
Release Note
- Automatically add new installations to a group based on lowest installation count
I did some benchmarks only to scratch an itch and triple check that joins are way faster than anything else.
BenchmarkSelectOnlyAll-10 1286 827112 ns/op
BenchmarkSelectWithinSelectAll-10 2 874080562 ns/op
BenchmarkLeftJoinAll-10 142 8309563 ns/op
BenchmarkSelectThenSelectAll-10 7 191876863 ns/op
BenchmarkSelectOnly200-10 6621 170949 ns/op
BenchmarkSelectWithinSelect200-10 6 175394083 ns/op
BenchmarkLeftJoin200-10 176 6941688 ns/op
BenchmarkSelectThenSelect200-10 34 37195855 ns/op
The data:
-
SelectOnly
are selects without the count -
SelectWithinSelect
areSELECT ID, Name, (SELECT COUNT()...) Count FROM ...
queries -
LeftJoin
are the left join queries -
SelectThenSelect
are theSELECT * FROM Group
thenSELECT COUNT() FROM Installations
on each group from the first query (this is the DTO approach). - The numbers mean
All
for all rows (~2000) and200
forLIMIT 200
(since that’s the default page size). - Data is not real enough, since the logic for selecting a group has more to it, but it is close enough.
As per our conversation, it makes more sense to have this on the DTO instead of modifying underlying logic, since this relates to two different tables. Even with the performance benefits, this piece of logic is only used at one place at the moment (I've left a comment in the code).
As a fun note, this line of code drove me crazy for some time while developing the tests (I was specifying a custom ID on the tests). /me laughs hysterically