Skip to content

Commit 6f622e8

Browse files
authored
Revert "Replace auto-read with proper flow-control in HTTP pipeline (#127259)" (#127403)
This reverts commit 3cf7061 and unmutes the associated tests Closes #127391 Closes #127392
1 parent 79e9dfd commit 6f622e8

File tree

16 files changed

+1082
-426
lines changed

16 files changed

+1082
-426
lines changed

docs/changelog/127259.yaml

Lines changed: 0 additions & 5 deletions
This file was deleted.

modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/http/netty4/Netty4IncrementalRequestHandlingIT.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ public void testClientConnectionCloseMidStream() throws Exception {
205205

206206
// await stream handler is ready and request full content
207207
var handler = clientContext.awaitRestChannelAccepted(opaqueId);
208+
assertBusy(() -> assertNotEquals(0, handler.stream.bufSize()));
208209

209210
assertFalse(handler.isClosed());
210211

@@ -214,6 +215,7 @@ public void testClientConnectionCloseMidStream() throws Exception {
214215
assertEquals(requestTransmittedLength, handler.readUntilClose());
215216

216217
assertTrue(handler.isClosed());
218+
assertEquals(0, handler.stream.bufSize());
217219
}
218220
}
219221

@@ -230,6 +232,7 @@ public void testServerCloseConnectionMidStream() throws Exception {
230232

231233
// await stream handler is ready and request full content
232234
var handler = clientContext.awaitRestChannelAccepted(opaqueId);
235+
assertBusy(() -> assertNotEquals(0, handler.stream.bufSize()));
233236
assertFalse(handler.isClosed());
234237

235238
// terminate connection on server and wait resources are released
@@ -238,6 +241,7 @@ public void testServerCloseConnectionMidStream() throws Exception {
238241
handler.channel.request().getHttpChannel().close();
239242
assertThat(safeGet(exceptionFuture), instanceOf(ClosedChannelException.class));
240243
assertTrue(handler.isClosed());
244+
assertBusy(() -> assertEquals(0, handler.stream.bufSize()));
241245
}
242246
}
243247

@@ -253,6 +257,7 @@ public void testServerExceptionMidStream() throws Exception {
253257

254258
// await stream handler is ready and request full content
255259
var handler = clientContext.awaitRestChannelAccepted(opaqueId);
260+
assertBusy(() -> assertNotEquals(0, handler.stream.bufSize()));
256261
assertFalse(handler.isClosed());
257262

258263
// terminate connection on server and wait resources are released
@@ -264,6 +269,7 @@ public void testServerExceptionMidStream() throws Exception {
264269
final var exception = asInstanceOf(RuntimeException.class, safeGet(exceptionFuture));
265270
assertEquals(ServerRequestHandler.SIMULATED_EXCEPTION_MESSAGE, exception.getMessage());
266271
safeAwait(handler.closedLatch);
272+
assertBusy(() -> assertEquals(0, handler.stream.bufSize()));
267273
}
268274
}
269275

@@ -304,7 +310,7 @@ public void testClientBackpressure() throws Exception {
304310
});
305311
handler.readBytes(partSize);
306312
}
307-
assertTrue(handler.receivedLastChunk);
313+
assertTrue(handler.stream.hasLast());
308314
}
309315
}
310316

modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/MissingReadDetector.java

Lines changed: 0 additions & 78 deletions
This file was deleted.

modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpAggregator.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import io.netty.handler.codec.http.HttpObjectAggregator;
1616
import io.netty.handler.codec.http.HttpRequest;
1717
import io.netty.handler.codec.http.HttpRequestDecoder;
18-
import io.netty.handler.codec.http.LastHttpContent;
1918

2019
import org.elasticsearch.http.HttpPreRequest;
2120
import org.elasticsearch.http.netty4.internal.HttpHeadersAuthenticatorUtils;
@@ -49,9 +48,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
4948
}
5049
if (aggregating || msg instanceof FullHttpRequest) {
5150
super.channelRead(ctx, msg);
52-
if (msg instanceof LastHttpContent == false) {
53-
ctx.read(); // HttpObjectAggregator is tricky with auto-read off, it might not call read again, calling on its behalf
54-
}
5551
} else {
5652
streamContentSizeHandler.channelRead(ctx, msg);
5753
}

modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpContentSizeHandler.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ private void handleRequest(ChannelHandlerContext ctx, HttpRequest request) {
123123
isContinueExpected = true;
124124
} else {
125125
ctx.writeAndFlush(EXPECTATION_FAILED_CLOSE.retainedDuplicate()).addListener(ChannelFutureListener.CLOSE);
126-
ctx.read();
127126
return;
128127
}
129128
}
@@ -137,7 +136,6 @@ private void handleRequest(ChannelHandlerContext ctx, HttpRequest request) {
137136
decoder.reset();
138137
}
139138
ctx.writeAndFlush(TOO_LARGE.retainedDuplicate()).addListener(ChannelFutureListener.CLOSE_ON_FAILURE);
140-
ctx.read();
141139
} else {
142140
ignoreContent = false;
143141
currentContentLength = 0;
@@ -152,13 +150,11 @@ private void handleRequest(ChannelHandlerContext ctx, HttpRequest request) {
152150
private void handleContent(ChannelHandlerContext ctx, HttpContent msg) {
153151
if (ignoreContent) {
154152
msg.release();
155-
ctx.read();
156153
} else {
157154
currentContentLength += msg.content().readableBytes();
158155
if (currentContentLength > maxContentLength) {
159156
msg.release();
160157
ctx.writeAndFlush(TOO_LARGE_CLOSE.retainedDuplicate()).addListener(ChannelFutureListener.CLOSE);
161-
ctx.read();
162158
} else {
163159
ctx.fireChannelRead(msg);
164160
}

0 commit comments

Comments
 (0)