Skip to content

Commit cc102d9

Browse files
committed
JVMCBC-1664 ArrayIndexOutOfBoundsException when request takes more than 1 hour
Motivation ---------- When a request takes more than ~1 hour* (like a long-running Analytics query), AggregatingValueRecorder.recordValue() throws ArrayIndexOutOfBoundsException. This exception can mask an underlying cause, like request timeout or cancellation. * The actual threshold is closer to 1 hour, 13 minutes, 18 seconds. Modifications ------------- If the recorder throws an exception, log a warning instead of recording a value. Rejected Alternatives --------------------- Could initialize AggregatingValueRecorder.recorderStats with a `highestTrackableLatency` of several hours, but that would require allocating hundreds more histogram buckets, with an unknown impact on performance / resource usage. Change-Id: I7da205baa9b34cc06d4b3b797c8d8ae0471ab0ec Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/229979 Tested-by: Build Bot <[email protected]> Reviewed-by: Michael Reiche <[email protected]>
1 parent 45eadfc commit cc102d9

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed

core-io/src/main/java/com/couchbase/client/core/msg/RequestContext.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import com.couchbase.client.core.retry.RetryReason;
2929
import com.couchbase.client.core.topology.NodeIdentifier;
3030
import com.couchbase.client.core.util.HostAndPort;
31+
import org.slf4j.Logger;
32+
import org.slf4j.LoggerFactory;
3133
import reactor.util.annotation.Nullable;
3234

3335
import java.time.Duration;
@@ -51,6 +53,7 @@
5153
* @since 2.0.0
5254
*/
5355
public class RequestContext extends CoreContext {
56+
private static final Logger log = LoggerFactory.getLogger(RequestContext.class);
5457

5558
/**
5659
* Holds the last dispatch latency if set already (or at all).
@@ -239,7 +242,11 @@ public RequestContext logicallyComplete(@Nullable Throwable err) {
239242
if (!(environment().meter() instanceof NoopMeter)) {
240243
long latency = logicalRequestLatency();
241244
if (latency > 0) {
242-
core().responseMetric(request, err).recordValue(latency);
245+
try {
246+
core().responseMetric(request, err).recordValue(latency);
247+
} catch (Exception e) {
248+
log.warn("Failed to record request latency ({}). {}", Duration.ofNanos(latency), this, e);
249+
}
243250
}
244251
}
245252

core-io/src/main/java/com/couchbase/client/core/protostellar/ProtostellarRequest.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@
3131
import com.couchbase.client.core.retry.RetryReason;
3232
import com.couchbase.client.core.retry.RetryStrategy;
3333
import com.couchbase.client.core.service.ServiceType;
34+
import org.slf4j.Logger;
35+
import org.slf4j.LoggerFactory;
3436
import reactor.util.annotation.Nullable;
3537

3638
import java.time.Duration;
37-
import java.util.Collections;
3839
import java.util.HashMap;
3940
import java.util.HashSet;
4041
import java.util.Map;
@@ -49,6 +50,8 @@
4950
*/
5051
@Stability.Internal
5152
public class ProtostellarRequest<TGrpcRequest> {
53+
private static final Logger log = LoggerFactory.getLogger(ProtostellarRequest.class);
54+
5255
private final CoreProtostellar core;
5356
private final @Nullable RequestSpan span;
5457
private final long absoluteTimeout;
@@ -151,7 +154,11 @@ public void raisedResponseToUser(@Nullable Throwable err) {
151154
long latency = logicalRequestLatency();
152155
if (latency > 0) {
153156
Core.ResponseMetricIdentifier rmi = new Core.ResponseMetricIdentifier(serviceType.id(), requestName);
154-
core.responseMetric(rmi).recordValue(latency);
157+
try {
158+
core.responseMetric(rmi).recordValue(latency);
159+
} catch (Exception e) {
160+
log.warn("Failed to record request latency ({}). {}", Duration.ofNanos(latency), context(), e);
161+
}
155162
}
156163
}
157164
}

0 commit comments

Comments
 (0)