Skip to content

fix: make the fetch order actually take effect, and quieten its failure paths - #194

Merged
marevol merged 3 commits into
masterfrom
fix/queue-order-followups
Aug 30, 2026
Merged

fix: make the fetch order actually take effect, and quieten its failure paths#194
marevol merged 3 commits into
masterfrom
fix/queue-order-followups

Conversation

@marevol

@marevol marevol commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Follow-ups from verifying #193 against a live Fess + OpenSearch. Each item was measured on a real crawl, not inferred.

The polling batch made every order look the same

fetchUrlQueueList hands out a whole batch before querying the queue again, so the polling fetch size is also how often the UrlQueueOrder is consulted. At 1000, a crawl whose frontier fits in one batch proceeds level by level whatever is configured — the sort only reorders siblings. On a 10-page fixture all five orders produced an identical breadth-first shape at 1000, and their intended shapes at 1.

Lowered to 100. On a 901-page fixture (300 branches, depth 3) with depthFirstUrlQueueOrder:

pollingFetchSize backtracks first depth-2 at first depth-3 at
1000 0 #302 #602
100 2 #102 #202

A batch of B against a level W URLs wide dives roughly W / B times, so this is a proportional improvement rather than a strict depth-first search — DepthFirstUrlQueueOrder's javadoc already said as much and now spells out the single-batch case. The extra queue queries are negligible next to fetching the pages themselves.

A new test pins the semantics directly: with a batch of 2, a deeper URL offered while the batch is being consumed still waits for the following batch.

A failing weigher printed a stack trace per page

Both weigher call sites logged the exception at WARN, so a weigher that fails consistently produced one full stack trace for every page with children — the same problem already fixed one file over for an unresolvable crawl.order. Measured: 7 pages with children, 7 stack traces; now 7 single lines and the traces at DEBUG.

The wording was also wrong. A weigher that throws part way through has already mutated part of the batch, so "Falling back to inherited weights" claims more than the code does. The messages now name the parent URL (or the child count) so a repeated warning points at the page that tripped it.

Neither weight-based order said which to reach for

sequentialUrlQueueOrder is weight DESC, createTime ASC; weightFirstUrlQueueOrder is weight DESC. They differ only between entries of equal weight, and the fact that gets missed is that weight is already the default order's primary key — a UrlQueueWeigher changes the fetch order with no crawl.order setting at all. Confirmed live: with a weigher scoring /a/=3.0, /b/=2.0, /c/=1.0 and crawl.order unset, the crawl came back strictly weight-ordered within each level.

Both javadocs now say this, and WeightFirstUrlQueueOrder's says what it is for — a large weighted backlog where only the score should decide what comes next, with no bias towards whatever was discovered first — and warns that without a weigher every entry ties and the order has nothing to sort by.

For the record, the second sort field is not what makes the difference at scale. On a queue-shaped index of 2,000,000 entries, size=100, two runs:

weights weight DESC weight DESC, createTime ASC
all at the default 1.0 4.0 / 1.0 ms 11.0 / 8.0 ms
spread 0.5–5.0 by a weigher 4.0 / 11.0 ms 4.0 / 8.0 ms

The single-field sort is consistently cheaper only when every entry ties, which is the case where its result is arbitrary anyway; once weights differ the two are within noise of each other. Either way it is milliseconds per queue poll, i.e. per 100 URLs.

Tests

mvn -B clean install passes: 2044 / 15 / 52 across the three modules, no failures.

fess-docs is updated in codelibs/fess-docs#514; the matching Fess change is codelibs/fess#3354.

@marevol marevol added this to the 15.9.0 milestone Aug 30, 2026
@marevol marevol self-assigned this Aug 30, 2026
A batch is handed out in full before the queue is queried again, so the
polling fetch size is also how often the UrlQueueOrder is re-evaluated. At
1000 a crawl whose frontier fits in one batch proceeds level by level no
matter which order is configured: depthFirstUrlQueueOrder, newestFirstUrlQueueOrder
and randomUrlQueueOrder all degenerate into the discovery order and only
reorder siblings. Measured against a live crawl of a 10-page fixture, all
five orders produced the same breadth-first shape at 1000 and their intended
shapes at 1.

100 keeps the query count negligible next to the cost of fetching the pages
themselves while making the order mean something on ordinary sites. The
javadoc now says what the number controls, and DepthFirstUrlQueueOrder's
existing caveat spells out the single-batch case.

Adds a test that pins the semantics: with a batch of 2, a deeper URL offered
while the batch is being consumed still waits for the following batch.
… page

Both weigher call sites logged the exception at WARN, so a weigher that fails
consistently prints one full stack trace for every page that has children -
the same problem that was already fixed one file over for an unresolvable
crawl.order value. The stack trace moves to DEBUG and the WARN stays a single
line.

The wording was also wrong. A weigher that throws part way through has already
mutated some of the batch, so "Falling back to inherited weights" claims more
than the code does; the entries are queued with whatever weights the weigher
left behind. The DefaultResponseProcessor message now names the parent URL and
the CrawlerThread one the number of children, so a repeated warning points at
which page tripped it.
Neither javadoc said which to reach for. The pair only differs in what happens
between entries of equal weight: sequentialUrlQueueOrder falls back to discovery
order, weightFirstUrlQueueOrder lets the index decide.

SequentialUrlQueueOrder now states that weight is its primary key, which is the
fact that gets missed - a UrlQueueWeigher changes the fetch order under the
default with no crawl.order setting at all. WeightFirstUrlQueueOrder now says
what it is for, a backlog scored by a weigher where only the score should decide
what is crawled next, and warns that without a weigher every entry ties and the
order means nothing.
@marevol
marevol force-pushed the fix/queue-order-followups branch from 7be652c to 8707407 Compare August 30, 2026 03:45
@marevol
marevol merged commit a41f0a7 into master Aug 30, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant