The only other solution I could think of for Timeline was reducing the query to a tighter PhabricatorUser query instead of an overbroad BadgeAward query, if we don't want this on handles.

At least, querying a handful of Users seems cheaper than unlimited badge awards.

Hrrm, let me check if a huge ... UNION ALL ... query can work for now -- it's dumb but might be fine. And badges are always visible to everyone so we don't have to worry about weird policy stuff.

um this is getting spooky

Something like this seems to "work":

diff --git a/src/view/phui/PHUITimelineView.php b/src/view/phui/PHUITimelineView.php
index 57b07a94bf..2c263e2fe9 100644
--- a/src/view/phui/PHUITimelineView.php
+++ b/src/view/phui/PHUITimelineView.php
@@ -243,30 +243,69 @@ final class PHUITimelineView extends AphrontView {
       return;
     }
 
+    // This is an unusual query: we want to select a set number of awards for
+    // each of a potentially large number of users. Query classes don't
+    // normally support this kind of query. Get the results in a single query
+    // by merging a lot of individual queries using UNION ALL.
+
+    $award_table = new PhabricatorBadgesAward();
+    $badge_table = new PhabricatorBadgesBadge();
+    $conn = $award_table->establishConnection('r');
+
+    $limit = 2;
+
+    $parts = array();
+    $idx = 0;
+    foreach ($user_phids as $user_phid) {
+      $parts[] = qsprintf(
+        $conn,
+        'SELECT * FROM (
+          SELECT award.recipientPHID, award.badgePHID
+            FROM %T award JOIN %T badge
+            ON award.badgePHID = badge.phid
+            WHERE award.recipientPHID = %s AND badge.status IN (%Ls)
+            ORDER BY award.id DESC LIMIT %d) awards_%d',
+        $award_table->getTableName(),
+        $badge_table->getTableName(),
+        $user_phid,
+        array(
+          PhabricatorBadgesBadge::STATUS_ACTIVE,
+        ),
+        $limit,
+        $idx++);
+    }
 
-    $awards = id(new PhabricatorBadgesAwardQuery())
-      ->setViewer($this->getViewer())
-      ->withRecipientPHIDs($user_phids)
-      ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
-      ->execute();
+    $rows = queryfx(
+      $conn,
+      '%Q',
+      implode(' UNION ALL ', $parts));
+
+    if (!$rows) {
+      return;
+    }
 
