aratno commented on code in PR #1905:
URL:
https://github.com/apache/cassandra-java-driver/pull/1905#discussion_r1463708817
##########
core/src/main/java/com/datastax/oss/driver/internal/core/session/throttling/RateLimitingRequestThrottler.java:
##########
@@ -117,35 +117,37 @@ public RateLimitingRequestThrottler(DriverContext
context) {
}
@Override
- public void register(@NonNull Throttled request) {
- long now = clock.nanoTime();
- lock.lock();
- try {
- if (closed) {
- LOG.trace("[{}] Rejecting request after shutdown", logPrefix);
- fail(request, "The session is shutting down");
- } else if (queue.isEmpty() && acquire(now, 1) == 1) {
- LOG.trace("[{}] Starting newly registered request", logPrefix);
- request.onThrottleReady(false);
- } else if (queue.size() < maxQueueSize) {
- LOG.trace("[{}] Enqueuing request", logPrefix);
- if (queue.isEmpty()) {
- scheduler.schedule(this::drain, drainIntervalNanos,
TimeUnit.NANOSECONDS);
+ public void register(@NonNull Throttled request) {
+ long now = clock.nanoTime();
+ lock.lock();
+ try {
+ if (closed) {
+ LOG.trace("[{}] Rejecting request after shutdown", logPrefix);
+ fail(request, "The session is shutting down");
+ } else if (queue.isEmpty() && acquire(now, 1) == 1) {
+ LOG.trace("[{}] Starting newly registered request", logPrefix);
+ request.onThrottleReady(false);
+ } else if (queue.size() < maxQueueSize) {
+ LOG.trace("[{}] Enqueuing request", logPrefix);
+ if (queue.isEmpty()) {
+ scheduler.schedule(this::drain, drainIntervalNanos,
TimeUnit.NANOSECONDS);
+ }
+ queue.add(request);
+ } else {
+ LOG.trace("[{}] Rejecting request because of full queue",
logPrefix);
+ fail(
+ request,
+ String.format(
+ "The session has reached its maximum capacity "
+ + "(requests/s: %d, queue size: %d)",
+ maxRequestsPerSecond, maxQueueSize));
+ }
+ } catch (Exception e) {
+ fail(request, "Error in registering throttler");
Review Comment:
Currently, RateLimitingRequestThrottler includes:
```
private static void fail(Throttled request, String message) {
request.onThrottleFailure(new RequestThrottlingException(message));
}
```
I think it would be ideal to include a way to preserve the exception cause,
like this:
```
private static void fail(Throttled request, String message, Throwable
cause) {
request.onThrottleFailure(new RequestThrottlingException(message,
cause));
}
```
Then, the logs would include the chain of causes that led to the error.
Currently, there's very little information an operator could use to address the
error.
--
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]
