commit eb343023b7a26b66a3c356da75958bb7db3f2112
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

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>