----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36973/#review101352 -----------------------------------------------------------
Apart from the few comments, looks good to me! samza-kv-rocksdb/src/main/java/org/apache/samza/storage/kv/RocksDbKeyValueReader.java (line 88) <https://reviews.apache.org/r/36973/#comment158731> Adding to what Yi suggested, this kind of option parsing is not obvious for the user. Take the case of "job.coordinator.system". This propoerty is populated if there is only 1 kafka system defined. User is not aware of this and has issues identifying the problem when new systems get added. samza-kv-rocksdb/src/main/java/org/apache/samza/storage/kv/RocksDbKeyValueStoreHelper.java (line 42) <https://reviews.apache.org/r/36973/#comment158722> Isn't the taskNames.size() always going to be 1 per Line 73 in RocksDBKeyValueReader? I am trying to understand why we need to pass in the SamzaContainerContext here? samza-kv-rocksdb/src/main/java/org/apache/samza/storage/kv/RocksDbReadingTool.java (line 80) <https://reviews.apache.org/r/36973/#comment158720> Instead of making each type of key an optional argument, I think it is better to create a required argument that take a value. Eg: --key-type={long|string|int} - Navina Ramesh On July 31, 2015, 3:11 a.m., Yan Fang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36973/ > ----------------------------------------------------------- > > (Updated July 31, 2015, 3:11 a.m.) > > > Review request for samza. > > > Bugs: SAMZA-626 > https://issues.apache.org/jira/browse/SAMZA-626 > > > Repository: samza > > > Description > ------- > > *refactored some java code > > *changed RocksDbKeyValueStore.options form scala to java > > *moved default serde name from container to util, because it is useful to > other classes > > *added a class to read the running rocksdb > > *added a commondline tool > > *updated the doc accordingly > > > Diffs > ----- > > build.gradle 0852adc > docs/learn/documentation/versioned/container/state-management.md 50d4b65 > samza-core/src/main/java/org/apache/samza/config/JavaSerializerConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java > af7d4ca > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 27b2517 > samza-core/src/main/scala/org/apache/samza/util/Util.scala 419452c > > samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala > 84fdeaa > samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala ead6f94 > > samza-kv-rocksdb/src/main/java/org/apache/samza/storage/kv/RocksDbKeyValueReader.java > PRE-CREATION > > samza-kv-rocksdb/src/main/java/org/apache/samza/storage/kv/RocksDbKeyValueStoreHelper.java > PRE-CREATION > > samza-kv-rocksdb/src/main/java/org/apache/samza/storage/kv/RocksDbReadingTool.java > PRE-CREATION > > samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStorageEngineFactory.scala > 571a50e > > samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala > a423f7b > > samza-kv-rocksdb/src/test/java/org/apache/samza/storage/kv/TestRocksDbKeyValueReader.java > PRE-CREATION > samza-shell/src/main/bash/read-rocksdb-tool.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/36973/diff/ > > > Testing > ------- > > > Thanks, > > Yan Fang > >
