GGraziadei opened a new pull request, #2132: URL: https://github.com/apache/stormcrawler/pull/2132
Fixes #2129. `FetchItemQueues` used a single monitor for adding, taking and finishing items, and `getFetchItem()` rotated a `LinkedHashMap` linearly over every queue whose crawl delay had not elapsed. With many hosts the fetcher threads held that lock most of the time and the executor thread calling `execute()` stalled on every incoming tuple. ### Change - Queues live in a `ConcurrentHashMap`; the ones that may have an item ready are referenced from a `DelayQueue` of tickets ordered by their next fetch time. Taking an item is O(log n) and adding never waits for the fetcher threads. - Per-queue state (size bound, in-progress count, next fetch time, crawl delays) uses atomics; a per-queue monitor is used only for the add/reap race. Empty queues are removed from the map as soon as they drain. - `FetchItem` is built before any lock is taken, so a DNS lookup in `byIP` mode no longer blocks the fetcher threads. ### Behaviour preserved Politeness per queue, `asap` release, `fetcher.threads.per.queue` and `fetcher.maxThreads.*`, `fetcher.max.queue.size`, crawl delay overrides from metadata and robots.txt, queue modes, metrics and the debug dump. One observable difference: among hosts that are ready at the same time, the order is "ready for the longest first" rather than round-robin by insertion order. ### Tests New `FetchItemQueuesTest`: politeness, `asap`, several hosts back to back, queue size bound, multiple threads per queue, crawl delay from metadata, reaping of empty queues, and a concurrent producers/consumers test checking that nothing is lost or duplicated. Existing `FetcherBoltTest` and `SimpleFetcherBoltTest` pass unchanged. ### Benchmark 50 fetcher threads, 1 producer thread, 20 URLs per host, `fetcher.server.delay` 1s, 8s per run, best of the measured samples. | hosts | metric | before | after | |---|---|---|---| | 20000 | `getFetchItem` avg | 1.58 ms | 24 us | | 20000 | `getFetchItem` p99 | 5.0 ms | 24 us | | 20000 | `addFetchItem` avg | 14.1 ms | 24 us | | 20000 | `addFetchItem` p99 | 458 ms | 0.11 ms | | 2000 | `addFetchItem` p99 | 2.5 ms | 0.13 ms | | 20000 | fetched/s | 20001 | 20000 | Throughput is unchanged, as expected: it is bounded by politeness. What goes away is the executor stall and the CPU burnt scanning queues that are not ready. --- ### For all changes - [x] Is there a issue associated with this PR? Is it referenced in the commit message? - [x] Does your PR title start with `#XXXX` where `XXXX` is the issue number you are trying to resolve? - [x] Has your PR been rebased against the latest commit within the target branch (typically main)? - [x] Is your initial contribution a single, squashed commit? - [x] Is the code properly formatted with `mvn git-code-format:format-code -Dgcf.globPattern="**/*" -Dskip.format.code=false`? ### For code changes - [x] Have you ensured that the full suite of tests is executed via `mvn clean verify`? - [x] Have you written or updated unit tests to verify your changes? - [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? (no new dependencies) - [x] If applicable, have you updated the LICENSE file, including the main LICENSE file? (not applicable) - [x] If applicable, have you updated the NOTICE file, including the main NOTICE file? (not applicable) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
