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


Thanks for the patch Bruno. The implementation needs to be slightly reworked to 
ensure thread-safe handling of jmsSession object. What I suggest is that you 
refactor the implementation so that:

1. configure() method creates the necessary naming context
2. start() method initializes the connection factory and creates a connection.
3. process() method creates a session, uses it to publish the event and then 
commits it.
4. stop() method closes the connection.

Another point of consideration is to enable batch message support in this. 
However, that can be done at a later stage via a separate issue.

Thanks

- Arvind


On 2012-02-01 19:02:07, Bruno Mahé wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3723/
> -----------------------------------------------------------
> 
> (Updated 2012-02-01 19:02:07)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> Here is a review form a rebased patch of FLUME-923.
> Sorry Dave, reviewboard didn't want your patch. So I re-exported from my 
> branch
> 
> 
> This addresses bug FLUME-923.
>     https://issues.apache.org/jira/browse/FLUME-923
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/sink/SinkType.java 6b08c09 
>   flume-ng-dist/pom.xml 19f6b0c 
>   flume-ng-node/pom.xml b9b062e 
>   flume-ng-sinks/flume-jms-sink/pom.xml PRE-CREATION 
>   
> flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java
>  PRE-CREATION 
>   
> flume-ng-sinks/flume-jms-sink/src/test/java/org/apache/flume/sink/jms/TestJMSSink.java
>  PRE-CREATION 
>   flume-ng-sinks/pom.xml b6cf477 
>   pom.xml abee9a5 
> 
> Diff: https://reviews.apache.org/r/3723/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bruno
> 
>

Reply via email to