Give the Redis write queue the folding it never had by ralyodio · Pull Request #140 · profullstack/rssamplifier.com

ralyodio · 2026-08-19T15:53:14Z

Fixes the regression in #138.

What was wrong

#132 was merged but never actually wired — REDIS_URL had never been set on either service, so connect() had always taken the serializeWrites fallback. Switching it on in production on 2026-08-19 made throughput worse.

queueWrites posted one job per caller, and the worker — correctly at concurrency: 1, because SQLite permits one writer — turned each job into its own remote transaction. client.js measures that at ~370ms, so the queue's ceiling was roughly 2.7 writes/second for the whole cluster.

The in-process path it replaced had no such global limit: serializeWrites is per-process, and the crawl path runs under TURSO_CRAWL_AUTOCOMMIT=1 issuing single-statement execute() writes — which neither wrapper intercepts at all.

Measured, same database, same hour:

	with Redis	after removing it
items / 10 min	4,132	8,132
crawls / 10 min	582	1,003
import drain	stalled >1h at 65,474	draining

FIFO with one consumer, so the import's large periodic batch sat behind every small crawl batch. That's why it didn't slow down, it stopped.

What this changes

Folding moves to writeFolder.js and both paths share it. The first caller runs immediately; everyone arriving while it awaits the database is sent as one transaction, up to a statement ceiling. Failure handling is unchanged and now shared — a constraint error splits the group so a neighbouring feed isn't collateral damage, a timeout rejects the group without being multiplied into one retry per caller.

The queued path gets its own ceiling, TURSO_QUEUE_GROUP_STATEMENTS, default 50. It deliberately does not borrow TURSO_WRITE_GROUP_STATEMENTS, which is set to 1 in production (folding off) because a five-crawl group once exceeded the 30s deadline in-process. On the queued path folding isn't a tuning knob, it's the difference between a working queue and 2.7 writes/second.

Connections are shared. connect() built a fresh Queue + QueueEvents on every call, so the poller logged MaxListenersExceededWarning: 11 closing listeners added to [Queue] within minutes and climbing. Now shared per (url, prefix) and refcounted, so closeWriteQueue still means something to whoever holds the last one.

This does NOT make it safe to re-enable

Two things survive this PR and belong in #138:

Head-of-line blocking is inherent to one FIFO consumer. A bulk drain can still delay interactive writes — for less long, not never. Separate queues per class of write would be the real fix.
The ceiling is still a ceiling, now transaction-latency / statements-per-group instead of transaction-latency. Better, still finite.

Measure before setting REDIS_URL again. REDIS_URL remains removed from both services; this PR changes code only.

Verification

