From 0eb454fe7efd0f7715a07c4c483079fb33a0b235 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 28 Nov 2022 20:33:22 +0000 Subject: [PATCH] Clarify writability in Netty4HttpPipeliningHandler TIL we don't really mean that the physical channel is writable in these loops, so this commit adds a couple of comments to record that lesson. --- .../http/netty4/Netty4HttpPipeliningHandler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpPipeliningHandler.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpPipeliningHandler.java index 61c98788f17f7..f5a32a0ec768c 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpPipeliningHandler.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpPipeliningHandler.java @@ -216,7 +216,8 @@ private void doWrite(ChannelHandlerContext ctx, Netty4ChunkedHttpResponse readyR combiner.add((Future) first); currentChunkedWrite = new ChunkedWrite(combiner, promise, readyResponse); if (enqueueWrite(ctx, readyResponse, first)) { - // we were able to write out the first chunk directly, try writing out subsequent chunks until the channel becomes unwritable + // We were able to write out the first chunk directly, try writing out subsequent chunks until the channel becomes unwritable. + // NB "writable" means there's space in the downstream ChannelOutboundBuffer, we aren't trying to saturate the physical channel. while (ctx.channel().isWritable()) { if (writeChunk(ctx, combiner, readyResponse.body())) { finishChunkedWrite(); @@ -280,6 +281,7 @@ private boolean doFlush(ChannelHandlerContext ctx) throws IOException { return false; } while (channel.isWritable()) { + // NB "writable" means there's space in the downstream ChannelOutboundBuffer, we aren't trying to saturate the physical channel. WriteOperation currentWrite = queuedWrites.poll(); if (currentWrite == null) { doWriteQueued(ctx);