Skip to content

Commit 9489726

Browse files
authored
fix trappy http stream tests (#116829)
1 parent 13c8aae commit 9489726

File tree

4 files changed

+17
-19
lines changed

4 files changed

+17
-19
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public void testClientConnectionCloseMidStream() throws Exception {
174174

175175
// await stream handler is ready and request full content
176176
var handler = ctx.awaitRestChannelAccepted(opaqueId);
177-
assertBusy(() -> assertNotNull(handler.stream.buf()));
177+
assertBusy(() -> assertNotEquals(0, handler.stream.bufSize()));
178178

179179
assertFalse(handler.streamClosed);
180180

@@ -187,7 +187,7 @@ public void testClientConnectionCloseMidStream() throws Exception {
187187

188188
// wait for resources to be released
189189
assertBusy(() -> {
190-
assertNull(handler.stream.buf());
190+
assertEquals(0, handler.stream.bufSize());
191191
assertTrue(handler.streamClosed);
192192
});
193193
}
@@ -204,15 +204,13 @@ public void testServerCloseConnectionMidStream() throws Exception {
204204

205205
// await stream handler is ready and request full content
206206
var handler = ctx.awaitRestChannelAccepted(opaqueId);
207-
assertBusy(() -> assertNotNull(handler.stream.buf()));
207+
assertBusy(() -> assertNotEquals(0, handler.stream.bufSize()));
208208
assertFalse(handler.streamClosed);
209209

210210
// terminate connection on server and wait resources are released
211211
handler.channel.request().getHttpChannel().close();
212212
assertBusy(() -> {
213-
// Cannot be simplified to assertNull.
214-
// assertNull requires object to not fail on toString() method, but closing buffer can
215-
assertTrue(handler.stream.buf() == null);
213+
assertEquals(0, handler.stream.bufSize());
216214
assertTrue(handler.streamClosed);
217215
});
218216
}
@@ -228,14 +226,14 @@ public void testServerExceptionMidStream() throws Exception {
228226

229227
// await stream handler is ready and request full content
230228
var handler = ctx.awaitRestChannelAccepted(opaqueId);
231-
assertBusy(() -> assertNotNull(handler.stream.buf()));
229+
assertBusy(() -> assertNotEquals(0, handler.stream.bufSize()));
232230
assertFalse(handler.streamClosed);
233231

234232
handler.shouldThrowInsideHandleChunk = true;
235233
handler.stream.next();
236234

237235
assertBusy(() -> {
238-
assertNull(handler.stream.buf());
236+
assertEquals(0, handler.stream.bufSize());
239237
assertTrue(handler.streamClosed);
240238
});
241239
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,15 @@ public class Netty4HttpRequestBodyStream implements HttpBody.Stream {
3737
private final List<ChunkHandler> tracingHandlers = new ArrayList<>(4);
3838
private final ThreadContext threadContext;
3939
private ByteBuf buf;
40-
private boolean hasLast = false;
4140
private boolean requested = false;
4241
private boolean closing = false;
4342
private HttpBody.ChunkHandler handler;
4443
private ThreadContext.StoredContext requestContext;
4544

45+
// used in tests
46+
private volatile int bufSize = 0;
47+
private volatile boolean hasLast = false;
48+
4649
public Netty4HttpRequestBodyStream(Channel channel, ThreadContext threadContext) {
4750
this.channel = channel;
4851
this.threadContext = threadContext;
@@ -112,11 +115,12 @@ private void addChunk(ByteBuf chunk) {
112115
comp.addComponent(true, chunk);
113116
buf = comp;
114117
}
118+
bufSize = buf.readableBytes();
115119
}
116120

117121
// visible for test
118-
ByteBuf buf() {
119-
return buf;
122+
int bufSize() {
123+
return bufSize;
120124
}
121125

122126
// visible for test
@@ -130,6 +134,7 @@ private void send() {
130134
var bytesRef = Netty4Utils.toReleasableBytesReference(buf);
131135
requested = false;
132136
buf = null;
137+
bufSize = 0;
133138
try (var ignored = threadContext.restoreExistingContext(requestContext)) {
134139
for (var tracer : tracingHandlers) {
135140
tracer.onNext(bytesRef, hasLast);
@@ -164,6 +169,7 @@ private void doClose() {
164169
if (buf != null) {
165170
buf.release();
166171
buf = null;
172+
bufSize = 0;
167173
}
168174
channel.config().setAutoRead(true);
169175
}

modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpRequestBodyStreamTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public void testEnqueueChunksBeforeRequest() {
6767
for (int i = 0; i < totalChunks; i++) {
6868
channel.writeInbound(randomContent(1024));
6969
}
70-
assertEquals(totalChunks * 1024, stream.buf().readableBytes());
70+
assertEquals(totalChunks * 1024, stream.bufSize());
7171
}
7272

7373
// ensures all received chunks can be flushed downstream
@@ -119,7 +119,7 @@ public void testReadFromChannel() {
119119
channel.writeInbound(randomLastContent(chunkSize));
120120

121121
for (int i = 0; i < totalChunks; i++) {
122-
assertNull("should not enqueue chunks", stream.buf());
122+
assertEquals("should not enqueue chunks", 0, stream.bufSize());
123123
stream.next();
124124
channel.runPendingTasks();
125125
assertEquals("each next() should produce single chunk", i + 1, gotChunks.size());

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,6 @@ tests:
223223
- class: org.elasticsearch.repositories.s3.RepositoryS3RestIT
224224
method: testReloadCredentialsFromKeystore
225225
issue: https://github.com/elastic/elasticsearch/issues/116811
226-
- class: org.elasticsearch.http.netty4.Netty4IncrementalRequestHandlingIT
227-
method: testClientConnectionCloseMidStream
228-
issue: https://github.com/elastic/elasticsearch/issues/116815
229-
- class: org.elasticsearch.http.netty4.Netty4IncrementalRequestHandlingIT
230-
method: testServerExceptionMidStream
231-
issue: https://github.com/elastic/elasticsearch/issues/116838
232226
- class: org.elasticsearch.xpack.searchablesnapshots.hdfs.SecureHdfsSearchableSnapshotsIT
233227
issue: https://github.com/elastic/elasticsearch/issues/116851
234228
- class: org.elasticsearch.xpack.esql.analysis.VerifierTests

0 commit comments

Comments
 (0)