----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18222/#review35041 -----------------------------------------------------------
samza-api/src/main/java/org/apache/samza/system/SystemStreamMetadata.java <https://reviews.apache.org/r/18222/#comment65404> Would it be better to have a single Map of <Partition, Tuple (ie pojo) with oldest,newest,future>? May speed up accesses slightly. samza-api/src/main/java/org/apache/samza/system/SystemStreamMetadata.java <https://reviews.apache.org/r/18222/#comment65406> Not wild about Future. Upcoming? Next? Don't want to have confusion with Java Futures. samza-api/src/main/java/org/apache/samza/util/SinglePartitionSystemAdmin.java <https://reviews.apache.org/r/18222/#comment65409> Shouldn't this be SinglePartitionNoOffsets? A system could have a single partition and still support offsets, for instance, a file on a file system could be treated as such. samza-api/src/main/java/org/apache/samza/util/SinglePartitionSystemAdmin.java <https://reviews.apache.org/r/18222/#comment65412> Should fakeOffsets still be static? I could see consuming from a large number of such topics in a single container and a single reference to this class might help. Would also speed up comparisons. samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala <https://reviews.apache.org/r/18222/#comment65416> We've been burned by these types of operations before. Can you pull it out into a unit testable method and write a quick one? samza-core/src/main/scala/org/apache/samza/util/Util.scala <https://reviews.apache.org/r/18222/#comment65420> This is very difficult to follow. Can you refactor and test? samza-core/src/main/scala/org/apache/samza/util/Util.scala <https://reviews.apache.org/r/18222/#comment65421> These are just whitespace changes? samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemAdmin.scala <https://reviews.apache.org/r/18222/#comment65423> Can you explicitly add the return type and a unit test? - Jakob Homan On Feb. 18, 2014, 10:25 a.m., Chris Riccomini wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18222/ > ----------------------------------------------------------- > > (Updated Feb. 18, 2014, 10:25 a.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > more kafka system admin cleanup > > > clean up kafka system admin a bit, and add documentation. > > > clean up some docs, whice space, and tests. > > > add documentation to system admin and system stream metadata classes. > > > rename system admin method again. update choosers to use new api style. > > > remove SystemAdmin.getPartitions since it can be derived from > getStreamMetadata. rename getStreamMetadata to be more verbose and accurate. > update kafka system admin to work with new metadata API > > > add SystemAdmin.getStreamMetadata and use stream metadata for bootstrap > chooser. > > > trigger failure in stateful task, since we were cheating and forcing the > changelog to be smallest. > > > Diffs > ----- > > samza-api/src/main/java/org/apache/samza/system/SystemAdmin.java > e8144e9be8978c1915bb969056ae0d01c7362f32 > samza-api/src/main/java/org/apache/samza/system/SystemStreamMetadata.java > PRE-CREATION > > samza-api/src/main/java/org/apache/samza/util/SinglePartitionSystemAdmin.java > 75ec26a1c328dfe0d2ba309f20d0292e3bb96e8c > > samza-api/src/test/java/org/apache/samza/util/TestSinglePartitionSystemAdmin.java > d120818b2a71d7ab61ab7cffb4b09ea3566994f0 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 13421d27d999225c25e65dc2ed7af80ba5a0339c > > samza-core/src/main/scala/org/apache/samza/system/chooser/BootstrappingChooser.scala > e875744e2c79be8718bcdd80b13c5f910c1002a9 > > samza-core/src/main/scala/org/apache/samza/system/chooser/DefaultChooser.scala > 53fee8e067854895e0d615f6cac5126203999543 > samza-core/src/main/scala/org/apache/samza/util/Util.scala > 04a6dfca51d34ec41e48514427a9db8861fac1b2 > > samza-core/src/test/scala/org/apache/samza/system/chooser/TestBootstrappingChooser.scala > 8f5fb6694b24dd662d0f8511bacab3432db6194b > > samza-core/src/test/scala/org/apache/samza/system/chooser/TestDefaultChooser.scala > 7f9e89cfcaab4cdcb2d4ff276df4aaffdcd6b634 > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemAdmin.scala > c5487b97219000c20c77ff13c27fcb1f185855a1 > > samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemAdmin.scala > e010c5a5c81badc85d9cd61e300e9715ea5f4c83 > samza-test/src/main/java/org/apache/samza/system/mock/MockSystemAdmin.java > 13ac689d85e362926fac3d3f98d360e7c2cd914f > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterTaskManager.scala > a7b55646aa4cd5fce384d5eebf9211416b5d02de > > Diff: https://reviews.apache.org/r/18222/diff/ > > > Testing > ------- > > > Thanks, > > Chris Riccomini > >
