absurdfarce commented on code in PR #1743:
URL:
https://github.com/apache/cassandra-java-driver/pull/1743#discussion_r1750812556
##########
core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java:
##########
@@ -274,40 +279,19 @@ protected boolean isBusy(@NonNull Node node, @NonNull
Session session) {
}
protected boolean isResponseRateInsufficient(@NonNull Node node, long now) {
- // response rate is considered insufficient when less than 2 responses
were obtained in
- // the past interval delimited by RESPONSE_COUNT_RESET_INTERVAL_NANOS.
- if (responseTimes.containsKey(node)) {
- AtomicLongArray array = responseTimes.get(node);
- if (array.length() == 2) {
- long threshold = now - RESPONSE_COUNT_RESET_INTERVAL_NANOS;
- long leastRecent = array.get(0);
- return leastRecent - threshold < 0;
- }
- }
- return true;
+ NodeResponseRateSample sample = responseTimes.get(node);
+ return !(sample == null || sample.hasSufficientResponses(now));
}
+ /**
+ * Synchronously updates the response times for the given node. It is
synchronous because the
+ * {@link
#DefaultLoadBalancingPolicy(com.datastax.oss.driver.api.core.context.DriverContext,
+ * java.lang.String) CacheLoader.load} assigned is synchronous.
+ *
+ * @param node The node to update.
+ */
protected void updateResponseTimes(@NonNull Node node) {
- responseTimes.compute(
- node,
- (n, array) -> {
- // The array stores at most two timestamps, since we don't need more;
- // the first one is always the least recent one, and hence the one
to inspect.
- long now = nanoTime();
- if (array == null) {
- array = new AtomicLongArray(1);
- array.set(0, now);
- } else if (array.length() == 1) {
- long previous = array.get(0);
- array = new AtomicLongArray(2);
- array.set(0, previous);
- array.set(1, now);
- } else {
- array.set(0, array.get(1));
- array.set(1, now);
- }
- return array;
- });
+ this.responseTimes.compute(node, (k, v) -> v == null ? new
NodeResponseRateSample() : v.next());
Review Comment:
My expectation is that it wouldn't matter since we're creating (and GCing)
pretty small objects and the JVM usually handles that quite well but it's not
unreasonable to confirm that.
@SiyaoIsHiding is this something you can test? I suppose if we can
demonstrate roughly equivalent memory usage and throughput for a decent size
load that would be fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
