Batched crawl crawler (2/2) by Lama-Thematique · Pull Request #598 · mwmbl/mwmbl

Lama-Thematique · 2026-11-05T23:09:54Z

NOT Backward compatible with older servers

blueheron786 · 2026-11-05T23:29:35Z

Can you clarify what you mean by "not backward compatible with older servers?" There's only the prod server right now to worry about.

Lama-Thematique · 2026-11-05T23:39:41Z

It means the prod server need to have #597 deployed before you can crawl with this

blueheron786 · 2026-11-06T02:29:27Z

Review of the crawler side. The main concern is a behaviour change, not a bug in the new code.

1. Synced items are re-submitted every round. The old code posted [item for item in local_items if item.url not in remote_item_urls]. The new code posts every local item for the top 100 terms: new_items = [item for item in local_items] is a plain copy with no filter. Items already in SYNCED_WITH_MAIN_INDEX state go to the server again each round. That inflates upload and storage, refreshes last_crawled on URLs the crawler did not actually recrawl, and inflates the agreement and stat counters. It should filter on doc.state != DocumentState.SYNCED_WITH_MAIN_INDEX, which is what _new_terms already does in index_batches.py.

2. The gate is gone, but nothing says that was intended. count_new_index_entries and MIN_NEW_INDEX_ENTRIES are no longer called, and the test for the "keeps quiet about results the remote index beats" case was deleted. The run_indexing docstring still describes the old gate. If dropping it is deliberate, the docstring and the dead code should go too. count_new_index_entries and its tests in test/test_crawl_promotion.py would then be unused. If it isn't, it should be restored.

3. One post carries up to 100 terms' worth of documents. Before, it was one post per term. A body that is too large, for example over Django's DATA_UPLOAD_MAX_MEMORY_SIZE, gets a 4xx on every round. The batches were already popped from Redis, so the results would be lost each time. Please chunk the posts, or cap the number of items.

4. The description says "not backward compatible with older servers", but the code is compatible. A 597-less server returns {status, url}, so new_items is None and the remote_index.retrieve(..., refresh=True) fallback runs. That fallback looks correct and is a good thing to keep, but it is untested. Either fix the PR description or drop the fallback.

5. Lost logging.

logger.info(f"Response: {response.text}") is gone.
The "Indexed items: x/y" log is gone.
With the fallback branch, there is no hint which path was taken.

6. Tests. The .json mocks only return new_items: []. Please add:

a test that returned items are written to the right local pages, with each item matched to its term;
a test for the new_items is None fallback;
a test that synced items are not re-posted.

7. Small cleanups.

new_items = [item for item in local_items] can go once 1 is fixed.
PostResultsResponse(**response.json()) raises a confusing validation error if the server ever returns something unexpected. That is fine under the "fail loudly" rule, but the raw body should be logged first.

CRAWLER_VERSION is bumped to 1.1.0, which is correct. This PR must not merge before 597 is deployed, if the new-items path is meant to be used. The fallback covers the other case.

Lama-Thematique · 2026-11-06T05:16:38Z

I changed it, however i would like check with @daoudclarke that filtering by doc.state != DocumentState.SYNCED_WITH_MAIN_INDEX make sense, and that doesn't prevent resubmissions forever. meaning : that if we re-crawled the page, are we correctly re-submitting with this filter?
Intended, we don't want to query constantly the remote.
This is the goal of the batched api, group many individual queries to reduce network overhead.
include_new_items is what makes it incompatible sending this to an older server will return a 500 error.

response.text was too big and spamming so it was removed
Indexed items: x/y not worth additional work to keep it
added a log for the fall back branch

"a test that returned items are written to the right local pages, with each item matched to its term;" -> I don't see how this can an issue from the new code ?
a test for the new_items is None fallback; -> done
a test that synced items are not re-posted. -> complex to test, i'd rather avoid

new_items now filters for the SYNCED_WITH_MAIN_INDEX, PostResultsResponse is good as is, logging the raw body is not wanted.

Lama-Thematique · 2026-11-06T05:18:29Z

also @blueheron786 When you ask for a review from claude please ask claude to comes up with the implementation for the new test, they are annoying to write.

Lama-Thematique · 2026-11-06T05:26:48Z

merging this this should close #596

daoudclarke · 2026-11-06T06:05:35Z

Re 1, that's actually a good question, it's the current behaviour but may be a bug

daoudclarke · 2026-11-06T08:29:42Z

Update: the data in this comment was unrepresentative (sparse 2024 batches). See the re-run below for corrected numbers.

