mattermost-cloud icon indicating copy to clipboard operation
mattermost-cloud copied to clipboard

[MM-44904] Adding installations to group based on group total installations

Open fmartingr opened this issue 1 year ago • 1 comments

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

fmartingr avatar Oct 21 '22 15:10 fmartingr

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 are SELECT ID, Name, (SELECT COUNT()...) Count FROM ... queries
  • LeftJoin are the left join queries
  • SelectThenSelect are the SELECT * FROM Group then SELECT COUNT() FROM Installations on each group from the first query (this is the DTO approach).
  • The numbers mean All for all rows (~2000) and 200 for LIMIT 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

fmartingr avatar Oct 25 '22 19:10 fmartingr