----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52476/#review164786 -----------------------------------------------------------
Fix it, then Ship it! LGTM, thanks. Few code style/documentation related comments. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 107) <https://reviews.apache.org/r/52476/#comment236555> Can delete, doesn't describe the entire block, and unnecessary for the first part. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 113) <https://reviews.apache.org/r/52476/#comment236557> s/of the store/for the store. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 120) <https://reviews.apache.org/r/52476/#comment236554> @param doesn't go in the method description, use {@code} samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 135) <https://reviews.apache.org/r/52476/#comment236559> Explain what stale means here. Use @code instead of @param here. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 138) <https://reviews.apache.org/r/52476/#comment236561> Can just say "true if the store is stale". Description of what stale means should go in the method description. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 159) <https://reviews.apache.org/r/52476/#comment236562> No comma before 'if' if it's the last clause in the sentence. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 196) <https://reviews.apache.org/r/52476/#comment236565> s/in the store/for the store. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 198) <https://reviews.apache.org/r/52476/#comment236563> No space before colon, here and other places in this file. samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala (line 202) <https://reviews.apache.org/r/52476/#comment236564> Thanks! - Prateek Maheshwari On Feb. 8, 2017, 1:37 p.m., Shanthoosh Venkataraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52476/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2017, 1:37 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > Every local task store is backed up by a kafka changelog topic. Due to log > compaction, delete tombstones of the changelog topic have a ttl of > delete.retention.ms. Replaying the events from the changelog that has missing > delete tombstones, would result in creation of an inconsistent local > store(due to the missing of some delete events). This patch deletes the local > stores in which difference between current time and last modified time of the > offset file is greater than delete.retention.ms during the container startup. > > > Diffs > ----- > > samza-core/src/main/java/org/apache/samza/storage/StorageRecovery.java > 9329edf7d724f3a0d9235354bb77936f713e3b5f > samza-core/src/main/scala/org/apache/samza/config/StorageConfig.scala > a3587d0a40c57374ee1742234929d444e381e42d > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > c3308bfd7de04c335fef6cb66baa29286a230080 > samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala > 0b7bcdda1639eea8239a69c31bdf42558e9077d2 > > samza-core/src/test/scala/org/apache/samza/storage/TestTaskStorageManager.scala > 4d40f520e54beb643acd8410c772b75e2f6a9162 > samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala > 9320cf744ff90d647a198b51cb06d2a526fe68fa > > Diff: https://reviews.apache.org/r/52476/diff/ > > > Testing > ------- > > Unit testing and manual testing has been done to verify the functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
