← Back to all questions
StaffDeveloper ToolsDistributed Systems

Design a Code Review System

A full Staff-level walkthrough of a Critique-style code review system serving 100K+ developers and 50K changelists a day — rendering diffs for thousand-file CLs in under two seconds while keeping review state strongly consistent. It trades diff-compute cost against latency with a four-tier cache, resolves approvals from path-based OWNERS files, and anchors comments so they survive patchset uploads and rebases. Follow it top to bottom, or jump to any step.

Level
Staff
Category
Developer Tools · Distributed Systems
Interview time
60 min
100% free · No login required
WHAT THIS QUESTION TESTS
·Sub-two-second diff rendering for thousand-file CLs
·Strongly consistent review state — no lost comments or approvals
·Path-scoped OWNERS approval resolution
·Real-time comment fan-out to all CL viewers
★ STAFF-LEVEL SIGNALS
★Four-tier diff cache (in-memory → Redis → blob → VCS recompute) with content-addressed dedup
★Comment anchoring via patchset line maps plus surrounding-context hashes to survive rebases
★Affected-test selection over a build dependency graph instead of running the full suite
★Speculative pre-submit queue: integration tests run against an advancing HEAD before merge
1

Requirements

Functional Requirements

  • Create changelists (CLs): Developers submit code changes for review, including diffs against a base revision
  • Diff computation and display: Show side-by-side or unified diffs with syntax highlighting
  • Inline comments: Reviewers leave comments on specific lines, ranges, or files
  • Comment threads: Support threaded discussions with replies, resolution status
  • Review workflow: Request review, LGTM (approve), Request Changes, Submit (merge)
  • Multi-reviewer support: Multiple reviewers with different roles (owner, reviewer, CC)
  • Approval rules: Configurable per-directory/team (e.g., requires 2 LGTMs from owners)
  • CI/CD integration: Trigger automated builds/tests, display results on the CL
  • Search: Search CLs by author, reviewer, status, file path, date range, content
  • Code navigation: Click-through to definitions, references within the diff view
  • Review analytics: Time-to-review, review load per person, approval rates
  • Notifications: Email, in-app notifications for CL updates, comments, approvals

Non-Functional Requirements

  • Low latency: Diff rendering < 2 seconds even for large CLs (1000+ files)
  • High availability: 99.99% uptime (code review is on the critical path)
  • Scalability: Support 100K+ active developers, 50K+ CLs/day
  • Consistency: Strong consistency for review state (no lost comments or approvals)
  • Real-time updates: Collaborators see new comments within seconds (push-based)
  • Large diff support: Handle CLs with thousands of files and millions of lines

Constraints

  • Must integrate with existing VCS (Git, Piper/CitC at Google)
  • Must support monorepo (single repository with millions of files, billions of lines)
  • Diffs are against a snapshot, not necessarily HEAD (base may be stale)
  • Comment positions must survive rebases (line positions shift)
2

Back-of-envelope estimation

Users and Activity

  • Active developers: 100,000
  • CLs created per day: 50,000
  • Average reviews per CL: 3 (iterations/patchsets)
  • Comments per CL: 10
  • Average files per CL: 15
  • Average diff size per file: 100 lines changed

Traffic

  • Page views (CL view, diff view): 2 million/day
  • QPS for page loads: 2M / 86400 ~= 23 QPS average, peak ~100 QPS
  • Comment writes: 500K/day ~= 6 QPS average
  • CI status updates: 200K/day ~= 2 QPS
  • Search queries: 100K/day ~= 1 QPS
  • Notification events: 1M/day

Storage

  • Per CL metadata: ~5 KB (description, reviewers, status, timestamps)
  • Per comment: ~500 bytes (author, line, text, timestamp, thread)
  • Per diff (precomputed): ~50 KB average per file, 15 files = 750 KB per CL
  • Daily storage: 50K CLs (5 KB + 10 500B + 750 KB) = ~40 GB/day
  • Annual storage: ~15 TB
  • File content (stored in VCS, not duplicated): petabytes (monorepo)

Compute

  • Diff computation: Myers algorithm O(ND) where N = file length, D = edit distance
  • Average file: 500 lines, 50 changes -> ~25,000 operations
  • 50K CLs * 15 files = 750K diffs/day
  • Peak: ~30 diffs/second
  • Syntax highlighting: ~10ms per file (regex-based lexer)
3

API design

Changelist Management

POST /v1/changelists
Body: {
"title": "Fix null pointer in UserService",
"description": "The getUserById method could return null...",
"base_revision": "abc123",
"repository": "main",
"files": [
{"path": "src/UserService.java", "action": "MODIFY"},
{"path": "test/UserServiceTest.java", "action": "ADD"}
],
"reviewers": ["alice@", "bob@"],
"cc": ["team-backend@"],
"labels": ["bug", "P1"]
}
Response: { "cl_id": "cl/123456", "patchset": 1, "status": "PENDING_REVIEW" }
 
GET /v1/changelists/{cl_id}
Response: {
"cl_id": "cl/123456",
"title": "Fix null pointer in UserService",
"author": "charlie@",
"status": "APPROVED",
"patchsets": [
{"number": 1, "base": "abc123", "created_at": "...", "files": 2},
{"number": 2, "base": "abc123", "created_at": "...", "files": 2}
],
"reviewers": [
{"user": "alice@", "status": "LGTM", "patchset": 2},
{"user": "bob@", "status": "NOT_REVIEWED"}
],
"ci_status": {"build": "PASS", "tests": "PASS", "lint": "2 warnings"},
"comment_count": 7,
"unresolved_count": 1
}

Diff Retrieval

GET /v1/changelists/{cl_id}/patchsets/{ps}/diff
?file=src/UserService.java
&context_lines=3
&base_patchset=1 (optional: diff between patchsets)
&syntax_highlight=true
 
Response: {
"file_path": "src/UserService.java",
"base_revision": "abc123",
"diff_type": "MODIFY",
"hunks": [
{
"base_start": 42, "base_count": 7,
"new_start": 42, "new_count": 10,
"lines": [
{"type": "CONTEXT", "base_line": 42, "new_line": 42, "content": " public User getUserById(String id) {"},
{"type": "DELETE", "base_line": 43, "content": " return userMap.get(id);"},
{"type": "ADD", "new_line": 43, "content": " User user = userMap.get(id);"},
{"type": "ADD", "new_line": 44, "content": " if (user == null) {"},
{"type": "ADD", "new_line": 45, "content": " throw new NotFoundException(id);"},
{"type": "ADD", "new_line": 46, "content": " }"},
{"type": "ADD", "new_line": 47, "content": " return user;"},
{"type": "CONTEXT", "base_line": 44, "new_line": 48, "content": " }"}
],
"syntax_tokens": [...] // token ranges with highlighting classes
}
]
}

Comments

POST /v1/changelists/{cl_id}/comments
Body: {
"patchset": 2,
"file_path": "src/UserService.java",
"line_number": 44,
"side": "NEW", // NEW (right side) or BASE (left side)
"parent_comment_id": null, // null for new thread, or ID for reply
"content": "Should we also handle empty string IDs?",
"severity": "SUGGESTION" // BLOCKING, SUGGESTION, FYI
}
 
PUT /v1/changelists/{cl_id}/comments/{comment_id}/resolve
Body: { "resolved": true }
 
GET /v1/changelists/{cl_id}/comments
?patchset=2
&file_path=src/UserService.java
&include_resolved=false
Response: {
"threads": [
{
"thread_id": "t-789",
"file_path": "src/UserService.java",
"line_number": 44,
"side": "NEW",
"resolved": false,
"comments": [
{"id": "c-100", "author": "alice@", "content": "Should we...", "severity": "SUGGESTION", "created_at": "..."},
{"id": "c-101", "author": "charlie@", "content": "Good point, done in PS3", "created_at": "..."}
]
}
]
}

Review Actions

POST /v1/changelists/{cl_id}/reviews
Body: {
"action": "LGTM", // LGTM, REQUEST_CHANGES, COMMENT_ONLY
"patchset": 2,
"message": "Looks good, minor nit about error message."
}
 
POST /v1/changelists/{cl_id}/submit
Body: {
"merge_strategy": "SQUASH" // SQUASH, MERGE, REBASE
}
Response: {
"status": "SUBMITTED",
"commit_hash": "def456",
"submit_timestamp": "..."
}
// Returns 409 if: unresolved blocking comments, missing approvals, CI failures

Search

GET /v1/changelists/search
?query=author:charlie@ status:open reviewer:alice@ path:src/User*
&sort=updated_desc
&limit=50
 
Response: {
"total_count": 12,
"changelists": [
{"cl_id": "cl/123456", "title": "Fix null pointer...", "status": "APPROVED", ...},
...
]
}
4

Data model

Core Tables

Table: changelists
cl_id BIGSERIAL PRIMARY KEY
title VARCHAR(500)
description TEXT
author_id BIGINT REFERENCES users(user_id)
repository VARCHAR(255)
status ENUM('DRAFT', 'PENDING_REVIEW', 'APPROVED', 'SUBMITTED', 'ABANDONED')
created_at TIMESTAMP
updated_at TIMESTAMP
submitted_at TIMESTAMP NULL
submit_commit VARCHAR(64) NULL
labels TEXT[]
 
INDEX: (author_id, status, updated_at DESC)
INDEX: (status, updated_at DESC)
 
Table: patchsets
patchset_id BIGSERIAL PRIMARY KEY
cl_id BIGINT REFERENCES changelists
patchset_number INT
base_revision VARCHAR(64)
created_at TIMESTAMP
file_count INT
diff_stats JSONB -- {additions: 50, deletions: 20}
 
UNIQUE INDEX: (cl_id, patchset_number)
 
Table: patchset_files
file_id BIGSERIAL PRIMARY KEY
patchset_id BIGINT REFERENCES patchsets
file_path VARCHAR(4000)
action ENUM('ADD', 'MODIFY', 'DELETE', 'RENAME', 'COPY')
old_path VARCHAR(4000) NULL -- for renames
diff_blob_id VARCHAR(64) -- reference to precomputed diff
base_file_hash VARCHAR(64)
new_file_hash VARCHAR(64)
 
INDEX: (patchset_id, file_path)
 
Table: reviewers
reviewer_id BIGSERIAL PRIMARY KEY
cl_id BIGINT REFERENCES changelists
user_id BIGINT REFERENCES users
role ENUM('REVIEWER', 'CC', 'OWNER')
review_status ENUM('PENDING', 'LGTM', 'CHANGES_REQUESTED', 'COMMENTED')
reviewed_patchset INT NULL
updated_at TIMESTAMP
 
UNIQUE INDEX: (cl_id, user_id)
 
Table: comments
comment_id BIGSERIAL PRIMARY KEY
cl_id BIGINT REFERENCES changelists
thread_id BIGINT -- groups comments in a thread
parent_id BIGINT NULL -- NULL for root comment
patchset_number INT
file_path VARCHAR(4000) NULL -- NULL for CL-level comments
line_number INT NULL
side ENUM('BASE', 'NEW') NULL
author_id BIGINT REFERENCES users
content TEXT
severity ENUM('BLOCKING', 'SUGGESTION', 'FYI')
resolved BOOLEAN DEFAULT FALSE
resolved_by BIGINT NULL
created_at TIMESTAMP
updated_at TIMESTAMP
 
INDEX: (cl_id, patchset_number, file_path)
INDEX: (thread_id, created_at)
 
Table: ci_results
result_id BIGSERIAL PRIMARY KEY
cl_id BIGINT REFERENCES changelists
patchset_number INT
check_name VARCHAR(255) -- "build", "unit_tests", "lint"
status ENUM('PENDING', 'RUNNING', 'PASS', 'FAIL', 'ERROR')
details_url VARCHAR(2000)
started_at TIMESTAMP
completed_at TIMESTAMP NULL
summary TEXT NULL
 