-    $awards = mgroup($awards, 'getRecipientPHID');
+    $badge_phids = array();
+    $badge_map = array();
+    foreach ($rows as $row) {
+      $recipient_phid = $row['recipientPHID'];
+      $badge_phid = $row['badgePHID'];
 
-    foreach ($events as $event) {
+      $badge_phids[$badge_phid] = $badge_phid;
+      $badge_map[$recipient_phid][] = $badge_phid;
+    }
 
-      $author_awards = idx($awards, $event->getAuthorPHID(), array());
+    $badges = id(new PhabricatorBadgesQuery())
+      ->setViewer($viewer)
+      ->withPHIDs($badge_phids)
+      ->execute();
+    $badges = mpull($badges, null, 'getPHID');
 
-      $badges = array();
-      foreach ($author_awards as $award) {
-        $badge = $award->getBadge();
-        $badges[$award->getBadgePHID()] = $badge;
-      }
+    foreach ($events as $event) {
 
-      // TODO: Pick the "best" badges in some smart way. For now, just pick
-      // the first two.
-      $badges = array_slice($badges, 0, 2);
+      $author_awards = idx($badge_map, $event->getAuthorPHID(), array());
+      $author_badges = array_select_keys($badges, $author_awards);
 
-      foreach ($badges as $badge) {
+      foreach ($author_badges as $badge) {
         $badge_view = id(new PHUIBadgeMiniView())
           ->setIcon($badge->getIcon())
           ->setQuality($badge->getQuality())

...which produces queries which look like this:

SELECT * FROM (
  SELECT award.recipientPHID, award.badgePHID
    FROM `badges_award` award
    JOIN `badges_badge` badge ON award.badgePHID = badge.phid
    WHERE award.recipientPHID = 'PHID-USER-cvfydnwadpdj7vdon36z' AND badge.status IN ('open')
    ORDER BY award.id DESC LIMIT 2) awards_0
UNION ALL
SELECT * FROM (
  SELECT award.recipientPHID, award.badgePHID
    FROM `badges_award` award
    JOIN `badges_badge` badge ON award.badgePHID = badge.phid
    WHERE award.recipientPHID = 'PHID-USER-6otqc3truc7u43cy2i34' AND badge.status IN ('open')
    ORDER BY award.id DESC LIMIT 2) awards_1

However, I'm pretty sure that will hit the "maximum number of joins in a single query" limit at 32 or 64 users, and it's gross.

The way to do this with T9006 would be to create a new table, like PhabricatorBadgesDisplay, which would have columns like this:

• recipientPHID
• badgePHID
• awardPHID
• order

Then write to that table when a badge is awarded, revoked, disabled, enabled, or (in the future) a user reorders their badges. We only write the maximum number of rows that we'd ever use for display, and number them 1, 2, 3, 4, 5 in order.

Then the timeline query is:

• SELECT * FROM badge_display WHERE recipientPHID IN (%Ls) AND order <= 2

If we want to show 5 on hovercards, we:

• SELECT * FROM badge_display WHERE recipientPHID IN (%Ls) AND order <= 5

The logic to update that table -- especially for archiving/enabling a badge -- would be kind of a pain, but it's OK for it to be a bit messier/slower since it won't be happening all over the place, only when edits are made.

I'm also basically OK with just unbeta'ing without fixing this and dealing with it later -- it will take a while for any install to accumulate enough awards that this is an issue, and I think T9006 is a reasonable pathway forward when it does become one.

You don't want to use UserCache at all?

I'm okay with either approach.

The code in HEAD will eventually hit a scalability issue, but it's easy to understand and measure and well-contained. The user cache will scale better, but it's more complicated, and harder to measure and understand (the costs are spread out across every page, and some pages pay heavy costs to do big cache fills while others don't). If we had T9006, I'd prefer a dedicated table to the user cache, since I think that's the best of both worlds (scalable and self-contained).

If we do the user cache, we should fix the name/icon caching issue so the behavior of the cache is correct, though.

So up to you if you want to move forward and fix the cache behavior, or just punt this until T9006 happens. Or some version of that "UNION ALL" mess would probably work, but I'd basically rather just implement T9006 now than bring that upstream since it's so complicated and gross.

I can walk it off the Handle, but leave it on PhabricatorUser as a cache. This will let us not need weird award queries and we can swap in the new table when that's built.

or is having it on PhabricatorUser also a performance hit globally

Oh, that's a good compromise. UserCaches merely existing doesn't impact performance, so using UserCache but not Handles addresses my concerns.

maybe you should be CTO with that kind of thinking

applyFinalEffects still doesn't seem to kick in here, should it be in the editor itself?

I guess it should be moved up anyways, in case of name change, archiving, etc.

I am bad with arrays, so feel free to point out the right way to merge two and de-dupe keys.

I spent like 30 minutes trying to figure out why cache was failing and it was from me not using clearCaches instead of clearCache........ ....... :(

This gets award + name right but I think not award + revoke. I'd probably write it like this:

$user_phids = array();
$clear_everything = false;

foreach ($xactions as $xaction) {
  switch ($xaction->getTransactionType()) {
    case PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE:
    case PhabricatorBadgesBadgeRevokeTransaction::TRANSACTIONTYPE:
      foreach ($xaction->getNewValue() as $user_phid) {
        $user_phids[] = $user_phid;
      }
      break;
    default:
      $clear_everything = true;
      break;
  }
}

if ($clear_everything) {
  $awards = id(new PhabricatorBadgesAwardQuery())
    ->setViewer($this->getActor())
    ->withBadgePHIDs(array($badge_phid))
    ->execute();
  foreach ($awards as $award) {
    $user_phids[] = $award->getRecipientPHID();
  }
}

if ($user_phids) {
  PhabricatorUserCache::clearCaches(
    PhabricatorUserBadgesCacheType::KEY_BADGES,
    $user_phids);
}

While writing this, I moved the "default" case below the switch because I realized that we'll do multiple AwardQuery calls otherwise (for example, if you update name + icon + rarity, we load all awards three times if it's inside the loop).

In most cases, you don't need to go out of your way to make sure stuff is unique. Here, not bothering to make sure results are unique probably only ends up with a couple extra PHIDs in the eventual DELETE * FROM ... WHERE userPHID IN (%Ls) query, which isn't a big deal.

If you did want to make sure they're unique, you could replace the $user_phids[] = $some_user_phid; lines with $user_phids[$some_user_phid] = $some_user_phid;, or do $user_phids = array_unique($user_phids); at the end, but I usually don't bother with this since it's rarely important and makes the code a bit more complicated.

i still had to set badgePHID

and run through tests

You still have 20k awards in your test install?

I think I have 20K badges but only like 100 awards.

bin/lipsum generate badges.award

BTW the email not verified status thing is a little strange. What prompted the change? I assumed this dot was always for Calendar.

Prior to T12268, a grey dot meant either "disabled" (since D1429, in January 2012) or "unapproved" (since D7576, in November 2013).

I think the primary product role of the dot is to indicate "this user may not respond". All of the causes of the dot support this, with different levels of severity: they might not respond because their account has been disabled, or hasn't been approved yet, or they're away on vacation, or they're in a meeting. After the email verification stuff, they might not respond because they're not receiving email.

I think we see the grey dots far more than real installs do, since we have test accounts locally and a lot of inactive installs on this install.

Ah yeah, thanks for the color. I’ll think about a better UI treatment maybe.