You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/07/14 23:03:46 UTC
[GitHub] [kafka] ableegoldman opened a new pull request #9024: [DO NOT MERGE] test Streams log4j properties
ableegoldman opened a new pull request #9024:
URL: https://github.com/apache/kafka/pull/9024
When a Streams test fails on a PR build, we only ever get the broker logs along with the stacktrace. This is pretty annoying, since it's near impossible to debug an integration test failure that can't be reproduced locally without the client logs
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman removed a comment on pull request #9024: [DO NOT MERGE] test Streams log4j properties
Posted by GitBox <gi...@apache.org>.
ableegoldman removed a comment on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658471043
It worked!! Results for the failed tests actually include the Streams logs 😭
[Unit test](https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/7405/testReport/junit/org.apache.kafka.streams/TestUnitTest/testTest/)
[Integration test](https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/7405/testReport/junit/org.apache.kafka.streams.integration/TestIntegrationTest/testTest/)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658886954
Retest this please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9024: [DO NOT MERGE] test Streams log4j properties
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658457813
Retest this please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #9024: MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#discussion_r454721300
##########
File path: streams/src/test/resources/log4j.properties
##########
@@ -18,4 +18,9 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=[%d] %p %m (%c:%L)%n
-log4j.logger.org.apache.kafka=INFO
+log4j.logger.kafka=WARN
Review comment:
We've been discussing this...the way I see it we have three options:
1) the current proposal (demote brokers to WARN)
2) just demote ZK to ERROR and see if that helps enough
3) do a finer grained control of the broker logs, ie demote some less helpful packages to WARN but leave others at INFO
I personally am fine with either 1 or 2 for now, as we can always go back and change it again if it's too little or too much. But I'd rather get fewer broker logs and some Streams logs than no Streams logs at all 🙂
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658886844
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-668756679
1 unrelated failure: `SaslPlaintextConsumerTest.testCoordinatorFailover`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman edited a comment on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman edited a comment on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-667447234
Ok I totally forgot about this PR.
@guozhangwang @mjsax Let's just merge the demotion of broker/zookeeper logs and leave streams logs at INFO for now. If that's not sufficient for useful debugging (eg the `EosBetaUpgradeIntegrationTest`) then I'll take another look at fixing the tests which fail when DEBUG logging is turned on.
I did take a brief look at the failing tests, it's not a timeout or excessive logs or anything that seems at all correlated to the log level. It fails on an assertion of the value of `MockProcessorSupplier#capturedProcessors` -- seems pretty bizarre to me 🤷♀️
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on a change in pull request #9024: MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#discussion_r454720486
##########
File path: streams/src/test/resources/log4j.properties
##########
@@ -18,4 +18,9 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=[%d] %p %m (%c:%L)%n
-log4j.logger.org.apache.kafka=INFO
+log4j.logger.kafka=WARN
Review comment:
Do we really want to degrade broker side logs to WARN?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax merged pull request #9024: MINOR: change Streams integration test log levels
Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #9024:
URL: https://github.com/apache/kafka/pull/9024
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #9024: MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#discussion_r455186990
##########
File path: streams/src/test/resources/log4j.properties
##########
@@ -18,4 +18,9 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=[%d] %p %m (%c:%L)%n
-log4j.logger.org.apache.kafka=INFO
+log4j.logger.kafka=WARN
Review comment:
Hm yeah I guess we should try setting everything to DEBUG. Just trying to find the sweet spot where we can actually get the logs that are useful
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-659747346
Well it looks like we still get too many logs from the brokers even at INFO level. I think we should just demote all broker logs to WARN and just bump it back up to INFO if really necessary
On the other hand, seems like turning DEBUG logs on for Streams caused over 100 tests to fail...not sure what that's about
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-668258427
SG. +1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-667447234
Ok I totally forgot about this PR.
Let's just merge the demotion of broker/zookeeper logs and leave streams logs at INFO for now. If that's not sufficient for debugging eg the `EosBetaUpgradeIntegrationTest` then I'll take another look at debugging the tests that fail when DEBUG logging is turned on.
I did take a brief look at the tests, it's not a timeout or excessive logs or anything that seems at all correlated to the log level. It fails on an assertion of the value of `MockProcessorSupplier#capturedProcessors` 🤷♀️
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #9024: MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658513250
@mjsax didn't work :/
Can you try again?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #9024: [DO NOT MERGE] test Streams log4j properties
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658471043
It worked!! Results for the failed tests actually include the Streams logs 😭
[Unit test](https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/7405/testReport/junit/org.apache.kafka.streams/TestUnitTest/testTest/)
[Integration test](https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/7405/testReport/junit/org.apache.kafka.streams.integration/TestIntegrationTest/testTest/)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on a change in pull request #9024: MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#discussion_r455153704
##########
File path: streams/src/test/resources/log4j.properties
##########
@@ -18,4 +18,9 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=[%d] %p %m (%c:%L)%n
-log4j.logger.org.apache.kafka=INFO
+log4j.logger.kafka=WARN
Review comment:
Got it, that makes sense.
Are there are particular Streams sub-packages we are interested in, or just `o.a.k.streams.integration` itself is good enough?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-668255117
Retest this please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #9024: [DO NOT MERGE (yet)] MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-659774428
@ableegoldman probably because the too much logs cause some of the operations cannot be completed within the timeout; let's demote brokers to WARN and I will trigger jenkins again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9024: MINOR: bump Streams integration test log level to DEBUG
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9024:
URL: https://github.com/apache/kafka/pull/9024#issuecomment-658484461
Retest this please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org