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/12/05 20:50:41 UTC
Re: Review Request 64349: LogFeeder: filter objects for wildcard
input paths need to be cloned
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64349/
-----------------------------------------------------------
(Updated Dec. 5, 2017, 8:50 p.m.)
Review request for Ambari, Krisztian Kasa, Miklos Gergely, and Robert Nettleton.
Changes
-------
update summary
Summary (updated)
-----------------
LogFeeder: filter objects for wildcard input paths need to be cloned
Bugs: AMBARI-22600
https://issues.apache.org/jira/browse/AMBARI-22600
Repository: ambari
Description
-------
Log Feeder needs to clone filters for different input threads (for wildcard pattern).
As different threads tried to access the same grok filter object (mutable) that could cause to some docs used the wrong path (or other fields), especially when a grok error happened. (on error, in the code, we checked some objects regarding that what to do, but other threads changed those objects)
Also some other changes:
- use sdi_* dynamic fields
- add new input log files
- add new config to grok filter: skip_on_error (can be useful for filter chains)
Diffs
-----
ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/Filter.java afd903e
ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java 49d7e76
ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java 7df0b6e
ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-storm.json 34bdcf0
ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-1-TestAgg-2-3/6701/worker.log 6a10ad9
ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-2-TestAgg2-4-5/6700/worker.log e69de29
Diff: https://reviews.apache.org/r/64349/diff/1/
Testing
-------
done, i need to write some unit tests for the filter cloning.
Thanks,
Oliver Szabo
Re: Review Request 64349: LogFeeder: filter objects for wildcard
input paths need to be cloned
Posted by Krisztian Kasa <kk...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64349/#review192987
-----------------------------------------------------------
Ship it!
Ship It!
- Krisztian Kasa
On Dec. 5, 2017, 8:50 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64349/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 8:50 p.m.)
>
>
> Review request for Ambari, Krisztian Kasa, Miklos Gergely, and Robert Nettleton.
>
>
> Bugs: AMBARI-22600
> https://issues.apache.org/jira/browse/AMBARI-22600
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Log Feeder needs to clone filters for different input threads (for wildcard pattern).
>
> As different threads tried to access the same grok filter object (mutable) that could cause to some docs used the wrong path (or other fields), especially when a grok error happened. (on error, in the code, we checked some objects regarding that what to do, but other threads changed those objects)
>
> Also some other changes:
> - use sdi_* dynamic fields
> - add new input log files
> - add new config to grok filter: skip_on_error (can be useful for filter chains)
>
>
> Diffs
> -----
>
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/Filter.java afd903e
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java 49d7e76
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java 7df0b6e
> ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-storm.json 34bdcf0
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-1-TestAgg-2-3/6701/worker.log 6a10ad9
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-2-TestAgg2-4-5/6700/worker.log e69de29
>
>
> Diff: https://reviews.apache.org/r/64349/diff/1/
>
>
> Testing
> -------
>
> done, i need to write some unit tests for the filter cloning.
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 64349: LogFeeder: filter objects for wildcard
input paths need to be cloned
Posted by Sid Wagle <sw...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64349/#review192963
-----------------------------------------------------------
Ship it!
Ok good for now. This area definitely seems like it needs some rewrite to use Executors and better concurrency contructs but for branch-2.6 adding a +1
- Sid Wagle
On Dec. 5, 2017, 8:50 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64349/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 8:50 p.m.)
>
>
> Review request for Ambari, Krisztian Kasa, Miklos Gergely, and Robert Nettleton.
>
>
> Bugs: AMBARI-22600
> https://issues.apache.org/jira/browse/AMBARI-22600
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Log Feeder needs to clone filters for different input threads (for wildcard pattern).
>
> As different threads tried to access the same grok filter object (mutable) that could cause to some docs used the wrong path (or other fields), especially when a grok error happened. (on error, in the code, we checked some objects regarding that what to do, but other threads changed those objects)
>
> Also some other changes:
> - use sdi_* dynamic fields
> - add new input log files
> - add new config to grok filter: skip_on_error (can be useful for filter chains)
>
>
> Diffs
> -----
>
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/Filter.java afd903e
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java 49d7e76
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java 7df0b6e
> ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-storm.json 34bdcf0
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-1-TestAgg-2-3/6701/worker.log 6a10ad9
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-2-TestAgg2-4-5/6700/worker.log e69de29
>
>
> Diff: https://reviews.apache.org/r/64349/diff/1/
>
>
> Testing
> -------
>
> done, i need to write some unit tests for the filter cloning.
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 64349: LogFeeder: filter objects for wildcard
input paths need to be cloned
Posted by Oliver Szabo <os...@hortonworks.com>.
> On Dec. 5, 2017, 9:04 p.m., Sid Wagle wrote:
> > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java
> > Lines 215 (patched)
> > <https://reviews.apache.org/r/64349/diff/1/?file=1909192#file1909192line215>
> >
> > Have you considered using AtomicReference here vs deepCloning?
Maybe that would require more changes because then it would make sense to use it overall for handling filters or other config blocks as well where we originally creating them. actually that wildcard feature does not exist on trunk, we are planning to re-design it as possible so I think we can use that way on trunk. (if it will be needed)
- Oliver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64349/#review192930
-----------------------------------------------------------
On Dec. 5, 2017, 8:50 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64349/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 8:50 p.m.)
>
>
> Review request for Ambari, Krisztian Kasa, Miklos Gergely, and Robert Nettleton.
>
>
> Bugs: AMBARI-22600
> https://issues.apache.org/jira/browse/AMBARI-22600
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Log Feeder needs to clone filters for different input threads (for wildcard pattern).
>
> As different threads tried to access the same grok filter object (mutable) that could cause to some docs used the wrong path (or other fields), especially when a grok error happened. (on error, in the code, we checked some objects regarding that what to do, but other threads changed those objects)
>
> Also some other changes:
> - use sdi_* dynamic fields
> - add new input log files
> - add new config to grok filter: skip_on_error (can be useful for filter chains)
>
>
> Diffs
> -----
>
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/Filter.java afd903e
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java 49d7e76
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java 7df0b6e
> ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-storm.json 34bdcf0
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-1-TestAgg-2-3/6701/worker.log 6a10ad9
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-2-TestAgg2-4-5/6700/worker.log e69de29
>
>
> Diff: https://reviews.apache.org/r/64349/diff/1/
>
>
> Testing
> -------
>
> done, i need to write some unit tests for the filter cloning.
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 64349: LogFeeder: filter objects for wildcard
input paths need to be cloned
Posted by Sid Wagle <sw...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64349/#review192930
-----------------------------------------------------------
ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java
Lines 215 (patched)
<https://reviews.apache.org/r/64349/#comment271281>
Have you considered using AtomicReference here vs deepCloning?
- Sid Wagle
On Dec. 5, 2017, 8:50 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64349/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 8:50 p.m.)
>
>
> Review request for Ambari, Krisztian Kasa, Miklos Gergely, and Robert Nettleton.
>
>
> Bugs: AMBARI-22600
> https://issues.apache.org/jira/browse/AMBARI-22600
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Log Feeder needs to clone filters for different input threads (for wildcard pattern).
>
> As different threads tried to access the same grok filter object (mutable) that could cause to some docs used the wrong path (or other fields), especially when a grok error happened. (on error, in the code, we checked some objects regarding that what to do, but other threads changed those objects)
>
> Also some other changes:
> - use sdi_* dynamic fields
> - add new input log files
> - add new config to grok filter: skip_on_error (can be useful for filter chains)
>
>
> Diffs
> -----
>
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/Filter.java afd903e
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java 49d7e76
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java 7df0b6e
> ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-storm.json 34bdcf0
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-1-TestAgg-2-3/6701/worker.log 6a10ad9
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-2-TestAgg2-4-5/6700/worker.log e69de29
>
>
> Diff: https://reviews.apache.org/r/64349/diff/1/
>
>
> Testing
> -------
>
> done, i need to write some unit tests for the filter cloning.
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 64349: LogFeeder: filter objects for wildcard
input paths need to be cloned
Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64349/#review192996
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Nettleton
On Dec. 5, 2017, 8:50 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64349/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 8:50 p.m.)
>
>
> Review request for Ambari, Krisztian Kasa, Miklos Gergely, and Robert Nettleton.
>
>
> Bugs: AMBARI-22600
> https://issues.apache.org/jira/browse/AMBARI-22600
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Log Feeder needs to clone filters for different input threads (for wildcard pattern).
>
> As different threads tried to access the same grok filter object (mutable) that could cause to some docs used the wrong path (or other fields), especially when a grok error happened. (on error, in the code, we checked some objects regarding that what to do, but other threads changed those objects)
>
> Also some other changes:
> - use sdi_* dynamic fields
> - add new input log files
> - add new config to grok filter: skip_on_error (can be useful for filter chains)
>
>
> Diffs
> -----
>
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/Filter.java afd903e
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/filter/FilterGrok.java 49d7e76
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/input/Input.java 7df0b6e
> ambari-logsearch/docker/test-config/logfeeder/shipper-conf/input.config-storm.json 34bdcf0
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-1-TestAgg-2-3/6701/worker.log 6a10ad9
> ambari-logsearch/docker/test-logs/storm/worker-logs/streamline-2-TestAgg2-4-5/6700/worker.log e69de29
>
>
> Diff: https://reviews.apache.org/r/64349/diff/1/
>
>
> Testing
> -------
>
> done, i need to write some unit tests for the filter cloning.
>
>
> Thanks,
>
> Oliver Szabo
>
>