You are the engineer on call. Three tickets have come in against the comment feed. Some of them are bugs. Some of them may not be.
Your job is triage and a minimal fix — not a rewrite.
FEED-441 — opened by Ama (Support)
Comments are appearing on page 1 and then again on page 2. Different comments go missing entirely — a user linked us a comment of their own that they cannot find by paging through the thread it is in, and it is definitely still there, the direct link works.
Worst on the big threads. The busy live ones are unusable.
FEED-455 — opened by Ama (Support)
Four users this week with a completely empty feed. Not slow — empty. "No comments yet" on threads with thousands.
All four are long-standing accounts. The only thing I can find in common is that they all use the block feature a lot. One of them has 200-odd blocks. A brand new account with no blocks has never reported this.
FEED-460 — opened by Dev (Ops)
Sort order is broken on page 1. Newest first is the entire point of this feed and I am looking at a thread whose top comment is from March. Scroll down and the rest of the page is correctly ordered, newest at the top of the remainder. Just that one at the top is wrong.
Comment on FEED-455 from Sam (Data), 2 days ago
Seen this before at my last place. It is the connection pool —
pgdefaults to 10 clients and we are saturating it under load, so some queries come back partial instead of erroring. That is also why it looks random. Bumpmaxto 50 and add a statement timeout.
CREATE TABLE users (
id uuid PRIMARY KEY,
handle text NOT NULL UNIQUE,
created_at timestamptz NOT NULL DEFAULT now()
);
CREATE TABLE threads (
id uuid PRIMARY KEY,
title text NOT NULL
);
CREATE TABLE comments (
id uuid PRIMARY KEY,
thread_id uuid NOT NULL REFERENCES threads(id) ON DELETE CASCADE,
author_id uuid NOT NULL REFERENCES users(id),
body text NOT NULL,
is_pinned boolean NOT NULL DEFAULT false,
is_deleted boolean NOT NULL DEFAULT false,
created_at timestamptz NOT NULL DEFAULT now()
);
CREATE INDEX comments_thread_created_idx
ON comments (thread_id, created_at DESC);
CREATE TABLE blocks (
user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE,
-- Nulled rather than deleted when the blocked account is removed, so the
-- block survives an account being recreated on the same handle.
blocked_user_id uuid REFERENCES users(id) ON DELETE SET NULL,
created_at timestamptz NOT NULL DEFAULT now()
);
/**
* feed/comments.ts — the comment feed query. Serves the thread view and the
* mobile API. Roughly 4k requests/second at peak.
*/
import { Pool } from "pg";
import { logger } from "../logger";
const pool = new Pool({ connectionString: process.env.DATABASE_URL });
const PAGE_SIZE = 20;
export interface Comment {
id: string;
authorHandle: string;
body: string;
isPinned: boolean;
createdAt: Date;
}
export interface Page {
comments: Comment[];
/** Total matching this thread + viewer, for the "N comments" header. */
total: number;
hasMore: boolean;
}
/**
* One page of a thread, as seen by one viewer.
*
* `page` is 1-based and comes straight off the query string.
*/
export async function getCommentPage(
threadId: string,
viewerId: string,
page: number
): Promise<Page> {
const offset = (Math.max(1, page) - 1) * PAGE_SIZE;
const rows = await pool.query<{
id: string;
handle: string;
body: string;
is_pinned: boolean;
created_at: Date;
}>(
`SELECT c.id, u.handle, c.body, c.is_pinned, c.created_at
FROM comments c
JOIN users u ON u.id = c.author_id
WHERE c.thread_id = $1
AND c.is_deleted = false
AND c.author_id NOT IN (
SELECT b.blocked_user_id
FROM blocks b
WHERE b.user_id = $2
)
ORDER BY c.is_pinned DESC, c.created_at DESC
LIMIT $3 OFFSET $4`,
[threadId, viewerId, PAGE_SIZE, offset]
);
const counted = await pool.query<{ count: string }>(
`SELECT count(*) AS count
FROM comments c
WHERE c.thread_id = $1
AND c.is_deleted = false`,
[threadId]
);
const total = Number(counted.rows[0].count);
logger.debug("feed page served", {
threadId,
viewerId,
page,
returned: rows.rowCount,
total,
});
return {
comments: rows.rows.map((r) => ({
id: r.id,
authorHandle: r.handle,
body: r.body,
isPinned: r.is_pinned,
createdAt: r.created_at,
})),
total,
hasMore: offset + rows.rowCount < total,
};
}
A table with one row per ticket, in order:
| Ticket | Verdict | Cause |
|---|---|---|
| FEED-441 | ||
| FEED-455 | ||
| FEED-460 |
Verdict is exactly one of REAL or NOT A BUG. Cause is one line — for a real bug, what is actually wrong; for a non-bug, what the reporter is actually looking at and why the code above does not have the defect they describe.
One paragraph on Sam's connection-pool explanation. Right, partly right, or wrong — and if wrong, say what a saturated pg pool actually does to a query, because it is not what he says it does.
For each ticket you marked REAL, and only those:
For FEED-441 specifically, if the correct fix changes the shape of the API rather than one line, say so plainly and give the smallest version of it you would actually ship.
Do not give me the whole file back.
At most two further defects nobody has reported, each one you can quote a line for.
If there is nothing else worth raising, write exactly Nothing else.
Every claim should be checkable against the schema and the code above, or against documented PostgreSQL behavior you can state exactly. One of these bugs turns on a rule about how SQL evaluates a specific operator — if that is your diagnosis, state the rule, do not just name it. A confident diagnosis of a bug that is not here costs you more than missing one that is.
| Ticket | Verdict | Cause |
|---|---|---|
| FEED-441 | REAL | LIMIT/OFFSET is being used over a live feed ordered by a non-unique key; new head rows shift offsets and equal (is_pinned, created_at) rows can be arbitrarily ordered, so rows repeat or fall between pages. |
| FEED-455 | REAL | SQL rule: x NOT IN (S) is FALSE if S has an equal non-NULL value, otherwise UNKNOWN if S contains any NULL; WHERE keeps only TRUE, and blocked_user_id can be NULL via ON DELETE SET NULL, so one nulled block filters out every row. |
| FEED-460 | NOT A BUG | The reporter is looking at a pinned comment; ORDER BY c.is_pinned DESC, c.created_at DESC intentionally puts pinned comments first, then sorts the remaining page newest-first. |
Sam is partly right about the default pg.Pool max being 10, but wrong about the failure mode and the diagnosis. A saturated pg pool does not make PostgreSQL return partial result sets; pool.query requests queue waiting for a checked-out client to be returned, and if acquisition fails or a configured connectionTimeoutMillis is reached the promise rejects with an error. Once a query starts, PostgreSQL either completes the statement and pg resolves with its full rows, or aborts/errors (for example statement_timeout cancels with a query_canceled error); pool saturation causes waiting or errors, not “partial queries,” and bumping max does not fix the nullable NOT IN predicate.
This is not a one-line patch: the API shape must change from a 1-based page to an opaque keyset cursor, with Page returning nextCursor. Adding only c.id to ORDER BY would make ties deterministic but would not stop OFFSET from shifting when new comments arrive. The smallest version I would ship is below; it assumes the FEED-455 replacement is also applied, with the block filter still above the inserted keyset predicate.
Wrong lines, verbatim:
export interface Page {
comments: Comment[];
/** Total matching this thread + viewer, for the "N comments" header. */
total: number;
hasMore: boolean;
}
page: number
): Promise<Page> {
const offset = (Math.max(1, page) - 1) * PAGE_SIZE;
is_pinned: boolean;
created_at: Date;
}>(
`SELECT c.id, u.handle, c.body, c.is_pinned, c.created_at
ORDER BY c.is_pinned DESC, c.created_at DESC
LIMIT $3 OFFSET $4`,
[threadId, viewerId, PAGE_SIZE, offset]
logger.debug("feed page served", {
threadId,
viewerId,
page,
returned: rows.rowCount,
total,
});
return {
comments: rows.rows.map((r) => ({
total,
hasMore: offset + rows.rowCount < total,
};
Exact replacement:
type FeedCursor = {
isPinned: boolean;
createdAtKey: string;
id: string;
};
const decodeCursor = (s: string): FeedCursor =>
JSON.parse(Buffer.from(s, "base64url").toString("utf8")) as FeedCursor;
const encodeCursor = (r: {
is_pinned: boolean;
created_at_key: string;
id: string;
}): string =>
Buffer.from(
JSON.stringify({
isPinned: r.is_pinned,
createdAtKey: r.created_at_key,
id: r.id,
})
).toString("base64url");
export interface Page {
comments: Comment[];
nextCursor?: string;
/** Total matching this thread + viewer, for the "N comments" header. */
total: number;
hasMore: boolean;
}
cursor?: string
): Promise<Page> {
const after = cursor ? decodeCursor(cursor) : undefined;
is_pinned: boolean;
created_at: Date;
created_at_key: string;
}>(
`SELECT c.id, u.handle, c.body, c.is_pinned, c.created_at,
c.created_at::text AS created_at_key
AND (
$3::boolean IS NULL
OR (c.is_pinned, c.created_at, c.id) < ($3::boolean, $4::timestamptz, $5::uuid)
)
ORDER BY c.is_pinned DESC, c.created_at DESC, c.id DESC
LIMIT $6`,
[
threadId,
viewerId,
after?.isPinned ?? null,
after?.createdAtKey ?? null,
after?.id ?? null,
PAGE_SIZE + 1,
]
const hasMore = rows.rowCount > PAGE_SIZE;
const pageRows = rows.rows.slice(0, PAGE_SIZE);
const nextCursor = hasMore
? encodeCursor(pageRows[pageRows.length - 1])
: undefined;
logger.debug("feed page served", {
threadId,
viewerId,
cursor,
returned: pageRows.length,
total,
});
return {
comments: pageRows.map((r) => ({
total,
hasMore,
nextCursor,
};
Why: A total-order keyset cursor pins each next request to the last seen (is_pinned, created_at, id) tuple; new rows above the cursor no longer shift offsets, and c.id DESC resolves ties.
Wrong lines, verbatim:
AND c.author_id NOT IN (
SELECT b.blocked_user_id
FROM blocks b
WHERE b.user_id = $2
)
Exact replacement:
AND NOT EXISTS (
SELECT 1
FROM blocks b
WHERE b.user_id = $2
AND b.blocked_user_id = c.author_id
)
Why: x NOT IN (S) is UNKNOWN when S contains NULL and has no equal non-NULL value, and WHERE only keeps TRUE; NOT EXISTS matches only non-NULL viewer blocks, so a nulled block cannot poison the predicate.
The header total ignores the viewer’s blocks even though the doc says it is viewer-scoped. Quoted line: /** Total matching this thread + viewer, for the "N comments" header. */. The count query stops at AND c.is_deleted = false,with params[threadId]`, so it includes comments the row query filters out.
blocks.user_id has no index. Quoted line: user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE,. PostgreSQL does not automatically create an index on a foreign-key referencing column, yet the row filter uses WHERE b.user_id = $2; at 4k requests/second that will scan for block-heavy viewers.