(pinot) branch master updated: switch log.debug to log.info on query request (#15264)

[gortiz]

This is an automated email from the ASF dual-hosted git repository.

gortiz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 7faa906a3b switch log.debug to log.info on query request (#15264)
7faa906a3b is described below

commit 7faa906a3b25504a8fe897b717b41420c35849d6
Author: Alberto Bastos <alberto.var...@startree.ai>
AuthorDate: Thu Mar 27 10:38:33 2025 +0100

    switch log.debug to log.info on query request (#15264)
---
 .../apache/pinot/broker/querylog/QueryLogger.java  | 30 +++++++++++++++++++---
 .../BaseSingleStageBrokerRequestHandler.java       |  2 +-
 .../MultiStageBrokerRequestHandler.java            |  2 +-
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git 
a/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java 
b/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
index 9be07b172c..6890974346 100644
--- 
a/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
+++ 
b/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
@@ -74,11 +74,20 @@ public class QueryLogger {
     _droppedLogRateLimiter = droppedLogRateLimiter;
   }
 
+  public void log(long requestId, String query) {
+    if (!checkRateLimiter(null)) {
+      return;
+    }
+
+    _logger.info("SQL query for request {}: {}", requestId, query);
+
+    tryLogDropped();
+  }
+
   public void log(QueryLogParams params) {
     _logger.debug("Broker Response: {}", params._response);
 
-    if (!(_logRateLimiter.tryAcquire() || shouldForceLog(params))) {
-      _numDroppedLogs.incrementAndGet();
+    if (!checkRateLimiter(params)) {
       return;
     }
 
@@ -93,6 +102,18 @@ public class QueryLogger {
         .append(StringUtils.substring(params._requestContext.getQuery(), 0, 
_maxQueryLengthToLog));
     _logger.info(queryLogBuilder.toString());
 
+    tryLogDropped();
+  }
+
+  private boolean checkRateLimiter(@Nullable QueryLogParams params) {
+    boolean allowed = _logRateLimiter.tryAcquire() || shouldForceLog(params);
+    if (!allowed) {
+      _numDroppedLogs.incrementAndGet();
+    }
+    return allowed;
+  }
+
+  private void tryLogDropped() {
     if (_droppedLogRateLimiter.tryAcquire()) {
       // use getAndSet to 0 so that there will be no race condition between
       // loggers that increment this counter and this thread
@@ -112,7 +133,10 @@ public class QueryLogger {
     return _logRateLimiter.getRate();
   }
 
-  private boolean shouldForceLog(QueryLogParams params) {
+  private boolean shouldForceLog(@Nullable QueryLogParams params) {
+    if (params == null) {
+      return false;
+    }
     return params._response.isPartialResult() || 
params._response.getTimeUsedMs() > TimeUnit.SECONDS.toMillis(1);
   }
 
diff --git 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
index 14e75b24aa..97510585b8 100644
--- 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
+++ 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
@@ -300,7 +300,7 @@ public abstract class BaseSingleStageBrokerRequestHandler 
extends BaseBrokerRequ
       JsonNode request, @Nullable RequesterIdentity requesterIdentity, 
RequestContext requestContext,
       @Nullable HttpHeaders httpHeaders, AccessControl accessControl)
       throws Exception {
-    LOGGER.debug("SQL query for request {}: {}", requestId, query);
+    _queryLogger.log(requestId, query);
 
     //Start instrumentation context. This must not be moved further below 
interspersed into the code.
     Tracing.ThreadAccountantOps.setupRunner(String.valueOf(requestId));
diff --git 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
index c329d075bc..2dedf7b244 100644
--- 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
+++ 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
@@ -248,7 +248,7 @@ public class MultiStageBrokerRequestHandler extends 
BaseBrokerRequestHandler {
   protected BrokerResponse handleRequestThrowing(long requestId, String query, 
SqlNodeAndOptions sqlNodeAndOptions,
       @Nullable RequesterIdentity requesterIdentity, RequestContext 
requestContext, HttpHeaders httpHeaders)
       throws QueryException, WebApplicationException {
-    LOGGER.debug("SQL query for request {}: {}", requestId, query);
+    _queryLogger.log(requestId, query);
 
     long queryTimeoutMs = getTimeout(sqlNodeAndOptions.getOptions());
     Timer queryTimer = new Timer(queryTimeoutMs, TimeUnit.MILLISECONDS);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org