Benchmark: batched vs per-term submission (#597 + #598)

The hypothesis holds. For the same crawl, the batched flow uses about 2.4× less server CPU, and the server index ends up with the same content. Prod would likely save more than this local test shows.

Setup. I ran a gunicorn server built from this branch (it includes #597) on a 100k-page index seeded with 3,000 real crawl batches. The old crawler (main) and the new one (this PR) then replayed the same 400 real batches against it: 40 run_indexing rounds of 10 batches each. I measured CPU across the server's whole process tree and counted requests and bytes on the crawler side. Each arm ran twice, in opposite orders, and the repeats agreed within about 2%.

8000

Per 39 rounds (warm-up dropped)	Old (per term)	New (batched)
Server CPU	74.9s / 74.3s	31.1s / 29.9s
Requests	~7,900 (6,840 `/raw` GETs, ~1,010 POSTs)	39 POSTs
Storage uploads and DB writes for submissions	~1,010	39
Download	29.2 MB	20.6 MB
Upload	0.64 MB	1.58 MB
Crawler CPU	59s	19s
URLs added to the server index	+394	+419

Why it wins. Each old POST cost the server about 60ms for things that happen once per request: key lookup, device and stats writes, agreements, and the storage upload. With about 26 POSTs a round, that outweighed the 200 cheap /raw reads. The old flow would only use less server CPU if fewer than about 8 of the 100 terms passed its client-side check each round.

Prod should save more, not less:

I ran the old count_new_index_entries check against the prod index for three of these rounds. 27–40 terms passed per round, versus 26 locally, so prod crawlers make more old-style POSTs than this test did.
Uploads went to local disk here. On prod each one goes to Backblaze.
nginx doesn't cache /raw, so on prod every one of those GETs reaches Django too.

Found along the way:

The payload is 5–13× duplicated. The crawler sends each URL once for every top term it matches; one round sent 198 items covering only 31 distinct URLs. The server re-tokenises and re-indexes every copy. A one-line dedupe by URL cut server CPU by another 22% (24.1s) and upload by 4.6×. That number is only a guide: the server builds its list of response terms from each item's term, so the deduped run also got fewer pages back. A proper fix would send the term list separately, or derive the terms on the server, as suggested in the Batched crawl server (1/2) #597 review.
Batched requests are bigger and slower individually. The median POST took 0.7s, the worst 3.8s, and the largest response was 1.4 MB, so each request holds a sync worker for seconds. Total load is still much lower, but a cap on terms or items per request would also bound this.
The username is missing from the User-Agent. I found this by reading the code, not by testing. _fetch_username() used to be called when the RemoteIndex was built each round; now it only runs in the fallback path. So the results POST carries api_user_agent(None), which has no user … part. In the test traffic, the new crawler never requested /platform/user.

Limits of the test: one sync gunicorn worker, a 100k-page index rather than prod's 102M pages, and batches from 2024. None of these is likely to reverse the result, because the cost difference comes from per-request overhead, which doesn't depend on index size.

🤖 Generated with Claude Code

daoudclarke · 2026-11-06T11:08:25Z

Re-run with current crawler data (replaces the numbers in my earlier comment)

The batch files my first run replayed came from 2024. They held only 2–8 documents each, so each round was far lighter than a current crawler's. I re-ran everything with realistic inputs:

Batches: 270 batches from the current crawler (main) run locally, warm-up dropped. That's 25 rounds of 10 batches, about 900 documents per round.
Server index: seeded from prod. For each of the 307 terms these rounds sync, I loaded prod's /raw results into that term's page, so each term faces the competition it would on prod.
Arms: old and new, twice each, plus a URL-deduped variant.

The conclusion holds, with smaller numbers and a different explanation.

Per 24 rounds (warm-up dropped)	Old (per term)	New (batched)
Server CPU	579s / 565s	323s / 313s
Server request time	697s / 638s	334s / 335s
Requests	~6,330 (4,044 `/raw`, ~2,267 POSTs)	24 POSTs
Upload	7.8 MB	14.8 MB
Download	36.9 MB	25.8 MB
Crawler CPU	249s / 244s	217s / 215s
Distinct URLs in the server index at the end	26,112	26,184

Server CPU is 1.8× lower, and server request time is about 2× lower. The final index content is the same (+0.3% URLs), and repeat runs produced identical indexes.
The old gate filters almost nothing on real data. About 94 of the 100 terms passed every round, so the old flow made about 94 POSTs and 170 /raw GETs per round. Dropping the gate costs essentially nothing.
Why it wins. I profiled the server (py-spy) during batched POSTs. 98% of the time is in index_documents, and 75% is write-time re-ranking: sort_documents → score_result → regex matching. The old flow put each document through that once for every term POST it appeared in. This is my reading of the profile, not something I measured separately in the old flow. The ~60ms fixed cost per request I gave last time came from the unrealistically light rounds.
Deduping by URL is a small win on real data: about 5% less server CPU (297s) and 20% less upload. Last time I said 22%, which was also an artifact of the sparse batches.

What to watch: one batched POST now takes a median of 13s and up to 19s of server time, with about 600 KB up and 1 MB down. That's well inside prod's 120s gunicorn timeout, and total worker time is half what it was. But each crawler now holds one sync worker for 10–20s per round, and that grows with round size. Capping the items per request bounds it.

The next lever for scalability: both flows pay the write-time re-ranking cost, and most of it is regex work. 20% of POST time goes to re._compile: get_match_features builds a pattern for every document scored, and the re module's 512-entry cache thrashes. Compiling each term's regex once per page, or moving score_result into mwmbl_rank, should cut a large share of indexing CPU whichever submission protocol we use.

The username finding from my earlier comment still applies.

🤖 Generated with Claude Code

blueheron786 mentioned this pull request Nov 6, 2026

Batched crawl server (1/2) #597

Merged

crawler side batched api

18ff668

Lama-Thematique force-pushed the batched-crawl-result-crawler branch from 86343c7 to 18ff668 Compare November 6, 2026 05:14

Lama-Thematique and others added 2 commits November 6, 2026 11:45

clean dead code, reintroduced _fetch_username()

bc596d5

Merge branch 'main' into batched-crawl-result-crawler

eab01fe

daoudclarke merged commit 85e311a into mwmbl:main Nov 6, 2026
4 checks passed

daoudclarke mentioned this pull request Nov 8, 2026

Count newly indexed URLs, not submitted results, in crawler stats #609

Merged

Navigation Menu

Repository navigation

Batched crawl crawler (2/2) - #598

Batched crawl crawler (2/2)#598
daoudclarke merged 3 commits into
mwmbl:mainfrom
Lama-Thematique:batched-crawl-result-crawler

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Benchmark: batched vs per-term submission (#597 + #598)

Uh oh!

Re-run with current crawler data (replaces the numbers in my earlier comment)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants