Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 35 additions & 33 deletions SOLUTION.md
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
# Stage 4B Solution
# Stage 4B - Solution

## Overview

This document covers the three areas of improvement implemented in Stage 4B: query performance and database efficiency, query normalization and cache efficiency, and large-scale CSV data ingestion.

---

## Part 1 Query Performance
## Part 1 - Query Performance

### What was done

**Database indexes**

Added indexes on all columns used in WHERE clauses and ORDER BY clauses on the `classifications` table:

- `gender` equality filter
- `age_group` equality filter
- `age` range queries (`min_age`, `max_age`)
- `country_id`equality filter (plain index, not functional see query fix below)
- `gender_probability` range filter
- `country_probability` range filter
- `created_at` sort column
- `LOWER(name)` functional index for case-insensitive duplicate detection
- `gender`: equality filter
- `age_group`: equality filter
- `age`: range queries (`min_age`, `max_age`)
- `country_id`: equality filter (plain index, not functional - see query fix below)
- `gender_probability`: range filter
- `country_probability`: range filter
- `created_at`: sort column
- `LOWER(name)`: functional index for case-insensitive duplicate detection

Without indexes, every query on a table of millions of rows performs a full sequential scan. With indexes, the database jumps directly to matching rows.

Expand All @@ -47,15 +47,15 @@ Planning Time: 0.171 ms
Execution Time: 339.266 ms
```

The index is confirmed in use. The improvement on a single low-cardinality column (`gender` has only two values, ~50% selectivity) is 402ms to 339ms modest because the planner still touches most of the table. The index benefit is more pronounced on high-selectivity queries: filtering by `country_id` (one of ~20 countries, ~5% selectivity) or combining multiple filters reduces the scanned row count significantly. Planning time also dropped from 42ms to 0.17ms.
The index is confirmed in use. The improvement on a single low-cardinality column (`gender` has only two values, ~50% selectivity) is 402ms to 339ms - modest because the planner still touches most of the table. The index benefit is more pronounced on high-selectivity queries: filtering by `country_id` (one of ~20 countries, ~5% selectivity) or combining multiple filters reduces the scanned row count significantly. Planning time also dropped from 42ms to 0.17ms.

**Connection pooling**

Configured `pg.Pool` with explicit settings:

- `max: 20` limits concurrent database connections
- `idleTimeoutMillis: 30000` proactively closes idle connections before Neon terminates them
- `connectionTimeoutMillis: 5000` fails fast if no connection is available rather than hanging
- `max: 20`: limits concurrent database connections
- `idleTimeoutMillis: 30000`: proactively closes idle connections before Neon terminates them
- `connectionTimeoutMillis: 5000`: fails fast if no connection is available rather than hanging

**Primary / replica split**

Expand All @@ -66,7 +66,7 @@ Session-related reads (`getSessionByTokenHash`, `getUserById`) remain on the pri
**Query restructuring**

- Removed `COUNT(*) OVER()` window function from `getAllRecords`. This computed the total count across all matching rows before applying LIMIT, requiring a full scan on every paginated request. Replaced with two parallel queries: one for the page of data, one for the count. Both run simultaneously via `Promise.all`.
- Fixed `LOWER(country_id)` in WHERE clause this prevented the `country_id` index from being used. Changed to store and compare uppercase values consistently.
- Fixed `LOWER(country_id)` in WHERE clause - this prevented the `country_id` index from being used. Changed to store and compare uppercase values consistently.
- Fixed parameterized query placeholders that were missing the dollar sign prefix, causing filters to be silently ignored.
- Rewrote `insertRecord` to use `ON CONFLICT (name) DO UPDATE SET id = classifications.id RETURNING *, (xmax = 0) AS inserted`. This eliminates a second round-trip to fetch the existing record on duplicate inserts. The `xmax = 0` trick detects whether the row was inserted or was a conflict in a single query.

Expand All @@ -80,16 +80,16 @@ Measurements taken against the production Neon database with 500,000+ profiles o

| Operation | Before (no indexes, no cache) | After (indexes + cache) | Change |
|---|---|---|---|
| `GET /api/profiles` (no filters, cache miss) | ~1000ms | ~1000ms | baseline network-bound |
| `GET /api/profiles` (no filters, cache miss) | ~1000ms | ~1000ms | baseline - network-bound |
| `GET /api/profiles` (cache hit) | ~1000ms | ~123ms | ~88% faster |
| `GET /api/profiles?gender=male` (cache miss) | ~1000ms+ | ~800ms | indexes reduce scan cost |
| `GET /api/profiles?gender=male` (cache hit) | ~1000ms+ | ~123ms | ~88% faster |

The uncached read time of ~1 second reflects the remote database round-trip to Neon plus query execution on 500k rows. The cache hit time of ~123ms reflects the Upstash Redis round-trip only no database query. The P95 target of 2 seconds is met on both paths. The P50 target of 500ms is met on cache hits.
The uncached read time of ~1 second reflects the remote database round-trip to Neon plus query execution on 500k rows. The cache hit time of ~123ms reflects the Upstash Redis round-trip only - no database query. The P95 target of 2 seconds is met on both paths. The P50 target of 500ms is met on cache hits.

---

## Part 2 Query Normalization
## Part 2 - Query Normalization

### What was done

Expand All @@ -116,13 +116,13 @@ The key order is fixed by an explicit `keyOrder` array, not by insertion order.

The current implementation uses TTL-only invalidation. When a profile is created via `POST /api/profiles`, cached query results are not immediately invalidated. A user who creates a profile and immediately queries may not see it for up to 60 seconds.

This is a deliberate trade-off. Active cache invalidation on write would require tracking which cache keys are affected by a given insert non-trivial given the number of possible filter combinations. For this system, where profile creation is an admin-only batch operation rather than a real-time user action, 60-second staleness is acceptable. Analysts querying the system are not expected to observe individual insertions in real time.
This is a deliberate trade-off. Active cache invalidation on write would require tracking which cache keys are affected by a given insert - non-trivial given the number of possible filter combinations. For this system, where profile creation is an admin-only batch operation rather than a real-time user action, 60-second staleness is acceptable. Analysts querying the system are not expected to observe individual insertions in real time.

If stricter freshness were required, the approach would be to flush all `profiles:*` keys on any write to the `classifications` table. This is simple but aggressive it would eliminate the cache benefit during any ingestion period.
If stricter freshness were required, the approach would be to flush all `profiles:*` keys on any write to the `classifications` table. This is simple but aggressive - it would eliminate the cache benefit during any ingestion period.

---

## Part 3 CSV Data Ingestion
## Part 3 - CSV Data Ingestion

### What was done

Expand All @@ -134,7 +134,7 @@ Used `busboy` to intercept the multipart upload stream as it arrives over the ne

**Batch inserts**

Valid rows are accumulated into a batch array. When the stream ends, rows are flushed to the database in chunks using a single multi-row `INSERT ... ON CONFLICT (name) DO NOTHING` per chunk. Chunks are processed in groups of 10 concurrently to balance parallelism against connection pool limits.
Valid rows are accumulated into a batch array. When the stream ends, rows are flushed to the database in chunks using a single multi-row `INSERT ... ON CONFLICT (name) DO NOTHING` per chunk. Chunks are processed in groups of 10 concurrently to balance parallelism against connection pool limits. This CHUNKS_SIZE is adjustable, and was adjusted between different values between 10 and 20, for optimal speed selection. CHUNKS sizes refer to how many chunks are written to the primary DB at once. The entire batch is split into chunks, and then multiple chunks are written at the same time (in parallel) using `Promise.all()`

**Batch size selection**

Expand All @@ -150,13 +150,14 @@ The final implementation uses 5,000 rows per batch with 10 concurrent batch grou

Each row is validated before being added to the batch:

- Missing or empty `name` skipped, counted as `missing_fields`
- Unrecognised `gender` value skipped, counted as `invalid_gender`
- Non-numeric or negative `age` skipped, counted as `invalid_age`
- Invalid `country_id` (not in ISO 3166-1 alpha-2 map) skipped, counted as `invalid_country`
- Missing `gender_probability` or `country_probability` skipped, counted as `missing_fields`
- `age_group` is optional if missing or invalid, it is derived from `age`
- Missing or empty `name` - skipped, counted as `missing_fields`
- Unrecognised `gender` value - skipped, counted as `invalid_gender`
- Non-numeric or negative `age` - skipped, counted as `invalid_age`
- Invalid `country_id` (not in ISO 3166-1 alpha-2 map) - skipped, counted as `invalid_country`
- Missing `gender_probability` or `country_probability` - skipped, counted as `missing_fields`
- `age_group` is optional - if missing or invalid, it is derived from `age`

The stats object is continuously incremented as rows are being checked/skipped, in order to give accurate stats as the response.
A single bad row never fails the upload. The stream continues processing remaining rows.

**Partial failure handling**
Expand All @@ -165,11 +166,11 @@ If a batch insert fails midway, rows already inserted remain in the database. Th

**Stream error handling**

If the network drops or the client disconnects mid-upload, `busboy` emits an error event which is caught by the `parser.on('error', reject)` handler. The Promise rejects, the `end` handler is not called, and no partial batch is flushed. Rows already inserted in completed batches remain consistent with the no-rollback requirement. The response is a 500 rather than a summary, since the total row count is unknown.
If the network drops or the client disconnects mid-upload, `busboy` emits an error event which is caught by the `parser.on('error', reject)` handler. The Promise rejects, the `end` handler is not called, and no partial batch is flushed. Rows already inserted in completed batches remain - consistent with the no-rollback requirement. The response is a 500 rather than a summary, since the total row count is unknown.

**Concurrent uploads**

Tested with two simultaneous 500k-row uploads. Each request maintains its own `batch` array, `stats` object, and `busboy` instance there is no shared mutable state between concurrent uploads. Both completed successfully with correct row counts and no data corruption.
Tested with two simultaneous 500k-row uploads. Each request maintains its own `batch` array, `stats` object, and `busboy` instance - there is no shared mutable state between concurrent uploads. Both completed successfully with correct row counts and no data corruption.

**Duplicate handling**

Expand Down Expand Up @@ -197,10 +198,11 @@ Tested with two simultaneous 500k-row uploads. Each request maintains its own `b

- The entire valid batch accumulates in memory before flushing. For a 500k-row file with mostly valid rows, this is approximately 50MB of JavaScript objects. Mid-stream flushing would reduce peak memory but adds async complexity. The current approach is simpler and correct for the stated constraints.
- Concurrent uploads each hold their own batch in memory. Under high concurrency on a memory-constrained server, this could be a concern.
- Read replica lag means a profile inserted via `POST /api/profiles` may not immediately appear in `GET /api/profiles` results if the replica has not caught up. Under normal Neon replication conditions this lag is under a second, but it is a real trade-off. This is compounded by the 60-second cache TTL — a newly inserted profile may not appear in query results for up to 60 seconds after insertion.
- The database was scaled through replication, not vertical scaling. This maintains the limitation of the database cracking under high concurrency, shown when I tried to increase the `CHUNKS_SIZE` to 30, for 30 chunks of 5000 rows flushed into the database in parallel. This did not end well for the database. Vertical scaling is still neded in real life.
- Read replica lag means a profile inserted via `POST /api/profiles` may not immediately appear in `GET /api/profiles` results if the replica has not caught up. Under normal Neon replication conditions this lag is under a second, but it is a real trade-off. This is compounded by the 60-second cache TTL - a newly inserted profile may not appear in query results for up to 60 seconds after insertion. Another issue with this is that `GET /api/profiles` uses caching, so until the TTL expires (which is 60 seconds), if a write happens on the database, the analysts still get stale data. Since this system is a read-heavy system, not a write-heavy system, situations like this will seldom occur. Hence, it is OK for this application.

---

## PKCE State Redis Migration
## PKCE State - Redis Migration

As a side effect of adding Redis for caching, the in-memory `pkceStore` Map used for OAuth PKCE state was migrated to Redis. The Map was a single-process store if the server restarted or multiple instances were running, OAuth flows would fail. Redis provides a shared, persistent store with automatic TTL expiry (600 seconds), which is more correct and more resilient.
As a side effect of adding Redis for caching, the in-memory `pkceStore` Map used for OAuth PKCE state was migrated to Redis. The Map was a single-process store - if the server restarted or multiple instances were running, OAuth flows would fail. Redis provides a shared, persistent store with automatic TTL expiry (600 seconds), which is more correct and more resilient.
Loading