Skip to content

Commit d8f8621

Browse files
authored
Netty fixes and Storage perf test (Azure#45692)
Netty fixes and Storage perf test
1 parent 2c1fcaf commit d8f8621

37 files changed

+1206
-231
lines changed

eng/versioning/version_client.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,7 @@ com.azure.resourcemanager:azure-resourcemanager-kubernetesconfiguration-privatel
497497
com.azure.tools:azure-sdk-archetype;1.0.0;1.2.0-beta.1
498498
com.azure.tools:azure-sdk-build-tool;1.0.0;1.1.0-beta.1
499499
com.azure.v2:azure-client-sdk-parent;2.0.0-beta.1;2.0.0-beta.1
500+
com.azure.v2:azure-perf-test-parent;2.0.0-beta.1;2.0.0-beta.1
500501
com.azure.v2:azure-core;2.0.0-beta.1;2.0.0-beta.1
501502
com.azure.v2:azure-core-test;2.0.0-beta.1;2.0.0-beta.1
502503
com.azure.v2:azure-data-appconfiguration;2.0.0-beta.1;2.0.0-beta.1
@@ -506,6 +507,7 @@ com.azure.v2:azure-security-keyvault-certificates;5.0.0-beta.1;5.0.0-beta.1
506507
com.azure.v2:azure-security-keyvault-secrets;5.0.0-beta.1;5.0.0-beta.1
507508
com.azure.v2:azure-security-keyvault-keys;5.0.0-beta.1;5.0.0-beta.1
508509
com.azure.v2:azure-storage-blob;13.0.0-beta.1;13.0.0-beta.1
510+
com.azure.v2:azure-storage-perf;2.0.0-beta.1;2.0.0-beta.1
509511
io.clientcore:clientcore-parent;1.0.0-beta.3;1.0.0-beta.3
510512
io.clientcore:core;1.0.0-beta.10;1.0.0-beta.11
511513
io.clientcore:http-netty4;1.0.0-beta.1;1.0.0-beta.1
@@ -526,6 +528,7 @@ io.clientcore:annotation-processor-test;1.0.0-beta.1;1.0.0-beta.1
526528

527529
unreleased_com.azure.v2:azure-core;2.0.0-beta.1
528530
unreleased_com.azure.v2:azure-identity;2.0.0-beta.1
531+
unreleased_io.clientcore:http-netty4;1.0.0-beta.1
529532

530533
# Released Beta dependencies: Copy the entry from above, prepend "beta_", remove the current
531534
# version and set the version to the released beta. Released beta dependencies are only valid

sdk/clientcore/http-netty4/checkstyle-suppressions.xml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
<suppressions>
66
<suppress files="io.clientcore.http.netty4.NettyHttpClientBuilder.java" checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" />
7+
<suppress files="io.clientcore.http.netty4.implementation.Netty4ChannelBinaryData.java" checks="com.azure.tools.checkstyle.checks.RawExceptionThrowCheck" />
8+
<suppress files="io.clientcore.http.netty4.implementation.Netty4ChannelInputStream.java" checks="com.azure.tools.checkstyle.checks.RawExceptionThrowCheck" />
9+
<suppress files="io.clientcore.http.netty4.implementation.WrappedHttpHeaders.java" checks="com.azure.tools.checkstyle.checks.RawExceptionThrowCheck" />
710
<suppress files="io.clientcore.http.netty4.NettyHttpClientBuilder.java" checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" />
8-
<suppress files="io.clientcore.http.netty4.implementation.WrappedHttpHeaders" checks="com.azure.tools.checkstyle.checks.RawExceptionThrowCheck" />
911
</suppressions>

sdk/clientcore/http-netty4/pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838
<properties>
3939
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
4040
<legal><![CDATA[[INFO] Any downloads listed may be third party software. Microsoft grants you no rights for third party software.]]></legal>
41-
<jacoco.min.linecoverage>0.85</jacoco.min.linecoverage>
42-
<jacoco.min.branchcoverage>0.75</jacoco.min.branchcoverage>
41+
<jacoco.min.linecoverage>0.83</jacoco.min.linecoverage>
42+
<jacoco.min.branchcoverage>0.73</jacoco.min.branchcoverage>
4343
<javaModulesSurefireArgLine>
4444
--add-opens io.clientcore.http.netty4/io.clientcore.http.netty4=ALL-UNNAMED
4545
--add-opens io.clientcore.http.netty4/io.clientcore.http.netty4.implementation=ALL-UNNAMED

sdk/clientcore/http-netty4/spotbugs-exclude.xml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,20 @@
3030
<Bug pattern="OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE" />
3131
<Class name="io.clientcore.http.netty4.NettyHttpClientTests" />
3232
</Match>
33+
<Match>
34+
<Bug pattern="OS_OPEN_STREAM" />
35+
<Class name="io.clientcore.http.netty4.implementation.Netty4ChannelInputStreamTests" />
36+
</Match>
3337
<Match>
3438
<Bug pattern="OS_OPEN_STREAM_EXCEPTION_PATH" />
3539
<Class name="io.clientcore.http.netty4.NettyHttpClientTests" />
3640
</Match>
3741
<Match>
3842
<Bug pattern="RR_NOT_CHECKED" />
39-
<Class name="io.clientcore.http.netty4.implementation.HttpResponseDrainsBufferTests" />
43+
<Or>
44+
<Class name="io.clientcore.http.netty4.implementation.HttpResponseDrainsBufferTests" />
45+
<Class name="io.clientcore.http.netty4.implementation.Netty4ChannelInputStreamTests" />
46+
</Or>
4047
</Match>
4148
<Match>
4249
<Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON" />

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/NettyHttpClient.java

Lines changed: 114 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515
import io.clientcore.core.models.binarydata.FileBinaryData;
1616
import io.clientcore.core.models.binarydata.InputStreamBinaryData;
1717
import io.clientcore.core.utils.AuthenticateChallenge;
18+
import io.clientcore.core.utils.CoreUtils;
1819
import io.clientcore.core.utils.ProgressReporter;
1920
import io.clientcore.core.utils.ServerSentEventUtils;
2021
import io.clientcore.http.netty4.implementation.ChannelInitializationProxyHandler;
22+
import io.clientcore.http.netty4.implementation.Netty4ChannelBinaryData;
23+
import io.clientcore.http.netty4.implementation.Netty4EagerConsumeChannelHandler;
2124
import io.clientcore.http.netty4.implementation.Netty4ProgressAndTimeoutHandler;
2225
import io.clientcore.http.netty4.implementation.Netty4ResponseHandler;
2326
import io.clientcore.http.netty4.implementation.Netty4SslInitializationHandler;
27+
import io.clientcore.http.netty4.implementation.ResponseBodyHandling;
28+
import io.clientcore.http.netty4.implementation.ResponseStateInfo;
2429
import io.clientcore.http.netty4.implementation.WrappedHttpHeaders;
2530
import io.netty.bootstrap.Bootstrap;
2631
import io.netty.buffer.ByteBuf;
@@ -43,6 +48,7 @@
4348
import io.netty.handler.stream.ChunkedWriteHandler;
4449

4550
import javax.net.ssl.SSLException;
51+
import java.io.ByteArrayOutputStream;
4652
import java.io.IOException;
4753
import java.net.URI;
4854
import java.nio.channels.FileChannel;
@@ -54,6 +60,7 @@
5460
import static io.clientcore.core.utils.ServerSentEventUtils.attemptRetry;
5561
import static io.clientcore.core.utils.ServerSentEventUtils.processTextEventStream;
5662
import static io.clientcore.http.netty4.implementation.Netty4Utility.PROGRESS_AND_TIMEOUT_HANDLER_NAME;
63+
import static io.clientcore.http.netty4.implementation.Netty4Utility.awaitLatch;
5764
import static io.clientcore.http.netty4.implementation.Netty4Utility.createCodec;
5865
import static io.clientcore.http.netty4.implementation.Netty4Utility.setOrSuppressError;
5966
import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.trailersFactory;
@@ -106,7 +113,7 @@ public Response<BinaryData> send(HttpRequest request) {
106113
boolean addProgressAndTimeoutHandler
107114
= progressReporter != null || writeTimeoutMillis > 0 || responseTimeoutMillis > 0 || readTimeoutMillis > 0;
108115

109-
AtomicReference<Response<BinaryData>> responseReference = new AtomicReference<>();
116+
AtomicReference<ResponseStateInfo> responseReference = new AtomicReference<>();
110117
AtomicReference<Throwable> errorReference = new AtomicReference<>();
111118
CountDownLatch latch = new CountDownLatch(1);
112119

@@ -143,67 +150,123 @@ protected void initChannel(Channel ch) throws SSLException {
143150
}
144151
});
145152

