You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Oliver Szabo <os...@hortonworks.com> on 2017/03/09 13:46:42 UTC
Review Request 57461: Logfeeder: add de-duplication support
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57461/
-----------------------------------------------------------
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
Re: Review Request 57461: Logfeeder: add de-duplication support
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57461/#review168470
-----------------------------------------------------------
Ship it!
Ship It!
- Miklos Gergely
On March 9, 2017, 4:32 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, 4:32 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/3/
>
>
> 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
>
>
Re: Review Request 57461: Logfeeder: add de-duplication support
Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57461/
-----------------------------------------------------------
(Updated March 9, 2017, 4:32 p.m.)
Review request for Ambari, Miklos Gergely, Robert Nettleton, and Sebastian Toader.
Changes
-------
use evictingQueue
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 (updated)
-----
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/3/
Changes: https://reviews.apache.org/r/57461/diff/2-3/
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
Re: Review Request 57461: Logfeeder: add de-duplication support
Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57461/
-----------------------------------------------------------
(Updated March 9, 2017, 3:52 p.m.)
Review request for Ambari, Miklos Gergely, Robert Nettleton, and Sebastian Toader.
Changes
-------
add small changes
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 (updated)
-----
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/2/
Changes: https://reviews.apache.org/r/57461/diff/1-2/
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
Re: Review Request 57461: Logfeeder: add de-duplication support
Posted by Oliver Szabo <os...@hortonworks.com>.
> On March 9, 2017, 3:40 p.m., Miklos Gergely wrote:
> > 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.
thanks, actually as i have already investigated it, guava EvictingQueue can be good for that.
> On March 9, 2017, 3:40 p.m., Miklos Gergely wrote:
> > 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/diff/1/?file=1660140#file1660140line34>
> >
> > 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.
i will keep the "most" part, that shows it's mru-like data
- Oliver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57461/#review168460
-----------------------------------------------------------
On March 9, 2017, 3:52 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, 3:52 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/2/
>
>
> 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
>
>
Re: Review Request 57461: Logfeeder: add de-duplication support
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
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
>
>