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



As I see the cache is turned on by default, and the user may only turn it off 
via custom properties. I'm not sure if the user's won't find it misleading, 
maybe the new properties should be added to the logsearch properties. I'd also 
suggest to have the cache turned off by default. This is a powerful tool, but 
the user must know what's happening, by explicitly turning it on.


ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/cache/LRUCache.java
Lines 34 (patched)
<https://reviews.apache.org/r/57461/#comment240723>

    This variable should be called mostRecentLogs, or simply recentLogs. If we 
are planning to store the last n logs than a LinkedList would be a better data 
structure for it, as it can easily remove it's last element, and attach a new 
first one, while an array must always be shifted.



ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/cache/LRUCache.java
Lines 44 (patched)
<https://reviews.apache.org/r/57461/#comment240722>

    using super here is superfluous and misleading, as the size is not 
overriden, you can just use size()



ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/output/OutputManager.java
Lines 146 (patched)
<https://reviews.apache.org/r/57461/#comment240724>

    The two conditions could be &&-ed together, thus not having an extra level 
of blocks.


- Miklos Gergely


On March 9, 2017, 1:46 p.m., Oliver Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57461/
> -----------------------------------------------------------
> 
> (Updated March 9, 2017, 1:46 p.m.)
> 
> 
> Review request for Ambari, Miklos Gergely, Robert Nettleton, and Sebastian 
> Toader.
> 
> 
> Bugs: AMBARI-20378
>     https://issues.apache.org/jira/browse/AMBARI-20378
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Added de-duplication support on log feeder side.
> 
> For that I used an custom LRU cache. If it reaches maximum size, the least 
> recently element will be removed.
> Also because we often get the MRU element of the cache as well (to filter out 
> if we want to setup to filter out the last message if that repeated too 
> much), we store the MRU element in the cache. Its cheaper then get the name 
> from the LinkedMap beause we will need to iterate over the map until the last 
> element in the map (every time).
> 
> 
> Diffs
> -----
> 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/common/LogFeederConstants.java
>  d1e7fba 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterJSON.java
>  ba63c61 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java
>  e13d9bd 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/cache/LRUCache.java
>  PRE-CREATION 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/mapper/MapperDate.java
>  eb3ae01 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/output/OutputLineFilter.java
>  PRE-CREATION 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/output/OutputManager.java
>  86b5c57 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/test/java/org/apache/ambari/logfeeder/filter/FilterJSONTest.java
>  06d8db2 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/test/java/org/apache/ambari/logfeeder/input/cache/LRUCacheTest.java
>  PRE-CREATION 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/test/java/org/apache/ambari/logfeeder/mapper/MapperDateTest.java
>  08680f6 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/test/java/org/apache/ambari/logfeeder/output/OutputLineFilterTest.java
>  PRE-CREATION 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/test/java/org/apache/ambari/logfeeder/output/OutputManagerTest.java
>  a080fa8 
>   
> ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-zookeeper.json
>  122a9e1 
> 
> 
> Diff: https://reviews.apache.org/r/57461/diff/1/
> 
> 
> Testing
> -------
> 
> unit test added and updated. 
> done.
> 
> logfeeder test output:
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 13.074s
> [INFO] Finished at: Thu Mar 09 14:40:44 CET 2017
> [INFO] Final Memory: 28M/324M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Oliver Szabo
> 
>

Reply via email to