146-
try {
147-
bootstrap.connect(host, port).addListener((ChannelFutureListener) connectListener -> {
148-
if (!connectListener.isSuccess()) {
149-
LOGGER.atError().setThrowable(connectListener.cause()).log("Failed to send request");
150-
errorReference.set(connectListener.cause());
151-
connectListener.channel().close();
152-
latch.countDown();
153-
return;
153+
bootstrap.connect(host, port).addListener((ChannelFutureListener) connectListener -> {
154+
if (!connectListener.isSuccess()) {
155+
LOGGER.atError().setThrowable(connectListener.cause()).log("Failed to send request");
156+
errorReference.set(connectListener.cause());
157+
connectListener.channel().close();
158+
latch.countDown();
159+
return;
160+
}
161+
162+
Channel channel = connectListener.channel();
163+
channel.closeFuture().addListener(closeListener -> {
164+
if (!closeListener.isSuccess()) {
165+
LOGGER.atError().setThrowable(closeListener.cause()).log("Channel closed with error");
166+
setOrSuppressError(errorReference, closeListener.cause());
154167
}
168+
});
169+
170+
// Only add CoreProgressAndTimeoutHandler if it will do anything, ex it is reporting progress or is
171+
// applying timeouts.
172+
// This is done to keep the ChannelPipeline shorter, therefore more performant, if this would
173+
// effectively be a no-op.
174+
if (addProgressAndTimeoutHandler) {
175+
channel.pipeline()
176+
.addLast(PROGRESS_AND_TIMEOUT_HANDLER_NAME, new Netty4ProgressAndTimeoutHandler(progressReporter,
177+
writeTimeoutMillis, responseTimeoutMillis, readTimeoutMillis));
178+
}
179+
180+
Netty4ResponseHandler responseHandler
181+
= new Netty4ResponseHandler(request, responseReference, errorReference, latch);
182+
channel.pipeline().addLast(responseHandler);
155183

156-
Channel channel = connectListener.channel();
157-
channel.closeFuture().addListener(closeListener -> {
158-
if (!closeListener.isSuccess()) {
159-
LOGGER.atError().setThrowable(closeListener.cause()).log("Channel closed with error");
160-
setOrSuppressError(errorReference, closeListener.cause());
184+
Throwable earlyError = errorReference.get();
185+
if (earlyError != null) {
186+
// If an error occurred between the connect and the request being sent, don't proceed with sending
187+
// the request.
188+
latch.countDown();
189+
return;
190+
}
191+
192+
sendRequest(request, channel, addProgressAndTimeoutHandler, errorReference)
193+
.addListener((ChannelFutureListener) sendListener -> {
194+
if (!sendListener.isSuccess()) {
195+
setOrSuppressError(errorReference, sendListener.cause());
196+
sendListener.channel().close();
197+
latch.countDown();
198+
} else {
199+
sendListener.channel().read();
161200
}
162201
});
202+
});
163203

164-
// Only add CoreProgressAndTimeoutHandler if it will do anything, ex it is reporting progress or is
165-
// applying timeouts.
166-
// This is done to keep the ChannelPipeline shorter, therefore more performant, if this would
167-
// effectively be a no-op.
168-
if (addProgressAndTimeoutHandler) {
169-
channel.pipeline()
170-
.addLast(PROGRESS_AND_TIMEOUT_HANDLER_NAME, new Netty4ProgressAndTimeoutHandler(
171-
progressReporter, writeTimeoutMillis, responseTimeoutMillis, readTimeoutMillis));
172-
}
204+
awaitLatch(latch);
205+
206+
ResponseStateInfo info = responseReference.get();
207+
if (info == null) {
208+
throw LOGGER.throwableAtError().log(errorReference.get(), CoreException::from);
209+
}
173210

174-
Netty4ResponseHandler responseHandler
175-
= new Netty4ResponseHandler(request, responseReference, errorReference, latch);
176-
channel.pipeline().addLast(responseHandler);
211+
Response<BinaryData> response;
212+
if (info.isChannelConsumptionComplete()) {
213+
// The network response is already complete, handle creating our Response based on the request method and
214+
// response headers.
215+
BinaryData body = BinaryData.empty();
216+
ByteArrayOutputStream eagerContent = info.getEagerContent();
217+
if (info.getResponseBodyHandling() != ResponseBodyHandling.IGNORE && eagerContent.size() > 0) {
218+
// Set the response body as the first HttpContent received if the request wasn't a HEAD request and
219+
// there was body content.
220+
body = BinaryData.fromBytes(eagerContent.toByteArray());
221+
}
177222

178-
Throwable earlyError = errorReference.get();
179-
if (earlyError != null) {
180-
// If an error occurred between the connect and the request being sent, don't proceed with sending
181-
// the request.
182-
latch.countDown();
183-
return;
223+
response = new Response<>(request, info.getStatusCode(), info.getHeaders(), body);
224+
} else {
225+
// Otherwise we aren't finished, handle the remaining content according to the documentation in
226+
// 'channelRead()'.
227+
BinaryData body = BinaryData.empty();
228+
ResponseBodyHandling bodyHandling = info.getResponseBodyHandling();
229+
Channel channel = info.getResponseChannel();
230+
if (bodyHandling == ResponseBodyHandling.IGNORE) {
231+
// We're ignoring the response content.
232+
CountDownLatch drainLatch = new CountDownLatch(1);
233+
channel.pipeline().addLast(new Netty4EagerConsumeChannelHandler(drainLatch, ignored -> {
234+
}));
235+
channel.config().setAutoRead(true);
236+
awaitLatch(drainLatch);
237+
} else if (bodyHandling == ResponseBodyHandling.STREAM) {
238+
// Body streaming uses a special BinaryData that tracks the firstContent read and the Channel it came
239+
// from so it can be consumed when the BinaryData is being used.
240+
// autoRead should have been disabled already but lets make sure that it is.
241+
channel.config().setAutoRead(false);
242+
String contentLength = info.getHeaders().getValue(HttpHeaderName.CONTENT_LENGTH);
243+
Long length = null;
244+
if (!CoreUtils.isNullOrEmpty(contentLength)) {
245+
try {
246+
length = Long.parseLong(contentLength);
247+
} catch (NumberFormatException ignored) {
248+
// Ignore, we'll just read until the channel is closed.
249+
}
184250
}
185251

186-
sendRequest(request, channel, addProgressAndTimeoutHandler, errorReference)
187-
.addListener((ChannelFutureListener) sendListener -> {
188-
if (!sendListener.isSuccess()) {
189-
setOrSuppressError(errorReference, sendListener.cause());
190-
sendListener.channel().close();
191-
latch.countDown();
192-
} else {
193-
sendListener.channel().read();
194-
}
195-
});
196-
});
252+
body = new Netty4ChannelBinaryData(info.getEagerContent(), channel, length);
253+
} else {
254+
// All cases otherwise assume BUFFER.
255+
CountDownLatch drainLatch = new CountDownLatch(1);
256+
channel.pipeline().addLast(new Netty4EagerConsumeChannelHandler(drainLatch, buf -> {
257+
try {
258+
buf.readBytes(info.getEagerContent(), buf.readableBytes());
259+
} catch (IOException ex) {
260+
throw LOGGER.throwableAtError().log(ex, CoreException::from);
261+
}
262+
}));
263+
channel.config().setAutoRead(true);
264+
awaitLatch(drainLatch);
197265

198-
latch.await();
199-
} catch (InterruptedException e) {
200-
Thread.currentThread().interrupt();
201-
throw LOGGER.throwableAtError().log("Request interrupted.", e, CoreException::from);
202-
}
266+
body = BinaryData.fromBytes(info.getEagerContent().toByteArray());
267+
}
203268

204-
Response<BinaryData> response = responseReference.get();
205-
if (response == null) {
206-
throw LOGGER.throwableAtError().log(errorReference.get(), CoreException::from);
269+
response = new Response<>(request, info.getStatusCode(), info.getHeaders(), body);
207270
}
208271

209272
if (response.getValue() != BinaryData.empty()

sdk/clientcore/http-netty4/src/main/java/io/clientcore/http/netty4/NettyHttpClientBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ static EventLoopGroup getEventLoopGroupToUse(EventLoopGroup configuredGroup,
319319
return configuredGroup;
320320
}
321321

322-
ThreadFactory threadFactory = new DefaultThreadFactory("clientcore-netty-client");
322+
ThreadFactory threadFactory = new DefaultThreadFactory("clientcore-netty-client", true);
323323

324324
// Use EpollEventLoopGroup if Epoll is available and 'channelClass' wasn't configured or was configured to
325325
// EpollSocketChannel.

0 commit comments

Comments
 (0)