From dbf28694468c86781b29a3cc61a1f6365d1f5b2d Mon Sep 17 00:00:00 2001 From: Shinsuke Sugaya Date: Sun, 30 Aug 2026 12:37:13 +0900 Subject: [PATCH 1/3] fix(opensearch): shrink the URL queue polling batch to 100 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. --- .../order/impl/DepthFirstUrlQueueOrder.java | 3 ++ .../impl/OpenSearchUrlQueueService.java | 9 ++++- .../impl/OpenSearchUrlQueueServiceTest.java | 38 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/DepthFirstUrlQueueOrder.java b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/DepthFirstUrlQueueOrder.java index ede86ae6..00d2f628 100644 --- a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/DepthFirstUrlQueueOrder.java +++ b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/DepthFirstUrlQueueOrder.java @@ -27,6 +27,9 @@ *

* The approximation is bounded by the polling fetch size: a batch is fetched, then fully * consumed before the next one, so URLs discovered mid-batch wait for the following batch. + * A crawl whose frontier fits in a single batch therefore proceeds level by level no matter + * which order is selected. Lowering the queue service's polling fetch size tightens the + * approximation, at the cost of one queue query per that many URLs. *

*/ public class DepthFirstUrlQueueOrder implements UrlQueueOrder { diff --git a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueService.java b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueService.java index 11059853..491306c4 100644 --- a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueService.java +++ b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueService.java @@ -75,8 +75,15 @@ public class OpenSearchUrlQueueService extends AbstractCrawlerService implements /** * The number of URLs to fetch when polling. + * + *

+ * A batch is handed out in full before the queue is queried again, so this is also how + * often the {@link UrlQueueOrder} is re-evaluated. A large batch makes every order + * degenerate towards the discovery order, because the URLs found while a batch is being + * consumed cannot be considered until the next one. + *

*/ - protected int pollingFetchSize = 1000; + protected int pollingFetchSize = 100; /** * The maximum size of the crawling queue. diff --git a/fess-crawler-opensearch/src/test/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueServiceTest.java b/fess-crawler-opensearch/src/test/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueServiceTest.java index f8d8eba9..1d4b7c29 100644 --- a/fess-crawler-opensearch/src/test/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueServiceTest.java +++ b/fess-crawler-opensearch/src/test/java/org/codelibs/fess/crawler/service/impl/OpenSearchUrlQueueServiceTest.java @@ -575,6 +575,44 @@ public void test_poll_followsTheConfiguredOrder() { } } + @Test + public void test_poll_reevaluatesTheOrderOncePerBatch() { + final String sessionId = "batch-session"; + final long base = System.currentTimeMillis(); + insertUrlQueue(sessionId, "http://www.example.com/shallow1", 1, base); + insertUrlQueue(sessionId, "http://www.example.com/shallow2", 1, base + 1L); + + final int defaultFetchSize = urlQueueService.pollingFetchSize; + urlQueueService.setUrlQueueOrder(new DepthFirstUrlQueueOrder()); + urlQueueService.setPollingFetchSize(2); + try { + // Both shallow URLs are fetched as one batch. Equal depth, so the newer wins. + assertEquals("http://www.example.com/shallow2", urlQueueService.poll(sessionId).getUrl()); + + // A deeper URL turns up while that batch is still being handed out. + insertUrlQueue(sessionId, "http://www.example.com/deep", 5, base + 2L); + + // The batch is drained before the queue is consulted again, so the deeper URL + // waits even though the order asks for the deepest first. + assertEquals("http://www.example.com/shallow1", urlQueueService.poll(sessionId).getUrl()); + assertEquals("http://www.example.com/deep", urlQueueService.poll(sessionId).getUrl()); + } finally { + urlQueueService.setUrlQueueOrder(new SequentialUrlQueueOrder()); + urlQueueService.setPollingFetchSize(defaultFetchSize); + urlQueueService.clearCache(); + } + } + + private void insertUrlQueue(final String sessionId, final String url, final int depth, final long createTime) { + final OpenSearchUrlQueue urlQueue = new OpenSearchUrlQueue(); + urlQueue.setSessionId(sessionId); + urlQueue.setUrl(url); + urlQueue.setCreateTime(createTime); + urlQueue.setDepth(depth); + urlQueue.setMethod("GET"); + urlQueueService.insert(urlQueue); + } + @Test public void test_di_registersTheBuiltInOrders() { for (final String name : new String[] { "sequentialUrlQueueOrder", "randomUrlQueueOrder", "depthFirstUrlQueueOrder", From 42c72d71e7e2e59a109d97300536d4af74bff3a8 Mon Sep 17 00:00:00 2001 From: Shinsuke Sugaya Date: Sun, 30 Aug 2026 12:37:25 +0900 Subject: [PATCH 2/3] fix: log a failing UrlQueueWeigher as one line, not a stack trace per 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. --- .../main/java/org/codelibs/fess/crawler/CrawlerThread.java | 7 +++++-- .../crawler/processor/impl/DefaultResponseProcessor.java | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/fess-crawler/src/main/java/org/codelibs/fess/crawler/CrawlerThread.java b/fess-crawler/src/main/java/org/codelibs/fess/crawler/CrawlerThread.java index 70330080..f86d2075 100644 --- a/fess-crawler/src/main/java/org/codelibs/fess/crawler/CrawlerThread.java +++ b/fess-crawler/src/main/java/org/codelibs/fess/crawler/CrawlerThread.java @@ -473,8 +473,11 @@ protected void offerChildUrls(final List> childList) { try { urlQueueWeigher.apply(crawlerContext.sessionId, childList); } catch (final Exception e) { - logger.warn("Failed to apply weigher {} to child URLs. Falling back to inherited weights.", - urlQueueWeigher.getClass().getName(), e); + logger.warn("Failed to apply weigher {} to {} child URL(s). Queueing them with whatever weights it left behind.", + urlQueueWeigher.getClass().getName(), childList.size()); + if (logger.isDebugEnabled()) { + logger.debug("Weigher {} failed.", urlQueueWeigher.getClass().getName(), e); + } } urlQueueService.offerAll(crawlerContext.sessionId, childList); } diff --git a/fess-crawler/src/main/java/org/codelibs/fess/crawler/processor/impl/DefaultResponseProcessor.java b/fess-crawler/src/main/java/org/codelibs/fess/crawler/processor/impl/DefaultResponseProcessor.java index 69e794c2..c6c235b3 100644 --- a/fess-crawler/src/main/java/org/codelibs/fess/crawler/processor/impl/DefaultResponseProcessor.java +++ b/fess-crawler/src/main/java/org/codelibs/fess/crawler/processor/impl/DefaultResponseProcessor.java @@ -279,8 +279,11 @@ protected void storeChildUrls(final CrawlerContext crawlerContext, final Set Date: Sun, 30 Aug 2026 12:37:50 +0900 Subject: [PATCH 3/3] docs(opensearch): say what separates the two weight-based orders 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. --- .../order/impl/SequentialUrlQueueOrder.java | 8 ++++++++ .../order/impl/WeightFirstUrlQueueOrder.java | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/SequentialUrlQueueOrder.java b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/SequentialUrlQueueOrder.java index e32d81a6..fc478d5e 100644 --- a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/SequentialUrlQueueOrder.java +++ b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/SequentialUrlQueueOrder.java @@ -23,6 +23,14 @@ /** * Fetches queued URLs by descending weight, then by discovery order. This is the default. + * + *

+ * Weight is the primary key, so a {@code UrlQueueWeigher} takes effect under this order + * without any {@code crawl.order} setting; discovery order only decides between entries of + * equal weight. Weights are uniform out of the box, which leaves discovery order as the + * effective sort. Use {@link WeightFirstUrlQueueOrder} instead when entries of equal weight + * should not be held to discovery order. + *

*/ public class SequentialUrlQueueOrder implements UrlQueueOrder { diff --git a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/WeightFirstUrlQueueOrder.java b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/WeightFirstUrlQueueOrder.java index e71b37ff..36f7cf83 100644 --- a/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/WeightFirstUrlQueueOrder.java +++ b/fess-crawler-opensearch/src/main/java/org/codelibs/fess/crawler/order/impl/WeightFirstUrlQueueOrder.java @@ -23,6 +23,20 @@ /** * Fetches queued URLs by descending weight only, leaving ties in the index order. + * + *

+ * This differs from the default {@link SequentialUrlQueueOrder} only in what happens between + * entries of equal weight: that order falls back to discovery order, this one lets the search + * engine return them however it likes. Choose it when a queue carries a large backlog scored + * by a {@code UrlQueueWeigher} and only the score should decide what is crawled next, with no + * bias towards whatever was discovered first. + *

+ * + *

+ * Without a weigher every entry is at the default weight, every entry ties, and the fetch + * order is whatever the index hands back - so this order only means something once weights + * differ. + *

*/ public class WeightFirstUrlQueueOrder implements UrlQueueOrder {