> On 2012-02-21 00:29:14, Arvind Prabhakar wrote: > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, > > lines 106-109 > > <https://reviews.apache.org/r/3924/diff/5/?file=76163#file76163line106> > > > > Not sure if this is a valid assumption since the first line of the > > append() method raises an exception if the protocol client is null. > > > > Also, if we must dispose the protocol client, the underlying > > transceiver should be closed as well in order to ensure there are no > > resource leaks.
Hi Arvind, Thanks for the feedback. If close() is called on the appender, the protocolClient is set to null, which is what I meant to do. This is to ensure that a future append cannot happen on a closed appender. I think the comment was outdated, and a call to append no longer creates a new client, but throws an exception. Sorry about that. If append is called on the appender after close(), it throws an Exception which I think is a valid case. I think it is also valid to throw an exception, if append is called before the activeOptions() function is called(in which case the protocolClient is null). Both these cases exist only when the appender is used programatically, not through a config file. I will close the transceiver in the close function, to make sure the resources are released immediately once the close function is called Just a note: even without this the resources would be cleared off by Java garbage collector since the protocolClient reference is no longer valid, hence all internal members of the client will be cleared, since no references to them exist anywhere outside. > On 2012-02-21 00:29:14, Arvind Prabhakar wrote: > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, > > line 40 > > <https://reviews.apache.org/r/3924/diff/5/?file=76163#file76163line40> > > > > A brief javadoc will be good that describes this implementation. Sure. - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5226 ----------------------------------------------------------- On 2012-02-20 22:54:11, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3924/ > ----------------------------------------------------------- > > (Updated 2012-02-20 22:54:11) > > > Review request for Flume. > > > Summary > ------- > > A class which takes in a LoggingEvent and sends it to a predefined AvroSource > at a given host and port. > > > This addresses bug FLUME-886. > https://issues.apache.org/jira/browse/FLUME-886 > > > Diffs > ----- > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml > PRE-CREATION > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java > PRE-CREATION > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java > PRE-CREATION > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java > PRE-CREATION > > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties > PRE-CREATION > branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION > branches/flume-728/pom.xml 1244858 > > Diff: https://reviews.apache.org/r/3924/diff > > > Testing > ------- > > Added unit tests. > > > Thanks, > > Hari > >
