The pull request was titled "Add People You May Know widget". It had one new endpoint, one SQL query, and a screenshot of ten smiling suggestions from the staging database. The tests passed. The demo looked great.
I sent it back anyway, with six comments. None of them said the query was wrong. It returns exactly the right people for a user with 40 friends. The problem is everyone else, and everything that happens after the first page load.
Here's the review, comment by comment, with the shape of what I asked for in .NET 10. The code is trimmed on purpose. Where a part is cut, it says so.
Tech Skill Builder is $5/month for a very limited time only (normally $10). Grab it now before it goes back to $10. The complete, working, tested project behind this review (
facebook-people-you-may-know-dotnet, Clean Architecture, 164 tests, plus an 87-page PDF) is inside. Join Tech Skill Builder
The PR as submitted
-- GET /users/{me}/suggestions SELECT f2.friend_id, COUNT(*) AS mutual FROM friendships f1 JOIN friendships f2 ON f2.user_id = f1.friend_id WHERE f1.user_id = @me AND f2.friend_id <> @me AND f2.friend_id NOT IN (SELECT friend_id FROM friendships WHERE user_id = @me) GROUP BY f2.friend_id ORDER BY mutual DESC LIMIT 10; Friends of my friends, minus me, minus people I already know, ordered by mutual count. It's the textbook answer, and that's exactly why it needs a careful review.
Comment 1: "This runs on every page view. How many rows does it touch?"
For a user with 200 friends who each have about 200 friends, the join walks around 40,000 adjacency rows per request. Someone with 3,000 friends turns that into millions. And the widget is something people scroll past half the time.
Some napkin math (my own assumptions for a big hypothetical network, not anyone's real figures): 100M daily users and ~200 friends each puts a full two-hop pass at around 4 × 10¹² row reads. Storing the result is cheap, though. Fifty suggestions at ~40 bytes is ~2 KB per user, ~200 GB for 100M users. That's a cache.
Ask: compute offline or nearline, store the top 50 per user, serve from cache. The request path should only read a list.
public sealed record PymkSettings { public int ServeTopK { get; init; } = 20; // returned per request public int StoreTopK { get; init; } = 50; // kept in cache, headroom for filtering public TimeSpan StaleAfter { get; init; } = TimeSpan.FromHours(24); // batch vs online fan-out limits, fatigue, hub threshold... // ... full settings in the complete project } Comment 2: "What happens when one of my friends has 4,000 friends?"
The query treats every mutual friend the same. But a mutual friend with 40 friends tells you a lot. One with 4,000 tells you almost nothing, and expanding them floods the candidate set with weak matches.
Facebook's own research backs this up. In Supervised Random Walks (Backstrom & Leskovec, WSDM 2011), roughly 92% of new friendships closed a triangle, and candidates with very few mutual friends almost never converted. So two hops is the right reach, as long as it's capped.
Ask: bound the walk, skip hubs, and weight mutual friends by how selective they are (Adamic-Adar: sum of 1/ln(degree)).
public sealed record FanOutLimits(int MaxFriendsExpanded = 5000, int HubDegreeCap = 5000, int MinMutualFriends = 1); public void AddMutual(UserId via, int viaDegree) { MutualFriends++; AdamicAdar += 1.0 / Math.Log(Math.Max(2, viaDegree)); // keep a small sample of mutual friends for the "3 mutual friends" label } // The walk itself: one multi-get of friend lists grouped by shard, // skip friends above HubDegreeCap, stop expanding at MaxFriendsExpanded. // ... full two-hop counter + shard-aware multi-get in the complete project One multi-get per shard instead of one call per friend matters more than any micro-optimisation inside the loop.
Comment 3: "Where do blocked users go?"
The PR's answer was "we'll add AND friend_id NOT IN (blocks) later". The tempting shortcut, which I've seen in real code, is to keep them in the ranking with a score of −1000. Then one day someone changes a weight and a blocked person floats back onto a screen.
Blocking also goes both ways. If I block you, I shouldn't see you, and you shouldn't see me either.
Ask: eligibility is a separate policy from ranking, and it returns a reason.
public enum ExclusionReason { None, Self, AlreadyFriends, Blocked, Dismissed, PendingRequest, Declined, PrivacyFriendsOfFriendsOnly, BelowMinMutual, IntegrityFiltered, Fatigued, UnknownUser, } // ExclusionPolicy.Evaluate(viewer, candidate, features, viewerFriends, exclusions, ...) // checks run in a fixed order and return the first reason that applies. // ... the full ordered rule list in the complete project The enum earns its keep the first time support asks why someone didn't show up.
Comment 4: "The cached list is from last night. I blocked someone 30 seconds ago."
Once you accept comment 1 and precompute, you get a new bug: the list is stale. Most staleness doesn't matter. Nobody notices a six-hour-old suggestion. But a block, a dismissal or a pending request has to apply right away.
Ask: run the same exclusion policy again on the cached list at read time, with fresh exclusions. Serve stale lists, refresh them in the background.
var list = await cache.GetAsync(viewer, ct); if (list is null) { // cache miss: compute online with tighter limits than the batch job, // store it, queue a full recompute if corners were cut } else if (list.IsOlderThan(settings.StaleAfter, clock.GetUtcNow())) { await queue.EnqueueAsync(new RecomputeRequest(viewer, RecomputeReason.Stale), ct); } // live filter with fresh exclusions, then take ServeTopK // ... LiveFilterAsync and the online compute path in the complete project The queue de-duplicates per user, so ten events in a minute cause one recompute instead of ten.
Comment 5: "Mutual count says who you might add. Who's going to accept?"
ORDER BY mutual DESC only looks at the viewer. Meta's transparency card for this feature describes predicting both sides: how likely you are to send a request and how likely the other person is to accept. A suggestion nobody accepts is just spam with good manners.
Ask: a scorer with signals from both sides, and an explanation built from the same numbers.
public sealed record RankingWeights( double MutualLog = 1.0, double AdamicAdar = 0.6, double MutualRatio = 2.0, double SharedGroupLog = 0.5, double ContactUpload = 0.8, double AcceptPrior = 0.7, double PendingPenaltyLog = 0.4, double FatiguePerImpression = 0.35); // SuggestionScorer.Explain(...) returns a per-term breakdown; // Score(...) is just its Total. GET /users/{id}/suggestions/{candidateId}/why returns the breakdown. // ... scorer, ranker and the explain endpoint in the complete project A linear model is a deliberate choice here, not the final word. Swap in a trained model later if you like. Just keep one invariant: the "why" total has to equal the ranked score. If the explanation drifts from the ranking, it's worse than having no explanation at all.
Comment 6: "What happens to everyone else's list when two people become friends?"
This was the comment that turned a 20-line PR into a design discussion.
When A and B become friends, every friend of A just gained B as a friend-of-friend. Their cached lists are now missing someone. The read-time filter from comment 4 can't help, because it only removes people and can never add them.
For a user with 150 friends, patching 150 cached lists is fine. For a user with 5,000 friends who's accepting requests all afternoon, it's a write storm.
Ask: put the decision in a small, pure, testable planner. Don't scatter it across a handler.
public enum DeltaStrategy { PatchFriends, LazyEpochBump } public static IReadOnlyList<EndpointPlan> Plan(UserId a, int degreeA, UserId b, int degreeB, int hubThreshold) { // one plan per endpoint, chosen by that endpoint's degree vs the threshold // ... planner, patcher and the hub-epoch refresh in the complete project } Where exactly that threshold should sit, and what a reader of a "lazy" list sees in the meantime, is the interesting part. I left it as an open question on the PR.
What I approved in the end
The second version of the PR wasn't really "a query" any more:
- Two-hop candidates computed offline/nearline, capped, hubs skipped
- Group co-members and contact uploads as extra, flagged sources
- An exclusion policy with reasons, applied at compute time and at read time
- Symmetric blocks, permanent dismissals, declines, pending requests, fatigue
- A both-sides scorer with a
/whythat matches the score - Cache with a stale-while-refresh read path and a de-duplicating recompute queue
- Feedback (impressions, dismissals, requests) flowing back into the signals
- Tests for every exclusion reason, plus block symmetry over HTTP
The SQL from the original PR still exists, in spirit. It's step one of a five-step pipeline: generate, filter, rank, serve, learn.
Your turn: a user with 4,999 friends accepts one more request. Would you patch thousands of cached lists right away, wait for tonight's batch, or leave them stale? Where would you draw the line? Tell me in the comments.
Get the complete project
The full, working, tested .NET 10 solution (facebook-people-you-may-know-dotnet) is in Tech Skill Builder. It has separate Domain, Application, Infrastructure and Api projects, a sharded in-memory graph store with shard-aware multi-gets, the exclusion policy with every reason, the both-sides scorer and /why endpoint, the suggestion cache, a de-duplicating recompute queue, background workers, the edge-delta planner and patcher, and 164 passing tests (144 unit + 20 integration). It also comes with an 87-page PDF that goes much deeper than this post: how Meta describes the feature, architecture and request-flow diagrams, the design trade-offs, a code walkthrough mapped to each box, and the full answer to the question above.
Tech Skill Builder is only $5/month for a very limited time (normally $10). Grab it now before it goes back to $10.
Join Tech Skill Builder