----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33419/#review84612 -----------------------------------------------------------
Apart from some questions, everything looks good to me! docs/learn/documentation/versioned/container/state-management.md <https://reviews.apache.org/r/33419/#comment135949> Thanks for proactively adding docs :) samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java <https://reviews.apache.org/r/33419/#comment135954> Is "moving away from scala based configs" a motivation for all the "Java" prefixed classes? If the plan is to refactor configs everywhere with Java based configs, then we should probably extend these classes from org.apache.samza.config.MapConfig. It provides some convenient methods because it is backed by a Map. Just curious if you explored that option. samza-core/src/main/java/org/apache/samza/config/JavaSystemConfig.java <https://reviews.apache.org/r/33419/#comment135987> Why is Config "protected" here and "private" in JavaStorageConfig? samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala <https://reviews.apache.org/r/33419/#comment136105> why not sanitize the entire file name, instead of just the task name? Personally, I think we should follow a standard convention for sanitizing names, irrespective of whether it is storename or taskname. Just my two cents. - Navina Ramesh On May 20, 2015, 10:04 p.m., Yan Fang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33419/ > ----------------------------------------------------------- > > (Updated May 20, 2015, 10:04 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-625 > https://issues.apache.org/jira/browse/SAMZA-625 > > > Repository: samza > > > Description > ------- > > Implemented in Java. > > * modified build.gradle to have the gradle compile scala first. Because some > jave code has dependencies to Scala code > * change the state store name by removing the space ( in TaskManager ) > * add scala java conversion method in Util because some classes only accept > scala map > * add java version of some configs > * remove duplicated config in samza-log4j > * add StorageRevoery class, which does most of the recoverying job. The logic > mimics what happens in SamzaContainer. > * add StateStorageTool, for the commandline usage > * unit tests > * docs > > > Diffs > ----- > > checkstyle/import-control.xml 5f8e103 > docs/learn/documentation/versioned/container/state-management.md 79067bb > samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/config/JavaSystemConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/storage/StateStorageTool.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/storage/StorageRecovery.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala > aeba61a > samza-core/src/main/scala/org/apache/samza/util/Util.scala 2feb65b > samza-core/src/test/java/org/apache/samza/config/TestJavaSystemConfig.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/MockStorageEngine.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/storage/MockStorageEngineFactory.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/MockSystemConsumer.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/MockSystemFactory.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java > PRE-CREATION > samza-log4j/src/main/java/org/apache/samza/config/Log4jSystemConfig.java > d5e24f2 > samza-shell/src/main/bash/state-storage-tool.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/33419/diff/ > > > Testing > ------- > > tested with multiple partitions and multiple stores recovery. > > > Thanks, > > Yan Fang > >