INDEX: (cl_id, patchset_number, check_name)
 
Table: approval_rules
rule_id BIGSERIAL PRIMARY KEY
path_pattern VARCHAR(4000) -- glob pattern, e.g., "src/security/**"
repository VARCHAR(255)
required_approvers INT -- minimum LGTMs needed
required_groups TEXT[] -- must include LGTM from these groups
auto_reviewers TEXT[] -- automatically added as reviewers

Diff Storage

Precomputed diffs stored in blob storage (GCS/S3):
 
DiffBlob {
blob_id: SHA256 of (base_hash + new_hash + algorithm_version)
format: BINARY_DIFF_V2
content: compressed diff with line mappings
 
// Line mapping enables comment position tracking across patchsets
line_map: {
base_to_new: [(42, 42), (43, null), (null, 43), (null, 44), ...],
new_to_base: [(42, 42), (43, null), (44, null), ...]
}
}
 
Storage: ~50 KB per file diff, stored in content-addressed blob store.
Deduplication: Same diff (same base_hash + new_hash) is computed once.

Storage Choice Rationale

Component
Storage
Reasoning
CL metadata, comments
PostgreSQL
Relational integrity, transactions for review state
Precomputed diffs
GCS/S3 blob storage
Large binary blobs, content-addressed
File content
VCS (Git/Piper)
Already exists, don't duplicate
Search index
Elasticsearch
Full-text search on CL descriptions, comments, file paths
Notification queue
Kafka/Pub-Sub
Reliable event delivery for notifications
Real-time updates
Redis Pub/Sub or WebSocket server
Low-latency push to connected clients
Analytics
BigQuery/ClickHouse
OLAP for review metrics
5

High-level architecture

Developer's Browser/IDE
|
+----------------------------+
| Load Balancer |
+----------------------------+
| |
+----------+ +----------+
v v
+-------------------+ +-------------------+
| Web Frontend | | API Gateway |
| - React/Angular | | - Auth |
| - Diff renderer | | - Rate limiting |
| - Comment UI | | - Routing |
| - WebSocket client| +-------------------+
+-------------------+ |
| +------------+------------+
v v v v
+-------------------+ +-----------+ +-----------+ +-----------+
| WebSocket Server | | CL Service| | Diff | | Search |
| - Real-time | | - CRUD | | Service | | Service |
| updates | | - Workflow| | - Compute | | - Index |
| - Presence | | - Reviews | | - Cache | | - Query |
+-------------------+ +-----------+ | - Render | +-----------+
| | +-----------+ |
v v | v
+-------------------+ +-----------+ v +-----------+
| Redis Pub/Sub | | PostgreSQL| +-----------+ | Elastic- |
+-------------------+ +-----------+ | Blob Store| | search |
| +-----------+ +-----------+
v
+-------------------+
| Notification |
| Service |
| - Email |
| - In-app |
| - IDE plugin push |
+-------------------+
|
+-------------------+
| Event Bus (Kafka) |
+-------------------+
| |
+----------+ +----------+
v v
+-------------------+ +-------------------+
| CI/CD Integration | | Analytics Service |
| - Trigger builds | | - Review metrics |
| - Report results | | - Dashboard |
+-------------------+ +-------------------+

Request Flow: Viewing a CL Diff

1. Developer navigates to /cl/123456
2. Frontend requests CL metadata: GET /v1/changelists/123456
3. CL Service fetches from PostgreSQL, returns metadata + reviewer status
4. Frontend requests file list: GET /v1/changelists/123456/patchsets/2/files
5. Developer clicks a file to view diff
6. Frontend requests diff: GET /v1/.../diff?file=UserService.java
7. Diff Service:
a. Check diff cache (Redis): HIT -> return immediately
b. MISS: Fetch base file content from VCS (at base_revision)
c. Fetch new file content from VCS (at patchset revision)
d. Compute diff using Myers algorithm
e. Apply syntax highlighting
f. Cache result, return to frontend
8. Frontend requests comments for this file
9. Render diff with inline comments
10. Establish WebSocket for real-time updates

