> On Oct. 23, 2015, 8:02 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-log4j/src/main/java/org/apache/samza/logging/log4j/StreamAppender.java, > > line 205 > > <https://reviews.apache.org/r/39464/diff/1/?file=1101368#file1101368line205> > > > > 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.
I agree. We only need certain configuration to be visible for logging to work. We can do this by adding the logging related configuration in the run-am.sh commandline or as a part of environmental variable and access it in the log appenders. This way we don't need any job coordinator logic in the appenders. Created a JIRA to track this - https://issues.apache.org/jira/browse/SAMZA-800 - Navina ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39464/#review103838 ----------------------------------------------------------- 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 > >