xiangfu0 commented on code in PR #20022:
URL: https://github.com/apache/pinot/pull/20022#discussion_r4366307952
##########
pinot-common/src/main/java/org/apache/pinot/common/config/NettyConfig.java:
##########
@@ -18,13 +18,56 @@
*/
package org.apache.pinot.common.config;
+import com.google.common.base.Preconditions;
import org.apache.pinot.spi.env.PinotConfiguration;
/// Container object for netty configuration of pinot clients and servers
(netty, grizzly, etc.)
+///
+/// Every setting is read as `<namespace>.<suffix>`, where the namespace is
`pinot.broker.netty` on the broker and
+/// `pinot.server.netty` on the server. The channel liveness settings below
only apply to the client side, i.e. the
+/// broker-to-server channels opened by `ServerChannels`; the server-side
`QueryServer` ignores them.
public class NettyConfig {
private static final String NATIVE_TRANSPORTS_ENABLED =
"native.transports.enabled";
+
+ /// Upper bound on how long a request write may stay pending before the
channel is closed, in milliseconds. A write
+ /// is pending while the kernel has not accepted the bytes, which on a
half-open or stalled socket is forever (the
+ /// peer stops acknowledging, the send buffer fills, and `isActive()` keeps
reporting `true` until the kernel gives
+ /// up on retransmits, typically after many minutes). Closing the channel
fails the in-flight queries on it fast
+ /// and lets the failure detector quarantine the server. Non-positive
disables the check. Broker key:
+ /// `pinot.broker.netty.write.timeout.ms`.
+ public static final String WRITE_TIMEOUT_MS = "write.timeout.ms";
+ public static final long DEFAULT_WRITE_TIMEOUT_MS = 10_000L;
Review Comment:
Fair, and the honest answer was that it depended on link speed: a 50 MB
segment list at 50 Mbit/s takes 8 s to hand to the kernel, so a fixed
completion timeout could fire on a healthy but slow link. The handler is now
progress-based instead: every write goes down the pipeline with a progressive
promise, Netty's outbound buffer advances it each time part of the message
reaches the socket, and the timer re-arms from the last progress. A large
request that keeps moving is never timed out, however long it takes in all;
only a socket that accepts nothing for 10 s is closed, which is the half-open
signature regardless of request size.
`ServerChannelsTest#testSlowButProgressingWriteIsNotTimedOut` drives an 8 MB
request through a peer draining about 3 MB/s with a 1 s timeout, so the write
outlives the timeout by more than a second and must still complete with the
channel open; the stall test still closes within the timeout. (One caveat worth
knowing: the signal is bytes the kernel accept
ed, so a *receiver application* that reads very slowly can make its kernel
batch window updates and look idle for stretches; a slow *network path* does
not, since acknowledgements free the send buffer continuously. A Pinot server
reads requests on its event loop, so the former only happens when that loop is
stuck, which is a case we want closed.)
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/BaseBrokerRoutingManager.java:
##########
@@ -520,6 +546,7 @@ private void processInstanceConfigChangeInternal() {
// always contains the selected servers
for (String newDisabledInstance : newDisabledServers) {
_enabledServerInstanceMap.remove(newDisabledInstance);
+ _previouslyEnabledServers.add(newDisabledInstance);
Review Comment:
Addressed in #20021 (this branch picks it up on rebase): a disabled server
whose instance config disappears is dropped from the map and reported through a
new `addServerRemovedCallback`, which the single-stage handler uses to evict
its channels, so the map is bounded by the servers currently disabled rather
than by every id the broker has ever seen.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
