> On May 3, 2016, 5:23 p.m., Chris Pettitt wrote: > > Looks good. Mostly minor stuff that you can choose to take or ignore.
Appreciate the thoughtful comments. Implemented all of them. - Jake ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46856/#review131525 ----------------------------------------------------------- On May 4, 2016, 12:18 a.m., Jake Maes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46856/ > ----------------------------------------------------------- > > (Updated May 4, 2016, 12:18 a.m.) > > > Review request for samza, Boris Shkolnik, Navina Ramesh, Jagadish > Venkatraman, and Yi Pan (Data Infrastructure). > > > Bugs: SAMZA-943 > https://issues.apache.org/jira/browse/SAMZA-943 > > > Repository: samza > > > Description > ------- > > SAMZA-943 Occasional test failure: > TestStreamPartitionCountMonitor.testStartStopBehavior > > * Rewrote the monitor in Java following the pattern of the > PollingScanDiskSpaceMonitor in SAMZA-924 > ** The main difference is that it uses a ScheduledExecutorService to > cleanly run the monitor in a loop and provide determinism around startup and > shutdown > * Got rid of the sleep() in the unit test > * Added a unit test to verify the scheduler calls the monitor method > * Enforced that the monitor isn't restarted (which is a problem for the > scheduler service) > ** This required that the reference to the monitor not be static (defined > in the JobCoordinator object) and instead instantiated whenever the > JobCoordinator is instantiated. > > > Diffs > ----- > > checkstyle/import-control.xml c15b8e74de8e5aac5ac83278c52ab3dba1630e50 > > samza-core/src/main/java/org/apache/samza/coordinator/StreamPartitionCountMonitor.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala > 384b2e777c73fc1e4bc8a29312c9ea5372162ca1 > > samza-core/src/main/scala/org/apache/samza/coordinator/StreamPartitionCountMonitor.scala > 6aeff5787a0018ca2cae7d901c25537fbc7dea23 > > samza-core/src/test/scala/org/apache/samza/coordinator/TestStreamPartitionCountMonitor.scala > f47f8189bd92c4071ae76ae323e066823f3a6f61 > > Diff: https://reviews.apache.org/r/46856/diff/ > > > Testing > ------- > > Added a test. > > Ran check-all.sh > > > Thanks, > > Jake Maes > >
