belliottsmith commented on a change in pull request #1045:
URL: https://github.com/apache/cassandra/pull/1045#discussion_r679927942
##########
File path: src/java/org/apache/cassandra/transport/PreV5Handlers.java
##########
@@ -95,23 +95,32 @@ private void
releaseItem(Flusher.FlushItem<Message.Response> item)
// since the request has been processed, decrement inflight
payload at channel, endpoint and global levels
channelPayloadBytesInFlight -= itemSize;
- ResourceLimits.Outcome endpointGlobalReleaseOutcome =
endpointPayloadTracker.release(itemSize);
+ boolean globalInFlightBytesBelowLimit =
endpointPayloadTracker.release(itemSize) == ResourceLimits.Outcome.BELOW_LIMIT;
- // now check to see if we need to reenable the channel's autoRead.
- // If the current payload side is zero, we must reenable autoread
as
+ // Now check to see if we need to reenable the channel's autoRead.
+ //
+ // If the current payload bytes in flight is zero, we must
reenable autoread as
// 1) we allow no other thread/channel to do it, and
- // 2) there's no other events following this one (becuase we're at
zero bytes in flight),
- // so no successive to trigger the other clause in this if-block
+ // 2) there are no other events following this one (becuase we're
at zero bytes in flight),
+ // so no successive to trigger the other clause in this if-block.
+ //
+ // The only exception to this is if the global request rate limit
has been breached, which means
+ // we'll have to wait until a scheduled wakeup task unpauses the
connection.
//
// note: this path is only relevant when part of a pre-V5
pipeline, as only in this case is
// paused ever set to true. In pipelines configured for V5 or
later, backpressure and control
// over the inbound pipeline's autoread status are handled by the
FrameDecoder/FrameProcessor.
ChannelConfig config = item.channel.config();
- if (paused && (channelPayloadBytesInFlight == 0 ||
endpointGlobalReleaseOutcome == ResourceLimits.Outcome.BELOW_LIMIT))
+
+ if (!config.isAutoRead())
Review comment:
This is a bit confusing to me in combination with the unpause at the
time we impose rate limiting. Since v4 clients may remain widespread for a
while it might be good to normalise this, but it might require tracking _why_
we are paused for when we apply a wake-up. For instance, if we are paused due
to insufficient memory, we should simply unpause the connection immediately. If
we are paused due to rate limiting, we probably just want to let the previously
scheduled unpause do its thing, right?
--
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]
