You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by rajith attapattu <ra...@gmail.com> on 2013/01/18 23:35:34 UTC
Review Request: Add the ability to turn message replay on/off.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9027/
-----------------------------------------------------------
Review request for qpid, Robbie Gemmell, Weston Price, and Keith Wall.
Description
-------
The XASessionImpl is using AUTO_ACK as the ack and when failover occurs messages are replayed.
This patch adds the ability to Add the ability to turn message replay on/off based on a boolean parameter when creating a session.
This addresses bug QPID-4541.
https://issues.apache.org/jira/browse/QPID-4541
Diffs
-----
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/example/src/main/java/org/apache/qpid/example/ListReceiver.java 1435368
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/example/src/main/java/org/apache/qpid/example/ListSender.java 1435368
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java 1435368
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQSession_0_10Test.java 1435368
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/message/AMQPEncodedListMessageUnitTest.java 1435368
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1435368
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1435368
Diff: https://reviews.apache.org/r/9027/diff/
Testing
-------
Verified with our existing test suite to ensure that this changes does not cause any unwanted side affects.
Weston Price has done testing in an XA envirnoment to verify that this fix has succesfully resolved QPID-4541.
Thanks,
rajith attapattu
Re: Review Request: Add the ability to turn message replay on/off.
Posted by rajith attapattu <ra...@gmail.com>.
> On Jan. 24, 2013, 9:38 a.m., Keith Wall wrote:
> > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java, line 91
> > <https://reviews.apache.org/r/9027/diff/2/?file=250239#file250239line91>
> >
> > Rather than extending the argument list, I think the code would be cleaner if you followed the same pattern used by AMQSession_0_10 to inform o.a.q.t.Session that normal (non-dtx) sessions are in used.
> >
> > public void createSession()
> > {
> > _qpidDtxSession = getQpidConnection().createSession(0);
> > _qpidDtxSession.setSession....;
> > _qpidDtxSession.dtxSelect();
> > _qpidDtxSession.setDtxTransacted(true); ## inspired by _qpidSession..setTransacted(true) in AMQSession_0_10 line 188
> > }
> >
Keith I don't think that's a good idea.
Transacted or noReplay should not be mutable. Once you make a session transacted, it should remain so. In that sense a setter is wrong.
(The same applies for setTransacted(true) as well).
These immutable properties should be set at creation time, and constructor is the best place to do so.
Also No-Replay is interesting outside of the dtx case. Some users will not care about replay. They might use failover for HA but not really for reliability.
At present we just force them to use both. So I wouldn't specifically narrow it's naming to dtx.
- rajith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9027/#review15629
-----------------------------------------------------------
On Jan. 18, 2013, 10:54 p.m., rajith attapattu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9027/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2013, 10:54 p.m.)
>
>
> Review request for qpid, Robbie Gemmell, Weston Price, and Keith Wall.
>
>
> Description
> -------
>
> The XASessionImpl is using AUTO_ACK as the ack and when failover occurs messages are replayed.
> This patch adds the ability to Add the ability to turn message replay on/off based on a boolean parameter when creating a session.
>
>
> This addresses bug QPID-4541.
> https://issues.apache.org/jira/browse/QPID-4541
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java 1435401
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQSession_0_10Test.java 1435401
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1435401
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1435401
>
> Diff: https://reviews.apache.org/r/9027/diff/
>
>
> Testing
> -------
>
> Verified with our existing test suite to ensure that this changes does not cause any unwanted side affects.
> Weston Price has done testing in an XA envirnoment to verify that this fix has succesfully resolved QPID-4541.
>
>
> Thanks,
>
> rajith attapattu
>
>
Re: Review Request: Add the ability to turn message replay on/off.
Posted by Keith Wall <ke...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9027/#review15629
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java
<https://reviews.apache.org/r/9027/#comment33743>
Rather than extending the argument list, I think the code would be cleaner if you followed the same pattern used by AMQSession_0_10 to inform o.a.q.t.Session that normal (non-dtx) sessions are in used.
public void createSession()
{
_qpidDtxSession = getQpidConnection().createSession(0);
_qpidDtxSession.setSession....;
_qpidDtxSession.dtxSelect();
_qpidDtxSession.setDtxTransacted(true); ## inspired by _qpidSession..setTransacted(true) in AMQSession_0_10 line 188
}
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQSession_0_10Test.java
<https://reviews.apache.org/r/9027/#comment33729>
Typo in new formal parameter name.
- Keith Wall
On Jan. 18, 2013, 10:54 p.m., rajith attapattu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9027/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2013, 10:54 p.m.)
>
>
> Review request for qpid, Robbie Gemmell, Weston Price, and Keith Wall.
>
>
> Description
> -------
>
> The XASessionImpl is using AUTO_ACK as the ack and when failover occurs messages are replayed.
> This patch adds the ability to Add the ability to turn message replay on/off based on a boolean parameter when creating a session.
>
>
> This addresses bug QPID-4541.
> https://issues.apache.org/jira/browse/QPID-4541
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java 1435401
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQSession_0_10Test.java 1435401
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1435401
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1435401
>
> Diff: https://reviews.apache.org/r/9027/diff/
>
>
> Testing
> -------
>
> Verified with our existing test suite to ensure that this changes does not cause any unwanted side affects.
> Weston Price has done testing in an XA envirnoment to verify that this fix has succesfully resolved QPID-4541.
>
>
> Thanks,
>
> rajith attapattu
>
>
Re: Review Request: Add the ability to turn message replay on/off.
Posted by rajith attapattu <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9027/
-----------------------------------------------------------
(Updated Jan. 18, 2013, 10:54 p.m.)
Review request for qpid, Robbie Gemmell, Weston Price, and Keith Wall.
Description
-------
The XASessionImpl is using AUTO_ACK as the ack and when failover occurs messages are replayed.
This patch adds the ability to Add the ability to turn message replay on/off based on a boolean parameter when creating a session.
This addresses bug QPID-4541.
https://issues.apache.org/jira/browse/QPID-4541
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java 1435401
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/AMQSession_0_10Test.java 1435401
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1435401
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java 1435401
Diff: https://reviews.apache.org/r/9027/diff/
Testing
-------
Verified with our existing test suite to ensure that this changes does not cause any unwanted side affects.
Weston Price has done testing in an XA envirnoment to verify that this fix has succesfully resolved QPID-4541.
Thanks,
rajith attapattu