You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2011/04/28 22:12:48 UTC
Re: Review Request: QPID-3235: clustered qpidd broker fails ocassionly the
cluster_tests.ShortTests.test_route_update
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/672/
-----------------------------------------------------------
(Updated 2011-04-28 20:12:48.042683)
Review request for qpid, Kenneth Giusti and Ted Ross.
Summary (updated)
-------
QPID-3235: clustered qpidd broker fails ocassionly the cluster_tests.ShortTests.test_route_update
Inconsistent stats changes on a Link were causing cluster
inconsistency. Fix is to disable those stats changes in a cluster.
Updated cluster_tests.py to reliably generate the error every time
without the fix.
Note this is also https://bugzilla.redhat.com/show_bug.cgi?id=675921
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Link.h 1096751
/trunk/qpid/cpp/src/qpid/broker/Link.cpp 1096751
/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1096751
/trunk/qpid/cpp/src/tests/cluster_tests.py 1096751
Diff: https://reviews.apache.org/r/672/diff
Testing
-------
Thanks,
Alan
Re: Review Request: QPID-3235: clustered qpidd broker fails ocassionly the
cluster_tests.ShortTests.test_route_update
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/672/#review615
-----------------------------------------------------------
Ship it!
- Kenneth
On 2011-04-28 20:12:48, Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/672/
> -----------------------------------------------------------
>
> (Updated 2011-04-28 20:12:48)
>
>
> Review request for qpid, Kenneth Giusti and Ted Ross.
>
>
> Summary
> -------
>
> QPID-3235: clustered qpidd broker fails ocassionly the cluster_tests.ShortTests.test_route_update
>
> Inconsistent stats changes on a Link were causing cluster
> inconsistency. Fix is to disable those stats changes in a cluster.
>
> Updated cluster_tests.py to reliably generate the error every time
> without the fix.
>
> Note this is also https://bugzilla.redhat.com/show_bug.cgi?id=675921
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1096751
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1096751
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1096751
> /trunk/qpid/cpp/src/tests/cluster_tests.py 1096751
>
> Diff: https://reviews.apache.org/r/672/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alan
>
>
Re: Review Request: QPID-3235: clustered qpidd broker fails ocassionly the
cluster_tests.ShortTests.test_route_update
Posted by Kenneth Giusti <kg...@apache.org>.
> On 2011-04-28 21:26:17, Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Link.cpp, line 73
> > <https://reviews.apache.org/r/672/diff/1/?file=17622#file17622line73>
> >
> > This patch looks fine, but wouldn't it be more betterer to check broker->isInCluster() and avoid allocating the agent & mgmtObject in the constructor?
>
> Alan Conway wrote:
> I thought about that but wasn't sure if it would cause problems to have no link object at all - and by implication no bridges. Do you think it would be better to eliminate them entirely from the management data?
Good point - I missed that. The bridge mgmt object assumes that the parent link mgmt object exists, and uses the parent link for it's own key. I can't imagine having one without the other.
I retract the comment.
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/672/#review604
-----------------------------------------------------------
On 2011-04-28 20:12:48, Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/672/
> -----------------------------------------------------------
>
> (Updated 2011-04-28 20:12:48)
>
>
> Review request for qpid, Kenneth Giusti and Ted Ross.
>
>
> Summary
> -------
>
> QPID-3235: clustered qpidd broker fails ocassionly the cluster_tests.ShortTests.test_route_update
>
> Inconsistent stats changes on a Link were causing cluster
> inconsistency. Fix is to disable those stats changes in a cluster.
>
> Updated cluster_tests.py to reliably generate the error every time
> without the fix.
>
> Note this is also https://bugzilla.redhat.com/show_bug.cgi?id=675921
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1096751
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1096751
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1096751
> /trunk/qpid/cpp/src/tests/cluster_tests.py 1096751
>
> Diff: https://reviews.apache.org/r/672/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alan
>
>
Re: Review Request: QPID-3235: clustered qpidd broker fails ocassionly the
cluster_tests.ShortTests.test_route_update
Posted by Alan Conway <ac...@redhat.com>.
> On 2011-04-28 21:26:17, Kenneth Giusti wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Link.cpp, line 73
> > <https://reviews.apache.org/r/672/diff/1/?file=17622#file17622line73>
> >
> > This patch looks fine, but wouldn't it be more betterer to check broker->isInCluster() and avoid allocating the agent & mgmtObject in the constructor?
I thought about that but wasn't sure if it would cause problems to have no link object at all - and by implication no bridges. Do you think it would be better to eliminate them entirely from the management data?
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/672/#review604
-----------------------------------------------------------
On 2011-04-28 20:12:48, Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/672/
> -----------------------------------------------------------
>
> (Updated 2011-04-28 20:12:48)
>
>
> Review request for qpid, Kenneth Giusti and Ted Ross.
>
>
> Summary
> -------
>
> QPID-3235: clustered qpidd broker fails ocassionly the cluster_tests.ShortTests.test_route_update
>
> Inconsistent stats changes on a Link were causing cluster
> inconsistency. Fix is to disable those stats changes in a cluster.
>
> Updated cluster_tests.py to reliably generate the error every time
> without the fix.
>
> Note this is also https://bugzilla.redhat.com/show_bug.cgi?id=675921
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1096751
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1096751
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1096751
> /trunk/qpid/cpp/src/tests/cluster_tests.py 1096751
>
> Diff: https://reviews.apache.org/r/672/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alan
>
>
Re: Review Request: QPID-3235: clustered qpidd broker fails ocassionly the
cluster_tests.ShortTests.test_route_update
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/672/#review604
-----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Link.cpp
<https://reviews.apache.org/r/672/#comment1248>
This patch looks fine, but wouldn't it be more betterer to check broker->isInCluster() and avoid allocating the agent & mgmtObject in the constructor?
- Kenneth
On 2011-04-28 20:12:48, Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/672/
> -----------------------------------------------------------
>
> (Updated 2011-04-28 20:12:48)
>
>
> Review request for qpid, Kenneth Giusti and Ted Ross.
>
>
> Summary
> -------
>
> QPID-3235: clustered qpidd broker fails ocassionly the cluster_tests.ShortTests.test_route_update
>
> Inconsistent stats changes on a Link were causing cluster
> inconsistency. Fix is to disable those stats changes in a cluster.
>
> Updated cluster_tests.py to reliably generate the error every time
> without the fix.
>
> Note this is also https://bugzilla.redhat.com/show_bug.cgi?id=675921
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Link.h 1096751
> /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1096751
> /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1096751
> /trunk/qpid/cpp/src/tests/cluster_tests.py 1096751
>
> Diff: https://reviews.apache.org/r/672/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alan
>
>