Skip to content

Commit e48e627

Browse files
author
David Noble
authored
[BUG] Cosmos | Direct TCP | Address null pointer exception in RntbdClientChannelPool.computeLoadFactor (#12339)
* Port from v4 * Corrected package misspelling in log4j.properties and removed System.exit from Main.java * Responded to code review comments * Updated sdk/cosmos/README.md with info on using system properties to modify default Direct TCP options * Addresses a NullPointerException raised when an available channel closes since it became available, improves two log messages, and increases the value of maxChannelsPerEndpoint from 10 to 130. The latter is based on performance runs that indicate this is a better value in syncrhonous and asynchronous point read scenarios * Tweaked logger message * Removed README.md because it was deleted on master and was not removed when I pulled from master
1 parent 6eeb0f1 commit e48e627

File tree

4 files changed

+19
-7
lines changed

4 files changed

+19
-7
lines changed

sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/DirectConnectionConfig.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ public final class DirectConnectionConfig {
1818
private static final Duration DEFAULT_IDLE_ENDPOINT_TIMEOUT = Duration.ofSeconds(70L);
1919
private static final Duration DEFAULT_CONNECT_TIMEOUT = Duration.ofSeconds(60L);
2020
private static final Duration DEFAULT_REQUEST_TIMEOUT = Duration.ofSeconds(5L);
21-
private static final int DEFAULT_MAX_CONNECTIONS_PER_ENDPOINT = 30;
22-
private static final int DEFAULT_MAX_REQUESTS_PER_CONNECTION = 10;
21+
private static final int DEFAULT_MAX_CONNECTIONS_PER_ENDPOINT = 130;
22+
private static final int DEFAULT_MAX_REQUESTS_PER_CONNECTION = 30;
2323

2424
private Duration connectTimeout;
2525
private Duration idleConnectionTimeout;
@@ -29,7 +29,7 @@ public final class DirectConnectionConfig {
2929
private int maxRequestsPerConnection;
3030

3131
/**
32-
* Constructor.
32+
* Constructor
3333
*/
3434
public DirectConnectionConfig() {
3535
this.connectTimeout = DEFAULT_CONNECT_TIMEOUT;

sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/RntbdTransportClient.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import com.azure.cosmos.implementation.directconnectivity.rntbd.RntbdRequestRecord;
1616
import com.azure.cosmos.implementation.directconnectivity.rntbd.RntbdServiceEndpoint;
1717
import com.azure.cosmos.implementation.guava25.base.Strings;
18+
import com.fasterxml.jackson.annotation.JsonCreator;
1819
import com.fasterxml.jackson.annotation.JsonIgnore;
1920
import com.fasterxml.jackson.annotation.JsonProperty;
2021
import com.fasterxml.jackson.core.JsonGenerator;
@@ -221,6 +222,11 @@ public static final class Options {
221222

222223
// region Constructors
223224

225+
@JsonCreator
226+
private Options() {
227+
this(ConnectionPolicy.getDefaultPolicy());
228+
}
229+
224230
private Options(final Builder builder) {
225231

226232
this.bufferPageSize = builder.bufferPageSize;

sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdClientChannelPool.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,12 @@ private double computeLoadFactor() {
623623
for (Channel channel : this.availableChannels) {
624624

625625
final RntbdRequestManager manager = channel.pipeline().get(RntbdRequestManager.class);
626+
627+
if (manager == null) {
628+
logger.debug("Channel({}) connection lost", channel);
629+
continue;
630+
}
631+
626632
final long pendingRequestCount = manager.pendingRequestCount();
627633

628634
if (pendingRequestCount < pendingRequestCountMin) {

sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdServiceEndpoint.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import com.azure.cosmos.implementation.GoneException;
88
import com.azure.cosmos.implementation.directconnectivity.RntbdTransportClient;
99
import com.azure.cosmos.implementation.guava25.collect.ImmutableMap;
10-
import com.azure.cosmos.implementation.guava27.Strings;
1110
import com.fasterxml.jackson.core.JsonGenerator;
1211
import com.fasterxml.jackson.databind.SerializerProvider;
1312
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
@@ -40,6 +39,7 @@
4039
import static com.azure.cosmos.implementation.directconnectivity.RntbdTransportClient.Options;
4140
import static com.azure.cosmos.implementation.guava25.base.Preconditions.checkNotNull;
4241
import static com.azure.cosmos.implementation.guava25.base.Preconditions.checkState;
42+
import static com.azure.cosmos.implementation.guava27.Strings.lenientFormat;
4343
import static java.util.concurrent.TimeUnit.NANOSECONDS;
4444

4545
@JsonSerialize(using = RntbdServiceEndpoint.JsonSerializer.class)
@@ -261,11 +261,11 @@ private RntbdRequestRecord writeWhenConnected(
261261
} else {
262262

263263
logger.debug("\n [{}]\n {}\n write failed due to {} ", this, requestArgs, cause);
264-
final String reason = cause.getMessage();
264+
final String reason = cause.toString();
265265

266266
final GoneException goneException = new GoneException(
267-
Strings.lenientFormat("failed to establish connection to %s: %s", this.remoteAddress, reason),
268-
cause instanceof Exception ? (Exception)cause : new IOException(reason, cause),
267+
lenientFormat("failed to establish connection to %s due to %s", this.remoteAddress, reason),
268+
cause instanceof Exception ? (Exception) cause : new IOException(reason, cause),
269269
ImmutableMap.of(HttpHeaders.ACTIVITY_ID, activityId.toString()),
270270
requestArgs.replicaPath()
271271
);

0 commit comments

Comments
 (0)