Request Flow: Submitting (Merging) a CL

1. Developer clicks "Submit"
2. API Gateway -> CL Service: POST /v1/changelists/123456/submit
3. CL Service runs submit checks:
a. All blocking comments resolved? (query comments table)
b. Required approvals met? (check approval_rules for each file path)
c. CI checks passed? (query ci_results)
d. No merge conflicts? (check base is compatible with HEAD)
4. If all pass:
a. Begin transaction
b. Merge changes into VCS (git merge/rebase/squash)
c. Update CL status to SUBMITTED
d. Record submit_commit hash
e. Commit transaction
5. Publish event to Kafka: CL_SUBMITTED
6. Notification Service sends emails to all participants
7. Analytics Service records submission metrics
6

Deep dives

WHERE STAFF IS WON

Deep Dive 1: Diff Computation and Large CL Handling

Problem: Computing and rendering diffs must be fast even for large changelists (1000+ files, files with thousands of lines).

Diff Algorithm Selection:

Algorithm | Time | Space | Quality | Use Case
Myers (default) | O(ND) | O(N) | Optimal | Most files
Patience | O(N log N) | O(N) | Better | Structured code
Histogram | O(N) | O(N) | Good | Large files
Delta (binary) | O(N) | O(N) | N/A | Binary files
 
Where N = file length, D = edit distance.
 
Myers algorithm: Find the shortest edit script (SES).
- For small diffs (D << N): effectively O(N), very fast
- For dissimilar files (D ~ N): O(N^2), slow
- Fallback: if Myers exceeds 10ms timeout, switch to line-hash approach
 
Patience diff enhancement:
- First: match unique common lines between files
- Then: run Myers on segments between matched anchors
- Result: diffs that align on function boundaries, not arbitrary lines
- Better for code (function signatures are unique anchors)

Large CL Optimization:

Problem: CL with 2000 files. Computing all diffs at once = 2000 * O(ND) = slow.
 
Strategy: Lazy diff computation with progressive loading
 
1. On CL creation:
- Compute diff STATS only (additions/deletions count) for all files
- This is cheap: just count lines in each file
- Total time: < 1 second for 2000 files
 
2. On file view:
- Compute full diff for requested file only
- Cache the result (content-addressed by base_hash + new_hash)
- If cache hit (same file changed in another CL): instant
 
3. Pre-warming:
- Background job computes diffs for "likely to be viewed" files
- Heuristic: files with most changes, test files, files in title/description
- Pre-warm top 50 files within 30 seconds of CL creation
 
4. Client-side rendering:
- For very large files (>10K lines): virtual scrolling
- Only render visible viewport + buffer zone
- Diff data is streamed as user scrolls
 
Diff caching hierarchy:
L1: In-memory (per-server LRU, 2 GB) -> < 1ms
L2: Redis cluster (shared, 100 GB) -> 1-5ms
L3: Blob storage (persistent, unlimited) -> 10-50ms
L4: Recompute from VCS -> 50-500ms

Comment Position Tracking Across Patchsets:

Problem: Developer comments on line 44 of Patchset 1.
Author uploads Patchset 2 with changes above line 44.
Where does the comment appear in Patchset 2?
 
Solution: Line mapping via diff chain
 
Patchset 1 (base -> PS1):
base line 42 -> PS1 line 42
base line 43 -> PS1 line 43 (deleted in PS1)
(new) -> PS1 line 44 (added in PS1) <-- comment here
base line 44 -> PS1 line 48
 
Patchset 2 (base -> PS2):
base line 42 -> PS2 line 42
base line 43 -> PS2 line 43 (deleted in PS2)
(new) -> PS2 line 44 (same addition as PS1)
(new) -> PS2 line 45 (new line added in PS2)
base line 44 -> PS2 line 49
 
To map PS1:44 -> PS2:
1. Map PS1:44 to the base context: "added line after base:43"
2. Map base context to PS2: "added line after base:43" = PS2:44
3. If the same added line exists in PS2:44: map directly
4. If the line was modified: show comment with "outdated" indicator
5. If the line was deleted: show comment as "orphaned"
 
Implementation: Store comments with both line number AND
content hash of the surrounding 3 lines (anchor context).
Use content matching as fallback when line mapping is ambiguous.

Deep Dive 2: Approval Workflow and OWNERS System

Problem: In a monorepo, different parts of the codebase have different owners. A CL touching files in multiple directories may need approval from multiple owners.

OWNERS File System:

Repository structure:
/
/OWNERS <- root owners
/src/
/src/OWNERS <- src owners
/src/security/
/src/security/OWNERS <- security team owners
/src/security/crypto/
/src/security/crypto/OWNERS
 
OWNERS file format:
# /src/security/OWNERS
set noparent # don't inherit from parent OWNERS
alice@google.com
bob@google.com
per-file: *.proto = charlie@google.com # proto file specialist
per-file: BUILD = infra-team@google.com # build config specialist
 
Inheritance rules:
1. By default, owners are inherited from parent directories
2. "set noparent" breaks inheritance
3. "per-file" rules override for matching files
4. "*" means anyone can approve (public directory)

Approval Resolution Algorithm:

def check_approval(cl):
"""Returns whether a CL has sufficient approvals to submit."""
 
# Collect all files in the CL
files = cl.get_changed_files()
 
# For each file, determine required approvers
file_approvals = {}
for file_path in files:
owners = resolve_owners(file_path)
file_approvals[file_path] = {
"required_owners": owners,
"approved_by": set()
}
 
# Check which reviewers have given LGTM
for reviewer in cl.reviewers:
if reviewer.status == "LGTM":
for file_path in files:
if reviewer.user_id in resolve_owners(file_path):
file_approvals[file_path]["approved_by"].add(reviewer.user_id)
 
# Verify each file has at least one owner approval
unapproved_files = []
for file_path, approval in file_approvals.items():
if not approval["approved_by"]:
unapproved_files.append(file_path)
 
return {
"approved": len(unapproved_files) == 0,
"unapproved_files": unapproved_files,
"suggested_reviewers": suggest_reviewers(unapproved_files)
}
 
def resolve_owners(file_path):
"""Walk up directory tree to find OWNERS."""
owners = set()
path = file_path
while path != "/":
dir_path = dirname(path)
owners_file = dir_path + "/OWNERS"
if exists(owners_file):
parsed = parse_owners(owners_file, basename(file_path))
owners.update(parsed.owners)
if parsed.no_parent:
break
path = dir_path
return owners
 
def suggest_reviewers(unapproved_files):
"""Suggest reviewers who can approve the most unapproved files."""
# Greedy set cover: pick the owner who covers the most unapproved files
reviewer_coverage = defaultdict(set)
for file_path in unapproved_files:
for owner in resolve_owners(file_path):
reviewer_coverage[owner].add(file_path)
 
suggested = []
remaining = set(unapproved_files)
while remaining:
best = max(reviewer_coverage, key=lambda o: len(reviewer_coverage[o] & remaining))
suggested.append(best)
remaining -= reviewer_coverage[best]
 
return suggested

Approval Caching:

Computing OWNERS for every file on every CL view is expensive.
Cache strategy:
1. Parse OWNERS files once per repository revision
2. Build a trie of owner assignments
3. Cache the trie in memory (invalidate on OWNERS file changes)
4. Lookup for any file path: O(path_depth), typically < 10

Deep Dive 3: CI/CD Integration and Presubmit Checks

Problem: Every CL must pass automated checks before submission. These checks range from seconds (lint) to hours (full integration tests). How do we design for both speed and thoroughness?

Check Categories and Timing:

Category | Trigger | Duration | Blocking?
Lint/Format | On upload | 5-30 sec | Yes (auto-fixable)
Build | On upload | 1-10 min | Yes
Unit tests | On upload | 2-30 min | Yes
Integration tests | On LGTM | 10-60 min | Yes (for submit)
Load tests | Manual | 1-4 hours | No (advisory)
Security scan | On upload | 5-15 min | Yes (if critical)
 
Strategy: Run cheap checks eagerly, expensive checks lazily.

CI Integration Architecture:

CL Upload Event
|
v
+---------------+
| Event Router |
+---------------+
/ | \
+---------+ +---------+ +-----------+
| Lint | | Build | | Security |
| Worker | | Worker | | Scanner |
+---------+ +---------+ +-----------+
| | |
v v v
+----------------------------------------+
| CI Results Aggregator |
| - Collects results from all checks |
| - Updates ci_results table |
| - Publishes status to CL |
+----------------------------------------+
|
v
+-------------------+ +-------------------+
| PostgreSQL | | WebSocket Server |
| (ci_results) | | (push to browser) |
+-------------------+ +-------------------+
 
Check lifecycle:
PENDING -> RUNNING -> PASS/FAIL/ERROR
 
Retry logic:
- Flaky test detected (same test, passed in prior run): auto-retry once
- Infrastructure failure: auto-retry with exponential backoff
- Genuine failure: report to developer with log excerpts

Incremental Checks (critical for developer productivity):

Problem: Running all tests on every patchset upload is wasteful.
Most changes only affect a small subset of tests.
 
Solution: Affected test analysis
 
1. Maintain a dependency graph: file -> build targets -> test targets
Example: UserService.java -> //src:user_lib -> //test:user_test
 
2. On CL upload:
a. Identify changed files
b. Traverse dependency graph to find affected test targets
c. Run ONLY affected tests
 
3. Dependency graph construction:
- Built from BUILD/Makefile/package.json dependency declarations
- Updated incrementally as build files change
- Cached in memory (for monorepo: can be large, ~1 GB for Google-scale)
 
Changed files: [UserService.java]
|
v (dependency graph)
Affected build targets: [//src:user_lib]
|
v (reverse dependency)
Affected test targets: [//test:user_test, //test:integration:auth_test]
|
v (skip: //test:unrelated_test -- not in affected set)
 
Result: Instead of running 100K tests, run 50 affected tests.
Time: 30 minutes -> 2 minutes

Pre-submit Queue (for submit-time checks):

Problem: Integration tests take 30+ minutes. Running them synchronously
at submit time would block the developer.
 
Solution: Submit queue with speculative execution
 
1. Developer clicks "Submit"
2. CL enters submit queue with status "QUEUED"
3. System speculatively runs integration tests
4. If tests pass: auto-merge and notify developer
5. If tests fail: reject, notify developer, CL returns to review state
 
Optimization: Speculative execution
- When CL receives LGTM, start integration tests immediately
(before developer clicks submit)
- By the time developer submits, tests may already be done
 
Merge conflict handling:
- Submit queue processes CLs in order
- CL N is tested against HEAD
- If CL N passes, it merges. HEAD advances.
- CL N+1 is now tested against new HEAD (includes CL N)
- If CL N+1 conflicts: developer notified, must rebase
7

Bottlenecks and Tradeoffs

Bottleneck 1: Monorepo Scale

  • Problem: In a monorepo with billions of lines, even listing files in a directory can be slow.
  • Solution:
  • Virtual filesystem (e.g., FUSE mount) that lazily fetches file content
  • File-level caching with content-addressed storage
  • Diff computation works on file hashes first; identical files (same hash) = no diff
  • Sparse checkout: developer's workspace only contains relevant files

Bottleneck 2: Real-Time Collaboration

  • Problem: Multiple reviewers commenting simultaneously; they should see each other's comments in real-time.
  • Solution:
  • WebSocket connections from all viewers of a CL
  • On comment post: write to DB, publish to Redis Pub/Sub channel for that CL
  • All connected clients receive the update within ~100ms
  • Conflict resolution: comments are append-only, no conflicts possible
  • Presence: show who is currently viewing the CL (via WebSocket heartbeat)

Bottleneck 3: Search Performance

  • Problem: Searching across millions of CLs by content, author, file path, date range.
  • Solution:
  • Elasticsearch index with fields: title, description, author, reviewer, file_paths, status, labels, date
  • Full-text search on CL descriptions and comments
  • Code search (within diffs) is more expensive: index only file paths and metadata, not diff content
  • For code content search: integrate with separate code search system (like Google Code Search)
Search index schema:
{
"cl_id": keyword,
"title": text (analyzed),
"description": text (analyzed),
"author": keyword,
"reviewers": keyword[],
"file_paths": keyword[] (path hierarchy tokenizer),
"status": keyword,
"labels": keyword[],
"created_at": date,
"updated_at": date,
"comment_text": text (analyzed),
"ci_status": keyword
}

Bottleneck 4: Notification Fatigue

  • Problem: Active developers receive hundreds of notifications per day, leading to alert fatigue.
  • Solution:
  • Intelligent batching: group notifications within 5-minute windows
  • Priority levels: blocking comments = immediate; FYI comments = batched
  • Muting: allow muting CLs, specific threads, or all-but-my-CLs
  • Smart digest: daily summary of pending reviews, ordered by urgency
  • ML-based: predict which CLs need attention based on staleness, author patterns

Bottleneck 5: Stale Diffs After Rebase

  • Problem: Developer rebases CL onto latest HEAD. All patchset diffs must be recomputed against the new base.
  • Solution:
  • Don't recompute diffs for old patchsets (they're historical)
  • Only compute diff for latest patchset against new base
  • Show "interdiff" between patchsets (PS1 vs PS2) regardless of base changes
  • This preserves review context: reviewer can see "what changed since my last review"

Monitoring and Observability

Key Metrics:
- Diff computation latency: p50, p95, p99 (target: p95 < 1s)
- CL page load time: (target: p50 < 500ms)
- Time-to-first-review: median time from CL creation to first comment
(organizational metric, target varies)
- Review turnaround: time from review request to LGTM
- Submit success rate: % of submit attempts that succeed first try
- CI check duration: per check type
- WebSocket connection count: concurrent viewers
- Comment creation rate: QPS
- Search query latency: (target: p95 < 500ms)
 
Alerting:
- Diff computation timeout rate > 1% -> check VCS performance
- CI queue depth > 100 -> scale CI workers
- Submit failure rate > 5% -> investigate merge conflicts, CI flakiness
- WebSocket disconnection rate > 10% -> check server health
- Search latency p95 > 2s -> reindex or scale Elasticsearch

Key Design Decisions Summary

Decision
Choice
Reasoning
Diff algorithm
Myers with Patience fallback
Optimal diffs for code, with fallback for large files
Diff storage
Content-addressed blob store
Deduplication, immutable, cacheable
Comment positioning
Line mapping + content anchor
Survives rebases and patchset updates
Approval model
OWNERS files with path-based rules
Scalable code ownership in monorepo
Real-time updates
WebSocket + Redis Pub/Sub
Low-latency push without polling
CI integration
Event-driven with affected test analysis
Fast feedback, minimal wasted compute
Search
Elasticsearch
Full-text search with structured filtering
Storage
PostgreSQL for metadata, blob store for diffs
Transactional consistency for review state
★

Rubric — Senior vs Staff

Dimension
Senior signal
Staff signal
Diff handling
Computes per-file diffs and caches the result.
Lazy plus pre-warmed diffs, a four-tier content-addressed cache, and virtual scrolling for huge files.
Approvals
Requires N LGTMs before a CL can submit.
Resolves path-scoped OWNERS with inheritance and per-file rules; greedy reviewer suggestion via set cover.
Comment durability
Stores comments keyed by file and line number.
Maps positions across patchsets, falls back to context-hash anchors, flags outdated and orphaned threads.
CI integration
Runs checks and blocks submit on failure.
Tiers cheap vs. expensive checks, selects affected tests, and uses a speculative pre-submit queue.
Consistency model
Keeps review state in a relational database.
Transactional submit gate, append-only comments for conflict-free real-time, deliberate blob-vs-SQL split.

Related questions

★ MORE WALKTHROUGHS

Want more breakdowns like this?

Join free early access for upcoming RAG, LLM eval, agents, and AI infrastructure walkthroughs.

Join Free Early Access →