albertobastos commented on code in PR #15264:
URL: https://github.com/apache/pinot/pull/15264#discussion_r2007299497
##
pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java:
##
@@ -88,9 +87,12 @@ public void log(QueryLogParams params) {
queryLogBui
gortiz commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2743114456
@mayankshriv The issue is that we have a normal slf4j log and a QueryLogger,
which I'm not sure what is being used for. To me, it looks like an artifact for
configuring some logging for qu
Jackie-Jiang commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2744125431
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)
--
This is an
gortiz merged PR #15264:
URL: https://github.com/apache/pinot/pull/15264
--
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.apach
Jackie-Jiang commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2752422973
@gortiz I agree that ideally we should throttle the logs with a logging
framework, but people (e.g. LinkedIn) rely on QueryLogger currently to not
flood their log, and if we remove t
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
albertobastos commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2745198616
> 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 get t
gortiz commented on code in PR #15264:
URL: https://github.com/apache/pinot/pull/15264#discussion_r2007193302
##
pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java:
##
@@ -88,9 +87,12 @@ public void log(QueryLogParams params) {
queryLogBuilder.ap
albertobastos commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2728467575
I see your point @mayankshriv.
Ideally the sql query should only be logged the first time, and just use the
requestId to trace it back during the second log. Was afraid of doi
mayankshriv commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2725076231
Thanks, this will be very useful. Note though, if we log all query strings
twice, that will likely bloat the logs (queries can be very long strings). We
should have it such that the q
codecov-commenter commented on PR #15264:
URL: https://github.com/apache/pinot/pull/15264#issuecomment-2722011874
##
[Codecov](https://app.codecov.io/gh/apache/pinot/pull/15264?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&u
albertobastos opened a new pull request, #15264:
URL: https://github.com/apache/pinot/pull/15264
For observability purposes, all incoming queries will be now logged also at
the start of the request processing and not only after a response has been
obtained (or some failure occurred). This w
12 matches
Mail list logo