GGraziadei opened a new pull request, #2135: URL: https://github.com/apache/stormcrawler/pull/2135
Fixes #2134. `fetcher.thread.timeout` ran the protocol call on a single-thread executor owned by each `FetcherThread` and abandoned it with `future.cancel(true)` when the deadline passed. okhttp does not honour the interrupt while connecting or reading, so the helper stayed blocked on the socket and the `FetcherThread` queued behind it at its next fetch: one host dribbling bytes could take a thread out of service for as long as `http.timeout`. The option also doubled the bolt's thread count, and the robots.txt lookup was not covered by the timeout at all. ### Change **okhttp protocol** - `fetcher.thread.timeout` is applied as a per-call deadline with `Call.timeout()`, enforced by okio's shared watchdog thread: on expiry the call is cancelled, the socket closed and the fetching thread gets an `InterruptedIOException` immediately, classified as "Socket timeout fetching" as before. - The deadline is clamped to `topology.message.timeout.secs`, so that it can never loosen the client-level `callTimeout` derived from it (a warning is logged). - With `http.content.partial.as.trimmed` the content received before the deadline is kept and flagged as trimmed for "time", which is what that option already did for the call timeout. Documented. - `Protocol` gains `default boolean supportsFetchTimeout()` (false), backward compatible for external and user protocols. okhttp returns true when the deadline is configured; `DelegatorProtocol` only when every delegate does, since the bolt cannot know in advance which delegate a URL is routed to. **Fetcher bolts** - New package-private `FetchTimeoutHelpers`, owned by `FetcherBolt` and `SimpleFetcherBolt`. Its `call()` runs both the robots.txt lookup and the fetch: on the calling thread when the protocol enforces the timeout itself, otherwise on a helper thread from one bounded pool per bolt (`fetcher.thread.timeout.helpers`, default 2 × `fetcher.threads.number`, 2 for `SimpleFetcherBolt`; threads created on demand, released after a minute idle). - A deadline on the helper path throws a typed `TimeoutException`; a full pool rejects at once with `SaturatedException`, reported as `FETCH_ERROR` with `fetch.exception` "No fetch helper available". Helpers are shared by all hosts, so a host that never answers can make fetches of other hosts fail this way until its helpers time out; stated in the docs. - With the default okhttp protocol no helper thread is ever created. - New `fetchhelpers` gauge and `fetch.timeout` / `fetch.helper.rejected` counters. - `fetcher.thread.timeout` is now defined in `Constants`; the public alias `FetcherBolt.FETCH_TIMEOUT_PARAM_KEY` (#1861) is removed. The configuration key is unchanged. - `configuration.adoc` documents `fetcher.thread.timeout`, which was missing, and the new helpers key. ### Behaviour | protocol | `fetcher.thread.timeout` set | before | after | |---|---|---|---| | okhttp (default) | yes | fetch abandoned on a per-thread helper that stays blocked; 50 extra threads | call cancelled, socket closed, no helper threads | | okhttp | no | unchanged | unchanged | | other (Playwright, custom) | yes | per-thread helper; a stuck helper pinned its `FetcherThread` | bounded shared pool; stuck helpers do not pin threads; explicit saturation error | | any | yes | robots.txt lookup outside the timeout | covered | Exception classification now also maps `InterruptedIOException` (okhttp's own timeouts) to "Socket timeout fetching" regardless of the option; previously such failures carried the exception class name in `fetch.exception`. ### Tests - `HttpProtocolFetchTimeoutTest`: a 5s response is cancelled at the 1s deadline; a 60s deadline is clamped to a 1s message timeout; partial content kept or failed depending on `http.content.partial.as.trimmed`. - `FetchTimeoutHelpersTest`: every branch of `call()` (off, self-timing protocol, helper thread, exception propagation, typed timeout with interrupt, saturation, bound from config, shutdown). - Fetcher bolt tests, run for both bolts, with a test protocol that hangs and ignores interruption: a stuck fetch followed by two fast ones on one fetcher thread yields two pages and one `FETCH_ERROR` (before: all three timed out); bounded pool with one rejection; hanging robots.txt reported at the timeout (before: the thread blocked for good). - `FetcherBoltTest`: no helper threads with okhttp; slow robots.txt bounded by the deadline with okhttp. - The existing `testThreadTimeout` passes unchanged through the okhttp path. --- ### 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]
