-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48182/#review136335
-----------------------------------------------------------


Fix it, then Ship it!





samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 
(line 412)
<https://reviews.apache.org/r/48182/#comment201356>

    This might be sufficient, but I've seen some excessively long GC pauses and 
the like on Hudson. Something like 10s has worked well for me in the past and 
the join should take nowhere near that long in most cases.



samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 
(lines 490 - 491)
<https://reviews.apache.org/r/48182/#comment201353>

    I would name these a little differently to improve readability. Something 
like runner3StartedLatch and runner1FinishedLatch; or startRunner2 and 
startRunner1 latch; or similar.



samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala 
(line 527)
<https://reviews.apache.org/r/48182/#comment201355>

    How about actually capturing the test failure and rethrowing from the main 
thread? It will give you a much more helpful error message.


- Chris Pettitt


On June 3, 2016, 9:30 p.m., Xinyu Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48182/
> -----------------------------------------------------------
> 
> (Updated June 3, 2016, 9:30 p.m.)
> 
> 
> Review request for samza, Chris Pettitt, Navina Ramesh, and Yi Pan (Data 
> Infrastructure).
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> All the existing stores/cache need to be thread safe in order to be used by 
> multithreaded tasks. The following changes are made to ensure the thread 
> safety of the stores:
> 
> For CachedStore, use sychronized lock for each public function;
> For current InMemoryKeyValueStore, use ConcurrentSkipListMap as underlying 
> map for thread safety.
> For store Iterator, do not support remove functionality (throw 
> UnsupportedOperationException like RocksDb does today).
> 
> 
> Diffs
> -----
> 
>   
> samza-kv-inmemory/src/main/scala/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.scala
>  72f25a354eaa98e8df379d07d9cc8613dfafd13a 
>   
> samza-kv-rocksdb/src/main/scala/org/apache/samza/storage/kv/RocksDbKeyValueStore.scala
>  f0965aec5f3ec2a214dc40c70832c58273623749 
>   
> samza-kv-rocksdb/src/test/scala/org/apache/samza/storage/kv/TestRocksDbKeyValueStore.scala
>  b7f1cdc4dbaeea2413cee2ad60d74528f3950513 
>   samza-kv/src/main/scala/org/apache/samza/storage/kv/CachedStore.scala 
> c28f8db8cb59bd5415e78535877acc1e5bee0f67 
>   samza-kv/src/test/scala/org/apache/samza/storage/kv/TestCachedStore.scala 
> eee74473726cb2a36d0b75fe5c9df737440980bc 
>   
> samza-test/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStores.scala
>  23f8a1a6bee8ef38e0640a4e90778e53d982deeb 
> 
> Diff: https://reviews.apache.org/r/48182/diff/
> 
> 
> Testing
> -------
> 
> Unit tests and local deployment.
> 
> 
> Thanks,
> 
> Xinyu Liu
> 
>

Reply via email to