commit | eb343023b7a26b66a3c356da75958bb7db3f2112 | [log] [tgz] |
---|---|---|
author | Edwin Kempin <ekempin@google.com> | Wed Oct 07 15:54:13 2020 +0200 |
committer | Edwin Kempin <ekempin@google.com> | Mon Oct 12 12:19:38 2020 +0200 |
tree | fb0c722aa6f1962b73bc0f8dcce3ab6a1cb1dbe6 | |
parent | 3e04e87fc571b0a66765550612e4fb7a23e1755f [diff] |
Avoid loading all accounts when code ownership is assigned to all users When a code ownership was assigned to all users by using the '*' wildcard as email, CodeOwnerResolver was loading all accounts. Loading all accounts requires parsing all user refs. Then we checked the visibility of all accounts, which means that all accounts were retrieved from the cache and, depending on the accounts visibility setting, groups for all users had to be loaded. Doing this is too expensive and if collection of performance logs is enabled this can lead to memory exceeded errors because each account lookup creates performance logs which are collected in memory. To fix this, CodeOwnerResolver is no longer resolving '*' to all users. Instead we return a CodeOwnerResolverResult that contains all resolved code owners plus a flag telling whether all users are code owners. Returning this as a flag, rather than returning all accounts, is much cheaper, but it means that callers now must handle this case on their own. However they can do something smarter than loading all accounts. There are 2 callers for which this is relevant: 1. CodeOwnerApprovalCheck: Here we need to determine whether a code owner approved the change, or whether a code owner is a reviewer of the change. If a file is owned by all users and the approver set is non-empty, we know that the file is approved (since there is at least one approver and any user is a code owner). If there is no approval but a reviewer, we know that the status for the file is pending (since all users are a code owner, the reviewer must be a code owner). 2. AbstractGetCodeOwnersForPath: Here we list code owners, but listing of code owners is always done with a limit (if no limit is specified, there is a default limit). If all users are code owners we want to include random users until the limit is reached. For this we asked for 2 times the number of users that we still need to complete the suggestion list. We use the factor 2 here because some users may still be filtered out (e.g. because they are service users or because they cannot see the branch). When selecting random users for the suggestion we must ensure that the included users are visible to the caller. This means the logic that picks the random users depends on the configured account visibility: * ALL: If all accounts are visible, we can randomly pick any account. For this we retrieve the list of all accounts (getting this should be rather cheap) and select some random entries. * NONE: If no other account is visible, we return an empty list. * SAME_GROUP / VISIBLE_GROUP: These are tricky, because potentially we will again end up loading all accounts and all groups until we find enough visible accounts. To avoid this we do a best effort to find enough accounts. We pick a random sample of users that is 3 times the limit and filter out non-visible accounts, in the hope that the sample contained enough visible accounts to reach our limit. If we didn't find enough visible accounts, we return less code owners than the requested limit, although there might be further code owners available. This is a compromise that we have to accept in order to ensure good performance. In addition, if the calling user can view all accounts (via the VIEW_ALL_ACCOUNTS global capability) we always apply the same logic as for the ALL account visibility setting. Change-Id: I896edd3a08230106316b327e84f18fab239f4503 Signed-off-by: Edwin Kempin <ekempin@google.com>
This plugin provides support for defining code owners for files in a repository.
If the code-owners plugin is enabled, changes can only be submitted if all touched files are covered by approvals from code owners.
Also see resources/Documentation/about.md
IMPORTANT: Before installing/enabling the plugin follow the instructions from the setup guide, see resources/Documentation/setup-guide.md