----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39464/#review103838 -----------------------------------------------------------
Ship it! LGTM. Thanks! samza-log4j/src/main/java/org/apache/samza/logging/log4j/StreamAppender.java (line 179) <https://reviews.apache.org/r/39464/#comment161936> This is also one of my concern when I was debugging the coordinator HTTP request timeout issue. Wouldn't it be better if the StreamAppender was given the config instead of embedding the code here to grab the configuration? Then, we don't even need this if-else condition here, since the caller should be aware of the process context this getConfig is called and supply the cooresponding config differently. However, this could be a later code cleanup task. - Yi Pan (Data Infrastructure) On Oct. 23, 2015, 6:33 p.m., Navina Ramesh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39464/ > ----------------------------------------------------------- > > (Updated Oct. 23, 2015, 6:33 p.m.) > > > Review request for samza, Yan Fang, Chris Riccomini, Jake Maes, Jagadish > Venkatraman, and Yi Pan (Data Infrastructure). > > > Bugs: SAMZA-723 > https://issues.apache.org/jira/browse/SAMZA-723 > > > Repository: samza > > > Description > ------- > > fixed 2 parts of the problem: > 1. Start streamAppender until the JobCoordinator is running > 2. deadlock in Producer thread and the main thread > > More explanation is in JIRA. > > > Diffs > ----- > > samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala > a926ce639dbf6776ebfb1d885621f4b6bf5f5aa5 > samza-log4j/src/main/java/org/apache/samza/config/Log4jSystemConfig.java > d98b8c658dd8ae4d952a1004a796d64c229012a7 > > samza-log4j/src/main/java/org/apache/samza/logging/log4j/StreamAppender.java > 776a36bd539f747d440a65f844cfcede52625e1b > > samza-log4j/src/test/java/org/apache/samza/logging/log4j/TestStreamAppender.java > 1c6f9a48590d55fe808940adba72415c3ab9614e > > Diff: https://reviews.apache.org/r/39464/diff/ > > > Testing > ------- > > Verified with hello-samza. > Still investigating to understand the original cause and the fix > > > Thanks, > > Navina Ramesh > >
