gortiz commented on PR #15264: URL: https://github.com/apache/pinot/pull/15264#issuecomment-2747803634
> We need to apply the same throttling as the QueryLogger does. This change can flood the log for super high QPS use cases (imagine over 10k query lines per second) I disagree. Not because we shouldn't throttle, but because throttling with QueryLogger is a bad practice. Logging and throttling logs is not something that should be done in the code. Instead, we should use the logging framework (in our case, log4j2) to throttle when needed. I understand we don't want to flood the logs with queries in high QPS scenarios, but that is a simplification. We don't want to flood our logs with anything in high QPS scenarios. For example, there are cases where queries start to fail (e.g., timeouts), and in that case, we flood the log with stack traces. We could create a class to prune logs when they are very spammy, but we would need to replicate the QueryLogger and ServerQueryLogger code everywhere. This is not something that scales very well. It is also easy to miss some parts of the code, which can be spammy. Instead, we should always log in the code and control whether the log should be printed or not (and where) using a log4j2 filter. I think [log4j2's Burst filter](https://logging.apache.org/log4j/2.12.x/manual/filters.html#BurstFilter) should be good enough for us, but in case we want something more fancy, we can even create our own log4j2 filters -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
