|
Cameron Gandevia
2012-12-03, 20:52
Hari Shreedharan
2012-12-04, 04:40
Cameron Gandevia
2012-12-07, 20:12
Cameron Gandevia
2013-01-18, 02:17
Hari Shreedharan
2013-01-22, 06:07
Hari Shreedharan
2013-01-22, 07:02
Cameron Gandevia
2013-01-22, 22:09
Hari Shreedharan
2013-01-23, 05:08
Cameron Gandevia
2013-02-21, 22:43
Hari Shreedharan
2013-02-21, 22:46
Cameron Gandevia
2013-02-22, 01:38
Hari Shreedharan
2012-12-10, 19:45
Cameron Gandevia
2013-01-18, 02:29
Hari Shreedharan
2013-01-22, 06:03
Brock Noland
2012-12-21, 18:02
Hari Shreedharan
2012-12-22, 22:54
Hari Shreedharan
2013-01-03, 23:44
Hari Shreedharan
2013-01-04, 02:53
|
-
Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2012-12-03, 20:52
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/ ----------------------------------------------------------- Review request for Flume. Description ------- The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender Diffs ----- flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb flume-ng-sdk/src/main/java/org/apache/flume/api/HostInfo.java 7388a45 flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 3c93921 Diff: https://reviews.apache.org/r/8328/diff/ Testing ------- Thanks, Cameron Gandevia +
Cameron Gandevia 2012-12-03, 20:52
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2012-12-04, 04:40
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14008 ----------------------------------------------------------- Does not look like you are creating LoadBalancingRpcClient anywhere. Also I don't think there is a need for more getInstance methods, just use the one currently available - pass in a properties instance. - Hari Shreedharan On Dec. 3, 2012, 8:52 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2012, 8:52 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 > flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb > flume-ng-sdk/src/main/java/org/apache/flume/api/HostInfo.java 7388a45 > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 3c93921 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2012-12-04, 04:40
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2012-12-07, 20:12
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/ ----------------------------------------------------------- (Updated Dec. 7, 2012, 8:12 p.m.) Review request for Flume. Changes ------- Code review fixes. Description ------- The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender Diffs (updated) ----- flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb Diff: https://reviews.apache.org/r/8328/diff/ Testing ------- Thanks, Cameron Gandevia +
Cameron Gandevia 2012-12-07, 20:12
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2013-01-18, 02:17
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/ ----------------------------------------------------------- (Updated Jan. 18, 2013, 2:17 a.m.) Review request for Flume. Changes ------- Fixed failing test cases Description ------- The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender Diffs (updated) ----- flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java.orig PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 Diff: https://reviews.apache.org/r/8328/diff/ Testing ------- Thanks, Cameron Gandevia +
Cameron Gandevia 2013-01-18, 02:17
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-01-22, 06:07
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review15565 ----------------------------------------------------------- I didn't do a full review, but there is ".orig" file which you need to remove. I will post a full review soon (after running tests without this file). flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java <https://reviews.apache.org/r/8328/#comment33646> Whitespaces should be removed. flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java.orig <https://reviews.apache.org/r/8328/#comment33645> Seems like this file was accidentally included? - Hari Shreedharan On Jan. 18, 2013, 2:17 a.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2013, 2:17 a.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java.orig PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-01-22, 06:07
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-01-22, 07:02
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review15566 ----------------------------------------------------------- +1. Looks good. Please remove the extra file, and the whitespaces and submit the patch on the jira. Also remove the System.out.println calls too. A flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java <https://reviews.apache.org/r/8328/#comment33647> Please remove these Stdout calls. flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java <https://reviews.apache.org/r/8328/#comment33648> Please remove this stdout call. - Hari Shreedharan On Jan. 18, 2013, 2:17 a.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2013, 2:17 a.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java.orig PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-01-22, 07:02
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2013-01-22, 22:09
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/ ----------------------------------------------------------- (Updated Jan. 22, 2013, 10:09 p.m.) Review request for Flume. Changes ------- Code review fixes -Removed System.out.println calls -Removed git diff file Description ------- The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender Diffs (updated) ----- flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 Diff: https://reviews.apache.org/r/8328/diff/ Testing ------- Thanks, Cameron Gandevia +
Cameron Gandevia 2013-01-22, 22:09
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-01-23, 05:08
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review15606 ----------------------------------------------------------- Ship it! Looks good, great patch! Could you please remove the whitespaces and attach the patch to the jira so I can commit it? Thanks! flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java <https://reviews.apache.org/r/8328/#comment33702> Nit: Whitespaces. - Hari Shreedharan On Jan. 22, 2013, 10:09 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2013, 10:09 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-01-23, 05:08
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2013-02-21, 22:43
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/ ----------------------------------------------------------- (Updated Feb. 21, 2013, 10:43 p.m.) Review request for Flume. Changes ------- Removed white spaces from Test classes Description ------- The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender Diffs (updated) ----- flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION flume-ng-doc/sphinx/FlumeUserGuide.rst 8eb3734 Diff: https://reviews.apache.org/r/8328/diff/ Testing ------- Thanks, Cameron Gandevia +
Cameron Gandevia 2013-02-21, 22:43
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-02-21, 22:46
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review16887 ----------------------------------------------------------- Cameron: Please attach the patch to the jira to confirm that you have granted ASF the license to your patch. Thanks! - Hari Shreedharan On Feb. 21, 2013, 10:43 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2013, 10:43 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst 8eb3734 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-02-21, 22:46
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2013-02-22, 01:38
> On Feb. 21, 2013, 10:46 p.m., Hari Shreedharan wrote: > > Cameron: > > > > Please attach the patch to the jira to confirm that you have granted ASF the license to your patch. Thanks! Done - Cameron ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review16887 ----------------------------------------------------------- On Feb. 21, 2013, 10:43 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2013, 10:43 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst 8eb3734 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Cameron Gandevia 2013-02-22, 01:38
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2012-12-10, 19:45
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- Generally looks good. One minor comment below. flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java <https://reviews.apache.org/r/8328/#comment30399> Why make this package private? - Hari Shreedharan On Dec. 7, 2012, 8:12 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 8:12 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2012-12-10, 19:45
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderCameron Gandevia 2013-01-18, 02:29
> On Dec. 10, 2012, 7:45 p.m., Hari Shreedharan wrote: > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 65-68 > > <https://reviews.apache.org/r/8328/diff/2/?file=235858#file235858line65> > > > > Why make this package private? > > Brock Noland wrote: > Hi, > > Just curious, is this the only thing holding this patch up? > > Hari Shreedharan wrote: > I was expecting the contributor to come back with this change. Since he has not, I think we can commit this. I'd like to review this once more and commit this. > > Hari Shreedharan wrote: > I am going to make this change and commit this. > > Hari Shreedharan wrote: > The tests added are failing too. Sorry guys I was out of town during the holidays. I have fixed the test cases. The RpcClient is now instantiated in the LoadBalancingLog4jAppender a subclass of the Log4jAppender which is why I made the access change. - Cameron ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- On Jan. 18, 2013, 2:17 a.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2013, 2:17 a.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java.orig PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Cameron Gandevia 2013-01-18, 02:29
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-01-22, 06:03
> On Dec. 10, 2012, 7:45 p.m., Hari Shreedharan wrote: > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 65-68 > > <https://reviews.apache.org/r/8328/diff/2/?file=235858#file235858line65> > > > > Why make this package private? > > Brock Noland wrote: > Hi, > > Just curious, is this the only thing holding this patch up? > > Hari Shreedharan wrote: > I was expecting the contributor to come back with this change. Since he has not, I think we can commit this. I'd like to review this once more and commit this. > > Hari Shreedharan wrote: > I am going to make this change and commit this. > > Hari Shreedharan wrote: > The tests added are failing too. > > Cameron Gandevia wrote: > Sorry guys I was out of town during the holidays. I have fixed the test cases. > > The RpcClient is now instantiated in the LoadBalancingLog4jAppender a subclass of the Log4jAppender which is why I made the access change. Thanks Cameron. Let me try running the tests with the new patch. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- On Jan. 18, 2013, 2:17 a.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2013, 2:17 a.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 315a68c > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java.orig PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst aa92974 > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-01-22, 06:03
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderBrock Noland 2012-12-21, 18:02
> On Dec. 10, 2012, 7:45 p.m., Hari Shreedharan wrote: > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 65-68 > > <https://reviews.apache.org/r/8328/diff/2/?file=235858#file235858line65> > > > > Why make this package private? Hi, Just curious, is this the only thing holding this patch up? - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- On Dec. 7, 2012, 8:12 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 8:12 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Brock Noland 2012-12-21, 18:02
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2012-12-22, 22:54
> On Dec. 10, 2012, 7:45 p.m., Hari Shreedharan wrote: > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 65-68 > > <https://reviews.apache.org/r/8328/diff/2/?file=235858#file235858line65> > > > > Why make this package private? > > Brock Noland wrote: > Hi, > > Just curious, is this the only thing holding this patch up? I was expecting the contributor to come back with this change. Since he has not, I think we can commit this. I'd like to review this once more and commit this. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- On Dec. 7, 2012, 8:12 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 8:12 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2012-12-22, 22:54
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-01-03, 23:44
> On Dec. 10, 2012, 7:45 p.m., Hari Shreedharan wrote: > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 65-68 > > <https://reviews.apache.org/r/8328/diff/2/?file=235858#file235858line65> > > > > Why make this package private? > > Brock Noland wrote: > Hi, > > Just curious, is this the only thing holding this patch up? > > Hari Shreedharan wrote: > I was expecting the contributor to come back with this change. Since he has not, I think we can commit this. I'd like to review this once more and commit this. I am going to make this change and commit this. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- On Dec. 7, 2012, 8:12 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 8:12 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-01-03, 23:44
-
Re: Review Request: FLUME-1765 - Add Load Balancing Support to Log4jAppenderHari Shreedharan 2013-01-04, 02:53
> On Dec. 10, 2012, 7:45 p.m., Hari Shreedharan wrote: > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 65-68 > > <https://reviews.apache.org/r/8328/diff/2/?file=235858#file235858line65> > > > > Why make this package private? > > Brock Noland wrote: > Hi, > > Just curious, is this the only thing holding this patch up? > > Hari Shreedharan wrote: > I was expecting the contributor to come back with this change. Since he has not, I think we can commit this. I'd like to review this once more and commit this. > > Hari Shreedharan wrote: > I am going to make this change and commit this. The tests added are failing too. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8328/#review14257 ----------------------------------------------------------- On Dec. 7, 2012, 8:12 p.m., Cameron Gandevia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8328/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 8:12 p.m.) > > > Review request for Flume. > > > Description > ------- > > The Log4jAppender should be extended to use the LoadBalancingRpcClient allowing users to configure a load balancing log4jappender > > > Diffs > ----- > > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/LoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java 083f5d1 > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-backoff-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancing-rnd-log4jtest.properties PRE-CREATION > flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-loadbalancinglog4jtest.properties PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst f8528bb > > Diff: https://reviews.apache.org/r/8328/diff/ > > > Testing > ------- > > > Thanks, > > Cameron Gandevia > > +
Hari Shreedharan 2013-01-04, 02:53
|