lhotari opened a new pull request, #4827: URL: https://github.com/apache/bookkeeper/pull/4827
### Motivation #4730 changed the `maxReadsInProgressLimit` default from 0 to 10000, enabling the read-in-progress semaphore and its autoRead gate (added in #1410 and extended to the v2 protocol in #3324) by default. This combination can permanently deadlock the entire request path of a bookie: - Permits are acquired in `BookieRequestProcessor#onReadRequestStart`, which runs on the Netty event loop thread (called from the read processor constructors/factories before the request is submitted to the read thread pool). When the semaphore is exhausted, `acquireUninterruptibly()` parks the event loop. Blocking an event loop thread is a bug in itself: every channel registered on that loop stalls — no requests are decoded (reads or writes), no responses are flushed, no write futures complete, no future listeners run, and no channel closes are processed. - Permits are released only from the `writeAndFlush` future listener in `PacketProcessorBase#sendResponseAndWait` (v2 throttled read path). Responses are written from read worker threads, so the write only enqueues a flush task on the response channel's event loop — release therefore *requires* event-loop progress. Once permits are exhausted, every event loop parks on its next incoming read request, all pending releases freeze, and the bookie serves no traffic until restarted, while still appearing healthy to probes that don't exercise the data path (the HTTP admin/metrics endpoints run on separate threads). Exhaustion does not require misbehaving clients: permits are acquired at ingest, so a sustained read backlog (e.g. cache-missing, disk-bound reads) alone reaches the limit — with defaults, 10000 permits vs. `numReadWorkerThreads` (8) × `maxPendingReadRequestPerThread` (10000) = 80000 executor queue slots, so the semaphore trips long before `ETOOMANYREQUESTS` load shedding would. Slow consumers (open but non-writable channels) reach the same state through write futures that never complete. This affects master, the released 4.18.0, and branch-4.17 (the pending 4.17.4 release would ship it), so this mitigation should be considered for those branches as well. A proper fix should make admission non-blocking for event loop threads and guarantee permit release even when a write future never completes; the limit could be re-enabled by default afterwards. ### Changes - Revert the `ServerConfiguration#getMaxReadsInProgressLimit()` default from 10000 to 0 (disabled unless explicitly configured, the behavior before #4730), documenting in the javadoc why it is disabled by default. - Update the default-value test in `ReadEntryProcessorTest` accordingly. -- 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]
