Skip to content

Commit 6689a8e

Browse files
authored
moshpit: stream bulk publishes instead of buffering them (#419)
Publishing a lot at once meant sending a lot at once, and the batch endpoint reads the whole request before the handler runs. Its ceiling is therefore however much we are willing to hold in memory for whoever asks, so "let us publish more" and "do not let anyone exhaust the daemon" pulled in opposite directions. Streaming removes the trade rather than picking a side. NDJSON is one item per line: the parser holds one line, the handler writes it, and a 500-item upload costs the same memory as a 1-item one. The cap can then be about total work instead of about a single allocation. POST /api/moshpit/sites/:name/content/stream takes application/x-ndjson and answers in NDJSON as it goes: {"type":"accepted","limits":{"items":500,...}} {"type":"progress","index":1,"ok":true,"created":true,"slug":"..."} {"type":"done","items":500,"created":500,"failed":0,"ms":1605} The first progress line arrives long before the last item is uploaded, which is what makes a progress bar possible at all. /pit/publish is a page that does this with a file picker; it hands the File straight to fetch, so the browser streams it from disk and the tab does not hold the payload either. Every cap bounds a distinct way of abusing a streaming endpoint, which is a better DoS target than a buffered one because it holds a socket for as long as a client keeps trickling: - 500 items, equal to a name's whole capacity, so one upload can fill a site and cannot do more than fill it - a per-line cap, checked after complete lines are drained, so a body with no newline in it cannot grow without bound under the total cap - a total-byte cap - a 20s idle timeout, rearmed on data, so a slow-but-live upload survives and a slowloris does not - four concurrent streams process-wide, then 503 with Retry-After rather than a queue, because a queue is a slower way to run out of memory A bad line costs that line, not the stream. A cap hit mid-stream cannot be a status code -- the 200 went out before the first item was written -- so it arrives as a final error line and everything already written stays written. The vhost needed its own block for this. nginx buffers a request body to disk before contacting the app by default, which would have turned "items land while the rest uploads" into one long silence followed by everything at once, and its 2m body cap would have refused the upload outright. Verified in a real browser over CDP, not just in tests: 300 items from a file picker drove the bar 0% -> 33% -> 62% -> 100%, 300 created in 1738ms, no console errors. A 500-item curl upload showed the first progress line at 70ms against a 1667ms upload. 575/575 apps/pwa and 2377/2382 root tests pass.
1 parent 816058e commit 6689a8e

8 files changed

Lines changed: 1110 additions & 0 deletions

apps/pwa/deploy/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ are in the right order already.
8080
documented-length posts (`MAX_PUBLISH_BYTES`, ~1.9M) and the vhost allows 2m
8181
so nginx is never the one saying no. Past that you get a 413 in JSON telling
8282
you to split the batch — safe to do, because publishing upserts on the slug.
83+
For a genuinely large import use the streaming endpoint instead, which does
84+
not buffer at all and has its own vhost block.
85+
- **Bulk upload appears to hang, then finishes all at once.** That is nginx
86+
buffering, and it means the `content/stream` location did not take. The
87+
endpoint streams both ways; `proxy_request_buffering off` and
88+
`proxy_buffering off` are what let items land while the rest is still
89+
uploading. `nginx -T | grep -A5 content/stream` shows whether it is live.
8390

8491
## Verifying, one layer at a time
8592

apps/pwa/deploy/nginx-vhost.conf

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,43 @@ server {
7070
proxy_send_timeout 30s;
7171
}
7272

73+
# Bulk publish. Everything about this location exists because the endpoint
74+
# streams in both directions, and nginx's defaults quietly undo that.
75+
location ~ ^/api/moshpit/sites/[^/]+/content/stream$ {
76+
proxy_pass http://127.0.0.1:@APP_PORT@;
77+
78+
proxy_set_header Host $host;
79+
proxy_set_header X-Real-IP $remote_addr;
80+
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
81+
proxy_set_header X-Forwarded-Proto $scheme;
82+
83+
# The upload is NDJSON read a line at a time, so the app never holds
84+
# more than one item. This is the cap on the whole upload rather than
85+
# on what fits in memory — the app enforces its own item and byte
86+
# limits underneath it.
87+
client_max_body_size 128m;
88+
89+
# THE important one. nginx buffers a request body to disk by default and
90+
# only then contacts the app, which would turn "items land while the
91+
# rest uploads" into one long silence followed by everything at once —
92+
# the exact behaviour the streaming endpoint exists to avoid.
93+
proxy_request_buffering off;
94+
# The same, for the progress lines coming back. The app also sends
95+
# X-Accel-Buffering: no, so this is belt and braces for an nginx that
96+
# was built to ignore the header.
97+
proxy_buffering off;
98+
# Chunked upload needs HTTP/1.1 upstream; the default is 1.0, which
99+
# cannot express a body of unknown length.
100+
proxy_http_version 1.1;
101+
102+
# A big import is legitimately slow. The progress lines keep the
103+
# response side busy, but the request side can be quiet while a client
104+
# on a poor uplink sends one item, so both directions get room. The
105+
# app's own idle timeout is the tighter, smarter limit.
106+
proxy_read_timeout 10m;
107+
proxy_send_timeout 10m;
108+
}
109+
73110
# Cheap liveness probe that does not touch the database.
74111
location = /healthz {
75112
proxy_pass http://127.0.0.1:@APP_PORT@;
Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
// Publishing a lot at once, without holding it all in memory.
2+
//
3+
// The JSON batch endpoint buffers the whole request before the handler runs, so
4+
// the most it can accept is the most we are willing to hold per request — and
5+
// that ceiling is set by what a hostile caller could do, not by what a real one
6+
// needs. Raising it to fit a real bulk import means agreeing to buffer that
7+
// much for anyone who asks.
8+
//
9+
// Streaming removes the trade. NDJSON is one item per line, so the parser holds
10+
// one line, the handler writes it, and the memory cost of a 500-item upload is
11+
// the same as a 1-item one. The cap can then be about the daemon's total work
12+
// rather than about a single allocation.
13+
//
14+
// The caps below exist because a streaming endpoint is a better DoS target than
15+
// a buffered one, not a worse one: it accepts an open socket for as long as the
16+
// client keeps trickling bytes. Every one of them bounds a different attack —
17+
// total bytes, a single unbounded line, item count, idle time, and how many of
18+
// these can run at once.
19+
20+
import { MAX_BODY, MAX_ITEMS_PER_NAME, MAX_TITLE } from "./moshpit-content.mjs";
21+
22+
/** The wire format: one JSON object per line. */
23+
export const STREAM_CONTENT_TYPES = ["application/x-ndjson", "application/ndjson", "application/jsonl"];
24+
25+
/**
26+
* The most items one stream may carry.
27+
*
28+
* Equal to a name's whole capacity on purpose: one upload can fill a site and
29+
* cannot do more than fill it, so there is no batch size that accomplishes
30+
* something a single stream could not.
31+
*/
32+
export const MAX_STREAM_ITEMS = MAX_ITEMS_PER_NAME;
33+
34+
/**
35+
* The most one line may weigh.
36+
*
37+
* An item's fields are already bounded (MAX_BODY + MAX_TITLE and a gallery of
38+
* URLs), and JSON escaping can widen a character to six bytes. This is that,
39+
* rounded up — enough that no legitimate item is refused, small enough that a
40+
* single line can never be an unbounded allocation.
41+
*/
42+
export const MAX_LINE_BYTES = (MAX_BODY + MAX_TITLE) * 6 + 64 * 1024;
43+
44+
/** The most a whole stream may weigh, however it is split into lines. */
45+
export const MAX_STREAM_BYTES = MAX_STREAM_ITEMS * MAX_LINE_BYTES;
46+
47+
/**
48+
* How long the stream may go without producing bytes.
49+
*
50+
* This is the slowloris guard. Without it a client can hold a connection and a
51+
* database handle open indefinitely by sending one byte a minute, which costs
52+
* them nothing and costs us a slot. Generous enough for a slow uplink mid-item.
53+
*/
54+
export const STREAM_IDLE_MS = 20_000;
55+
56+
/**
57+
* How many of these may run at once, process-wide.
58+
*
59+
* The real limit on concurrent bulk imports is the database behind them, not
60+
* the sockets. Past this the answer is 503 with a Retry-After rather than
61+
* queueing, because a queue is just a slower way to run out of memory.
62+
*/
63+
export const MAX_CONCURRENT_STREAMS = 4;
64+
65+
/** A cap was hit. Carries the status the route should report. */
66+
export class StreamLimitError extends Error {
67+
constructor(code, message, status = 413) {
68+
super(message);
69+
this.name = "StreamLimitError";
70+
this.code = code;
71+
this.status = status;
72+
}
73+
}
74+
75+
let active = 0;
76+
77+
/**
78+
* Take one of the concurrent-stream slots, or null when they are all taken.
79+
*
80+
* Returns the release function rather than a boolean so a caller cannot
81+
* acquire and forget which slot to give back — the only thing to hold onto is
82+
* the thing that frees it.
83+
*/
84+
export function acquireStreamSlot() {
85+
if (active >= MAX_CONCURRENT_STREAMS) return null;
86+
active += 1;
87+
let released = false;
88+
return () => {
89+
// Idempotent: the route releases in a finally block that can run after an
90+
// error path already released, and double-decrementing would hand out slots
91+
// that do not exist.
92+
if (released) return;
93+
released = true;
94+
active -= 1;
95+
};
96+
}
97+
98+
/** Live count, for tests and for the 503's message. */
99+
export function activeStreams() {
100+
return active;
101+
}
102+
103+
/** Test seam — drops any leaked slots between cases. */
104+
export function resetStreamSlots() {
105+
active = 0;
106+
}
107+
108+
export function isStreamContentType(header) {
109+
const type = String(header || "").split(";")[0].trim().toLowerCase();
110+
return STREAM_CONTENT_TYPES.includes(type);
111+
}
112+
113+
/**
114+
* Parse an NDJSON request body into items, one at a time.
115+
*
116+
* Yields `{ index, item }` as each line completes, so the caller can write it
117+
* and report progress before the next line has even arrived. Blank lines are
118+
* skipped — a trailing newline is the normal way to end a file, and treating it
119+
* as an empty item would fail every upload written by a well-behaved tool.
120+
*
121+
* A malformed line is yielded as `{ index, error }` rather than thrown. One bad
122+
* line in a bulk import should cost that line, not the several hundred good
123+
* ones already written before it — the same reasoning the batch endpoint's 207
124+
* follows.
125+
*/
126+
export async function* ndjsonItems(readable, {
127+
maxItems = MAX_STREAM_ITEMS,
128+
maxLineBytes = MAX_LINE_BYTES,
129+
maxBytes = MAX_STREAM_BYTES,
130+
idleMs = STREAM_IDLE_MS,
131+
} = {}) {
132+
const decoder = new TextDecoder("utf-8");
133+
let buffered = "";
134+
let bytes = 0;
135+
let index = 0;
136+
137+
let timer = null;
138+
let idle = null;
139+
const arm = () => {
140+
clearTimeout(timer);
141+
timer = setTimeout(() => {
142+
idle = new StreamLimitError("idle", `no data for ${idleMs}ms`, 408);
143+
// Ends the for-await below. Destroying is the point: an idle client is
144+
// holding a slot, so the socket goes with it.
145+
readable.destroy?.(idle);
146+
}, idleMs);
147+
};
148+
149+
// One line, validated and parsed. Returns null for a blank line.
150+
const take = (line) => {
151+
const text = line.trim();
152+
if (!text) return null;
153+
index += 1;
154+
if (index > maxItems) {
155+
throw new StreamLimitError("too_many_items", `publish up to ${maxItems} items in one stream`);
156+
}
157+
try {
158+
const parsed = JSON.parse(text);
159+
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
160+
return { index, error: "each line must be a JSON object" };
161+
}
162+
return { index, item: parsed };
163+
} catch {
164+
return { index, error: "that line is not valid JSON" };
165+
}
166+
};
167+
168+
arm();
169+
try {
170+
for await (const chunk of readable) {
171+
arm();
172+
173+
bytes += chunk.length;
174+
if (bytes > maxBytes) {
175+
throw new StreamLimitError("too_large", `a stream may carry up to ${maxBytes} bytes`);
176+
}
177+
178+
buffered += decoder.decode(chunk, { stream: true });
179+
180+
let nl = buffered.indexOf("\n");
181+
while (nl !== -1) {
182+
const line = buffered.slice(0, nl);
183+
buffered = buffered.slice(nl + 1);
184+
const out = take(line);
185+
if (out) yield out;
186+
nl = buffered.indexOf("\n");
187+
}
188+
189+
// Checked after draining complete lines, so this only ever measures an
190+
// unterminated one. Without it a client can send bytes forever with no
191+
// newline and grow `buffered` without bound — under the total cap the
192+
// whole time.
193+
if (Buffer.byteLength(buffered, "utf8") > maxLineBytes) {
194+
throw new StreamLimitError("line_too_large", `one item may be up to ${maxLineBytes} bytes`);
195+
}
196+
}
197+
198+
buffered += decoder.decode();
199+
const last = take(buffered);
200+
if (last) yield last;
201+
} catch (err) {
202+
// `readable.destroy(idle)` surfaces here as the error we constructed, but
203+
// some streams report an aborted read as a generic premature-close instead.
204+
// Reporting a timeout as a parse failure would send someone looking at
205+
// their JSON, so the idle flag wins over whatever the stream said.
206+
throw idle ?? err;
207+
} finally {
208+
clearTimeout(timer);
209+
}
210+
}

0 commit comments

Comments
 (0)