The extraction is behaviour-preserving: the 8 existing serializeWrites tests pass untouched
8 new tests on the folder itself: folding, per-caller result slicing, the ceiling not stranding an oversized caller, constraint errors isolating one caller, timeouts not amplifying, and the error classifier
pnpm -r test 1,101 pass / 0 fail, pnpm build clean, both on Node 22 (CI's version)

🤖 Generated with Claude Code

#132 was merged but never wired: REDIS_URL had not been set on either service, so `connect()` had always taken the `serializeWrites` fallback. Switching it on in production on 2026-08-19 made things worse, and this is why. `queueWrites` posted one job per caller, and the worker -- correctly at concurrency 1, because SQLite permits one writer -- turned each job into its own remote transaction. `client.js` measures that at about 370ms, so the queue's ceiling was roughly 2.7 writes a second for every process in the cluster combined. The in-process path it replaced had no such global limit: `serializeWrites` is per-process, and the crawl path runs under TURSO_CRAWL_AUTOCOMMIT=1 issuing single-statement `execute()` writes, which neither wrapper intercepts at all. Measured, same database, same hour: item ingestion halved (8,132 rows per ten minutes to 4,132), crawls fell from 1,003 to 582, and an import of 65,474 entries stopped dead for over an hour -- FIFO with one consumer, so its large periodic batch sat behind every small crawl batch. All of it recovered on removing the variable. So the folding moves into `writeFolder.js` and both paths use it. The first caller runs immediately; everyone arriving while it awaits the database is sent as one transaction, up to a statement ceiling. Failure handling is unchanged and now shared: a constraint error splits the group so a neighbouring feed is not collateral damage, while a timeout rejects the group without being multiplied into one retry per caller. The queued path gets its own ceiling, TURSO_QUEUE_GROUP_STATEMENTS, defaulting to 50 rather than borrowing TURSO_WRITE_GROUP_STATEMENTS -- that one is set to 1 in production, folding off, because a five-crawl group once exceeded the 30-second deadline in-process. On the queued path folding is not a tuning knob, it is the difference between a working queue and 2.7 writes a second, so it defaults on and modestly. Also fixed: `connect()` built a fresh Queue and QueueEvents on every call, each opening Redis connections and registering listeners, so the poller logged `MaxListenersExceededWarning: 11 closing listeners added to [Queue]` within minutes and kept climbing. They are now shared per (url, prefix) and refcounted, so `closeWriteQueue` still means something to whoever holds the last one. This does not make it safe to re-enable. Head-of-line blocking is inherent to one FIFO consumer -- a bulk drain can still delay interactive writes, just for less long -- and the ceiling is now transaction-latency / statements-per-group rather than transaction-latency. Measure before setting REDIS_URL again. See #138. The extraction is behaviour-preserving: the eight existing serializeWrites tests pass untouched. 1,101 tests, 0 failures, build clean, on Node 22. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…140) (#141) Every crawl of a feed that publishes a contact address but names nobody has been failing at the write since author enrichment shipped. In the hour this was found, 985 of 1,385 crawls errored; the queue stopped draining and /crawlstats went with it. `feedContacts` built each contact as `{ url, network }` and dropped the channel element it came from. Both `feed_links.source` and `author_links.source` are `not null`, so the statement bound `undefined` -- which the remote libSQL client will not serialize at all. It throws `Unsupported type of value` before any SQL runs, with no column named and no row to point at, and the crawl recorded it as `could not be crawled`: a publisher who looks down. The population is large and it is not a platform quirk. Any feed with a `<webMaster>` or `managingEditor` address whose name fails the person test takes this path -- WordPress and Substack alike, and Substack additionally names nobody else, so every newsletter on it qualified. Nothing caught it because the local SQLite driver the tests use binds `undefined` as null without complaint. The difference only exists on the wire, so `link-binds.test.js` asserts what the remote client accepts rather than what a local write happens to survive. Two fixes, because either alone leaves a hole: contacts now carry `source` (provenance worth keeping in its own right -- a mailbox from `itunes:owner` is a stronger claim than one from `webMaster`), and the four link bind sites default it, so no caller can put an unbindable value in a not-null column again. Also: a newsletter whose host fills in the iTunes block is no longer a podcast. Substack emits `<itunes:owner>` on every publication it serves and nothing else that looks like a show -- no `itunes:type`, no `podcast:` namespace, image enclosures rather than audio -- and that one tag filed the whole platform under /podcasts. This is the correction the video branch already makes: the tag has to be corroborated by what the feed actually ships. A declared show still stands on its own, so a podcast that has not released an episode yet keeps its category. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Carry a contact's provenance, which the crawler was failing without (#140) Every crawl of a feed that publishes a contact address but names nobody has been failing at the write since author enrichment shipped. In the hour this was found, 985 of 1,385 crawls errored; the queue stopped draining and /crawlstats went with it. `feedContacts` built each contact as `{ url, network }` and dropped the channel element it came from. Both `feed_links.source` and `author_links.source` are `not null`, so the statement bound `undefined` -- which the remote libSQL client will not serialize at all. It throws `Unsupported type of value` before any SQL runs, with no column named and no row to point at, and the crawl recorded it as `could not be crawled`: a publisher who looks down. The population is large and it is not a platform quirk. Any feed with a `<webMaster>` or `managingEditor` address whose name fails the person test takes this path -- WordPress and Substack alike, and Substack additionally names nobody else, so every newsletter on it qualified. Nothing caught it because the local SQLite driver the tests use binds `undefined` as null without complaint. The difference only exists on the wire, so `link-binds.test.js` asserts what the remote client accepts rather than what a local write happens to survive. Two fixes, because either alone leaves a hole: contacts now carry `source` (provenance worth keeping in its own right -- a mailbox from `itunes:owner` is a stronger claim than one from `webMaster`), and the four link bind sites default it, so no caller can put an unbindable value in a not-null column again. Also: a newsletter whose host fills in the iTunes block is no longer a podcast. Substack emits `<itunes:owner>` on every publication it serves and nothing else that looks like a show -- no `itunes:type`, no `podcast:` namespace, image enclosures rather than audio -- and that one tag filed the whole platform under /podcasts. This is the correction the video branch already makes: the tag has to be corroborated by what the feed actually ships. A declared show still stands on its own, so a podcast that has not released an episode yet keeps its category. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * A rate limit is a schedule instruction, not a broken feed Crawling faster got us 429s from Substack, and the crawler recorded each one as the publisher's fault: `markCrawlFailure` sets status='error', increments error_count, walks the backoff ladder, and at ten consecutive failures marks the feed dead. Every feed on one backend is throttled in the same minute, so this retires a whole platform for our own crawl rate -- and Substack is a large share of the directory. A 429 now reschedules and touches nothing else. `status`, `error_count`, `last_error` and `last_success_at` are left exactly as they were, and so is `last_fetched_at`: it means "when we last read this publisher", and a throttle is precisely the case where we did not. Stamping it would make a feed we have been bounced from all day look freshly crawled on every staleness report. 503 with a Retry-After is treated the same way. It is the same statement from a server that is briefly unwilling rather than permanently unable. The server picks the interval, since it is the only party that knows when its limit resets. `Retry-After` is parsed in both forms RFC 9110 allows -- a delay in seconds and an HTTP date -- floored at a minute so `Retry-After: 0` cannot spin, capped at a day so a misread date cannot mothball the feed, and defaulted to 30 minutes when the server names nothing. That is deliberately far shorter than the error ladder it replaces: the feed is healthy and we want it back soon. It is the *rate* that has to come down, and lengthening one feed's interval is the wrong instrument for that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…#144) `REDIS_URL` has never been set on either production service. `connect()` falls back to the in-process queue when it is absent -- deliberately, as what a Redis outage should degrade to -- so every write has been serialized per process while the Redis queue (#132) and the folding built for it (#140) sat dormant. Nothing was broken and nothing was logged. "We have a write queue" and "the write queue is running" looked identical from outside until somebody read the environment, which is a bad property for the component that decides the crawler's throughput. So `connect()` now says which path it took and *why*: the reason is the point, since "in-process" alone cannot distinguish a deliberate local run from a production service missing its broker. Announced once per path per process, because the web app calls `connect()` per request. The decision moves into an exported `writePath()` that returns the path and the reason and opens nothing. That is what made this testable: the first attempt asserted against `connect()` itself and hung the suite, because the client it returns keeps the event loop alive. The decision is the part worth testing; the client is not. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

ralyodio merged commit 26440d2 into main Aug 19, 2026
3 checks passed

ralyodio mentioned this pull request Aug 19, 2026

Say which write path the process took, because the fallback is silent #144

Merged

Navigation Menu

Uh oh!

Give the Redis write queue the folding it never had - #140

Give the Redis write queue the folding it never had#140
ralyodio merged 1 commit into
mainfrom
redis-queue-folding

ralyodio commented Aug 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

ralyodio commented Aug 19, 2026

What was wrong

What this changes

This does NOT make it safe to re-enable

Verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant