You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Mona Chitnis <mo...@yahoo.in> on 2012/12/05 01:22:11 UTC
Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
Review request for oozie.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1108
WIP patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
Diff: https://reviews.apache.org/r/8346/diff/
Testing
-------
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14341
-----------------------------------------------------------
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30537>
As the method signature has MessageReceiver, means the connection for endpoint already exists and session, receiver are also created. If a new connection is created at this point, the receiver will still belong to the old connection which is somehow null. So, probably the null check should be done beforehand.
It can be done before the method is called:
if (consumer!=null){
consumer.setMessageListener(this);
LOG.info(" Listener registered for end point ." + endPoint + " topic :" + topicName);
jas.addTopicReceiver(this, endPoint, topicName);
}
}
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30538>
It's not private but package-private :). Its accessible to classes of the same package
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/8346/#comment30540>
Recovery service runs at a fixed interval in a single thread and there is no danger of the list growing quickly. Here, its a different case.
So I think its better to queue it serially instead of using composite callable.
- Virag Kothari
On Dec. 11, 2012, 4:29 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2012, 4:29 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
(Updated Dec. 14, 2012, 9:57 p.m.)
Review request for oozie.
Changes
-------
Thanks for review Virag. Replaced the references.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1108
patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java 1422067
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1422067
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1422067
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1422067
Diff: https://reviews.apache.org/r/8346/diff/
Testing
-------
yes
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14520
-----------------------------------------------------------
looks good.. +1 after removing the 'yahoo' reference in JMSAccessorService test case.
- Virag Kothari
On Dec. 14, 2012, 8:37 p.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 14, 2012, 8:37 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java 1422067
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1422067
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1422067
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1422067
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1422067
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1422067
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1422067
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1422067
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1422067
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
(Updated Dec. 14, 2012, 8:37 p.m.)
Review request for oozie.
Changes
-------
Cleaner patch, addressed Virag's comments.
JMS connection robustness and cleanup will be done as part of separate JIRA
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1108
patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java 1422067
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1422067
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1422067
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1422067
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1422067
Diff: https://reviews.apache.org/r/8346/diff/
Testing
-------
yes
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14510
-----------------------------------------------------------
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30901>
We are not removing the server end point from the connSessionMap in any place. So it will not enter this else block and establish a new connection.
IMO, we should be doing the reliability of JMS connection in separate JIRA bcoz that will involve multiple things like closing old connectiions, updating data structures, retrying muliple times to establish new connections, etc.
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30902>
why is a function being called to access variable of the same class?
- Virag Kothari
On Dec. 14, 2012, 3:15 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 14, 2012, 3:15 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java 1421635
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1421635
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1421635
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1421635
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1421635
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1421635
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1421635
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1421635
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1421635
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1421635
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1421635
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1421635
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
(Updated Dec. 14, 2012, 3:15 a.m.)
Review request for oozie.
Changes
-------
Incorporated Virag's review comments
- added null check and subsequent instantiation of new connection much before in the logic
- removed list of callables and instead directly queue as callable
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1108
patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java 1421635
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1421635
branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1421635
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1421635
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1421635
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1421635
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1421635
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1421635
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1421635
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1421635
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1421635
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1421635
Diff: https://reviews.apache.org/r/8346/diff/
Testing
-------
yes
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
> On Dec. 11, 2012, 6:46 p.m., Virag Kothari wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java, line 302
> > <https://reviews.apache.org/r/8346/diff/4/?file=236766#file236766line302>
> >
> > A new connection should not be created for a new topic. Just updating the map with the topic name and receiver seems to be enough in this method.
If you see a couple of lines above, this is done only is connection to that particular endpoint does not exist beforehand. Endpoint is determined from the partition and passed to this method
> On Dec. 11, 2012, 6:46 p.m., Virag Kothari wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java, line 336
> > <https://reviews.apache.org/r/8346/diff/4/?file=236766#file236766line336>
> >
> > private
I think it is private by default. but doesnt hurt to add it
> On Dec. 11, 2012, 6:46 p.m., Virag Kothari wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java, line 548
> > <https://reviews.apache.org/r/8346/diff/4/?file=236767#file236767line548>
> >
> > the list should be synchronized. Also the list may grow very quickly.
> > So, instead of queueing a list of callables, is it not better to queue single command? In that way, the list will not be required
Following the same pattern as in RecoveryService, where a list is used. In turn, the CallableQueueService takes this list of callables and makes into a CompositeCallable (see method queueSerial).
- Mona
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14298
-----------------------------------------------------------
On Dec. 11, 2012, 4:29 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2012, 4:29 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14298
-----------------------------------------------------------
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30450>
A new connection should not be created for a new topic. Just updating the map with the topic name and receiver seems to be enough in this method.
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30451>
private
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/8346/#comment30453>
the list should be synchronized. Also the list may grow very quickly.
So, instead of queueing a list of callables, is it not better to queue single command? In that way, the list will not be required
- Virag Kothari
On Dec. 11, 2012, 4:29 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2012, 4:29 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
(Updated Dec. 11, 2012, 4:29 a.m.)
Review request for oozie.
Changes
-------
Addressed Virag's review comments.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1108
patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
Diff: https://reviews.apache.org/r/8346/diff/
Testing
-------
yes
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14098
-----------------------------------------------------------
Comments only for the receiver-handler 1-1 relationship code
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java
<https://reviews.apache.org/r/8346/#comment30165>
The earlier mapping in the 1st diff seemed fine to me. Plz look at other comments
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30160>
As this a generic JMS class, different consumers should be allowed to create from one session. Its better to leave this method unchanged.
I actually would prefer that you keep things similar as in 1st diff with the only change of having 1-1 relationship between MessageReceiver (listener) and MessageHandler. I have mentioned this change as a comment for _registerMessageReciever()
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/8346/#comment30162>
registerMessageReceiver(String topic){
MessageReceiver recvr = Services.get().get(JMSAccessorService.class).getTopicReceiver(topic);
if (recvr==null){
recvr = new MessageReceiver(new MessageHandler());
log.debug("Registering to listen on topic :" + topic);
recvr.registerTopic(topic);
}
if (recv!=null{
// do nothing
}
}
Wont the above code along with your 1st diff solve the problem of 1-1 relation between receiver and handler?
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/8346/#comment30163>
doing this will remove the existing handler for the receiver and set a new one.
- Virag Kothari
On Dec. 6, 2012, 1:04 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2012, 1:04 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
(Updated Dec. 6, 2012, 1:04 a.m.)
Review request for oozie.
Changes
-------
1. Added the queuing command feature in Partition Dep Manager
2. Added 'degistering' for topics from the JMS sessions map after there are no outstanding missing partitions for it
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1108
patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
Diff: https://reviews.apache.org/r/8346/diff/
Testing
-------
yes
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/
-----------------------------------------------------------
(Updated Dec. 5, 2012, 11:03 p.m.)
Review request for oozie.
Changes
-------
Addressed review comments. Finished patch with unit test and verified via end-to-end test
Description (updated)
-------
https://issues.apache.org/jira/browse/OOZIE-1108
patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
This addresses bug OOZIE-1108.
https://issues.apache.org/jira/browse/OOZIE-1108
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
Diff: https://reviews.apache.org/r/8346/diff/
Testing (updated)
-------
yes
Thanks,
Mona Chitnis
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
> On Dec. 5, 2012, 6:12 p.m., Virag Kothari wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java, line 211
> > <https://reviews.apache.org/r/8346/diff/1/?file=233983#file233983line211>
> >
> > The relationship between message handler and receiver is one to many, which will cause the messages from all consumers(receivers) to be handled by only one handler. Shouldn't it be 1-1?
You are probably right. Better not rely on the JMS Receiver's synchronization when multiple messages are to be handled simultaneously.
> On Dec. 5, 2012, 6:12 p.m., Virag Kothari wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java, line 66
> > <https://reviews.apache.org/r/8346/diff/1/?file=233982#file233982line66>
> >
> > its better to have this instance variable private and retrieve it through public method
okay
- Mona
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14053
-----------------------------------------------------------
On Dec. 5, 2012, 12:22 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2012, 12:22 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> WIP patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
> On Dec. 5, 2012, 6:12 p.m., Virag Kothari wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java, line 211
> > <https://reviews.apache.org/r/8346/diff/1/?file=233983#file233983line211>
> >
> > The relationship between message handler and receiver is one to many, which will cause the messages from all consumers(receivers) to be handled by only one handler. Shouldn't it be 1-1?
>
> Mona Chitnis wrote:
> You are probably right. Better not rely on the JMS Receiver's synchronization when multiple messages are to be handled simultaneously.
We have to rely on JMS Receiver's synchronization. However when all the synchronized receivers call the message handler, the handler will break if its not 1-1 relationship
- Virag
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14053
-----------------------------------------------------------
On Dec. 5, 2012, 12:22 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2012, 12:22 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> WIP patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14053
-----------------------------------------------------------
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
<https://reviews.apache.org/r/8346/#comment30061>
its better to have this instance variable private and retrieve it through public method
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/8346/#comment30060>
The relationship between message handler and receiver is one to many, which will cause the messages from all consumers(receivers) to be handled by only one handler. Shouldn't it be 1-1?
- Virag Kothari
On Dec. 5, 2012, 12:22 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2012, 12:22 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> WIP patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mona Chitnis
>
>
Re: Review Request: OOZIE-1108 Fix JMS message consumer to maintain single
session per topic registration
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8346/#review14039
-----------------------------------------------------------
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java
<https://reviews.apache.org/r/8346/#comment30041>
invoking method 'partitionAvailable' internally already calls CoordActionUpdatePushMissingDependency, and then available-map removes the entry. Hence removing these "stale-code" lines. Same goes for the place below
- Mona Chitnis
On Dec. 5, 2012, 12:22 a.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8346/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2012, 12:22 a.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1108
>
> WIP patch *on TOP* of OOZIE-1111 - the HCat URI changes. This patch also fixes a lot of affected test cases
>
>
> This addresses bug OOZIE-1108.
> https://issues.apache.org/jira/browse/OOZIE-1108
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java 1416645
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java 1416645
> branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java 1416645
>
> Diff: https://reviews.apache.org/r/8346/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mona Chitnis
>
>