Re: Review Request 44241: SAMZA-883 Improve logging for container handling and kafka refresh
> On March 2, 2016, 10:20 p.m., Boris Shkolnik wrote: > > Looks like this debug->info conversions are mainly for container > > allocations. So it should not add too much noise and should be quite > > helpfull. > > Navina Ramesh wrote: > @Boris: This change has been discarded. Please check the new RB here - > https://reviews.apache.org/r/44293 > > Container allocations can add a LOT of noise. I was just going to comment > on it in the RB. @navina I'll look for the comment on the other RB, but I didn't see this. The container allocations are only printed while there are outstanding requests. In the normal case, thats on the order of the number of containers only at the beginning of the job. And when there are problems, you probably care about this information even more. Also, I firmly believe these messages are among the most important things the AM log should contain, and therefore they should ALWAYS be there. - Jake --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44241/#review121736 --- On March 2, 2016, 2:24 a.m., Jake Maes wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44241/ > --- > > (Updated March 2, 2016, 2:24 a.m.) > > > Review request for samza, Jagadish Venkatraman and Yi Pan (Data > Infrastructure). > > > Repository: samza > > > Description > --- > > SAMZA-883 Improve logging for container handling and kafka refresh > > > Diffs > - > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/BrokerProxy.scala > 9aa98184f8ab186eff226482f894e8842b9525e5 > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala > b37375360bcad8e089d95e8e3da63a1b9aab > > samza-kafka/src/main/scala/org/apache/samza/util/ClientUtilTopicMetadataStore.scala > 0f91622c0c4c270a376f8b91b3e785cca57bc9dd > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java > 54db5e5b2b1b109d202e814809adbfd2bc84fb4b > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerUtil.java > 91fae98c074e1648e7168fb8e76d6e1e656816fc > > Diff: https://reviews.apache.org/r/44241/diff/ > > > Testing > --- > > Manual testing and log verification (made sure they weren't verbose) > > > In case there is some concern over the messages I switched from debug to > info. I let a job run for 4 hours: > NUM_OCCURRENCES | Message | Notes > 2 | "ClientUtilTopicMetadataStore [INFO] Fetching topic metadata." | Same > occurrences as the ClientUtil INFO log message from Kafka > 33 | "ContainerRequestState [INFO] Got a new container: ..." | Approx equal > to the number of containers in the normal case. All other logs updated in > ContainerRequestState will be of similar or lower frequency. > > > Thanks, > > Jake Maes > >
Re: Review Request 44241: SAMZA-883 Improve logging for container handling and kafka refresh
> On March 2, 2016, 10:20 p.m., Boris Shkolnik wrote: > > Looks like this debug->info conversions are mainly for container > > allocations. So it should not add too much noise and should be quite > > helpfull. @Boris: This change has been discarded. Please check the new RB here - https://reviews.apache.org/r/44293 Container allocations can add a LOT of noise. I was just going to comment on it in the RB. - Navina --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44241/#review121736 --- On March 2, 2016, 2:24 a.m., Jake Maes wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44241/ > --- > > (Updated March 2, 2016, 2:24 a.m.) > > > Review request for samza, Jagadish Venkatraman and Yi Pan (Data > Infrastructure). > > > Repository: samza > > > Description > --- > > SAMZA-883 Improve logging for container handling and kafka refresh > > > Diffs > - > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/BrokerProxy.scala > 9aa98184f8ab186eff226482f894e8842b9525e5 > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala > b37375360bcad8e089d95e8e3da63a1b9aab > > samza-kafka/src/main/scala/org/apache/samza/util/ClientUtilTopicMetadataStore.scala > 0f91622c0c4c270a376f8b91b3e785cca57bc9dd > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java > 54db5e5b2b1b109d202e814809adbfd2bc84fb4b > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerUtil.java > 91fae98c074e1648e7168fb8e76d6e1e656816fc > > Diff: https://reviews.apache.org/r/44241/diff/ > > > Testing > --- > > Manual testing and log verification (made sure they weren't verbose) > > > In case there is some concern over the messages I switched from debug to > info. I let a job run for 4 hours: > NUM_OCCURRENCES | Message | Notes > 2 | "ClientUtilTopicMetadataStore [INFO] Fetching topic metadata." | Same > occurrences as the ClientUtil INFO log message from Kafka > 33 | "ContainerRequestState [INFO] Got a new container: ..." | Approx equal > to the number of containers in the normal case. All other logs updated in > ContainerRequestState will be of similar or lower frequency. > > > Thanks, > > Jake Maes > >
Re: Review Request 44241: SAMZA-883 Improve logging for container handling and kafka refresh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44241/#review121736 --- Ship it! Looks like this debug->info conversions are mainly for container allocations. So it should not add too much noise and should be quite helpfull. - Boris Shkolnik On March 2, 2016, 2:24 a.m., Jake Maes wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44241/ > --- > > (Updated March 2, 2016, 2:24 a.m.) > > > Review request for samza, Jagadish Venkatraman and Yi Pan (Data > Infrastructure). > > > Repository: samza > > > Description > --- > > SAMZA-883 Improve logging for container handling and kafka refresh > > > Diffs > - > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/BrokerProxy.scala > 9aa98184f8ab186eff226482f894e8842b9525e5 > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala > b37375360bcad8e089d95e8e3da63a1b9aab > > samza-kafka/src/main/scala/org/apache/samza/util/ClientUtilTopicMetadataStore.scala > 0f91622c0c4c270a376f8b91b3e785cca57bc9dd > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java > 54db5e5b2b1b109d202e814809adbfd2bc84fb4b > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerUtil.java > 91fae98c074e1648e7168fb8e76d6e1e656816fc > > Diff: https://reviews.apache.org/r/44241/diff/ > > > Testing > --- > > Manual testing and log verification (made sure they weren't verbose) > > > In case there is some concern over the messages I switched from debug to > info. I let a job run for 4 hours: > NUM_OCCURRENCES | Message | Notes > 2 | "ClientUtilTopicMetadataStore [INFO] Fetching topic metadata." | Same > occurrences as the ClientUtil INFO log message from Kafka > 33 | "ContainerRequestState [INFO] Got a new container: ..." | Approx equal > to the number of containers in the normal case. All other logs updated in > ContainerRequestState will be of similar or lower frequency. > > > Thanks, > > Jake Maes > >
Re: Review Request 44241: SAMZA-883 Improve logging for container handling and kafka refresh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44241/ --- (Updated March 2, 2016, 2:24 a.m.) Review request for samza, Jagadish Venkatraman and Yi Pan (Data Infrastructure). Repository: samza Description --- SAMZA-883 Improve logging for container handling and kafka refresh Diffs - samza-kafka/src/main/scala/org/apache/samza/system/kafka/BrokerProxy.scala 9aa98184f8ab186eff226482f894e8842b9525e5 samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e089d95e8e3da63a1b9aab samza-kafka/src/main/scala/org/apache/samza/util/ClientUtilTopicMetadataStore.scala 0f91622c0c4c270a376f8b91b3e785cca57bc9dd samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java 54db5e5b2b1b109d202e814809adbfd2bc84fb4b samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerUtil.java 91fae98c074e1648e7168fb8e76d6e1e656816fc Diff: https://reviews.apache.org/r/44241/diff/ Testing (updated) --- Manual testing and log verification (made sure they weren't verbose) In case there is some concern over the messages I switched from debug to info. I let a job run for 4 hours: NUM_OCCURRENCES | Message | Notes 2 | "ClientUtilTopicMetadataStore [INFO] Fetching topic metadata." | Same occurrences as the ClientUtil INFO log message from Kafka 33 | "ContainerRequestState [INFO] Got a new container: ..." | Approx equal to the number of containers in the normal case. All other logs updated in ContainerRequestState will be of similar or lower frequency. Thanks, Jake Maes
Re: Review Request 44241: SAMZA-883 Improve logging for container handling and kafka refresh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44241/ --- (Updated March 2, 2016, 2:16 a.m.) Review request for samza, Jagadish Venkatraman and Yi Pan (Data Infrastructure). Changes --- Missed a commit with an import. Fixed now. Repository: samza Description --- SAMZA-883 Improve logging for container handling and kafka refresh Diffs (updated) - samza-kafka/src/main/scala/org/apache/samza/system/kafka/BrokerProxy.scala 9aa98184f8ab186eff226482f894e8842b9525e5 samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e089d95e8e3da63a1b9aab samza-kafka/src/main/scala/org/apache/samza/util/ClientUtilTopicMetadataStore.scala 0f91622c0c4c270a376f8b91b3e785cca57bc9dd samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java 54db5e5b2b1b109d202e814809adbfd2bc84fb4b samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerUtil.java 91fae98c074e1648e7168fb8e76d6e1e656816fc Diff: https://reviews.apache.org/r/44241/diff/ Testing --- Manual testing and log verification (made sure they weren't verbose) Thanks, Jake Maes
