You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Pavel Moravec <pm...@redhat.com> on 2014/03/07 10:49:39 UTC
Review Request 18900: QPID-5608: [amqp1.0] delete-on-close policy do not
work for producers to exchanges
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18900/
-----------------------------------------------------------
Review request for qpid, Gordon Sim and Kenneth Giusti.
Bugs: QPID-5608
https://issues.apache.org/jira/browse/QPID-5608
Repository: qpid
Description
-------
The fix follows how IncomingToQueue class behave here for queues. Potential issues with the fix (causing me to raise the review request before committing it):
1) change in Exchange::decOtherUsers - cant it be done in more elegant way?
2) can't the petch in general cause problems to some other lifetime policies?
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Exchange.h 1574030
/trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1574030
/trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1574030
Diff: https://reviews.apache.org/r/18900/diff/
Testing
-------
automated tests seem passed, the reproducer from JIRA is fixed
Thanks,
Pavel Moravec
Re: Review Request 18900: QPID-5608: [amqp1.0] delete-on-close policy do not
work for producers to exchanges
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18900/#review36504
-----------------------------------------------------------
Ship it!
I think it is probably ok. The destroy doesn't actually delete the exchange object (that is controlled through shared_ptrs), it merely deletes it from the registry and signals deletion through management. Should be the same as a manual delete while the exchange is in use.
/trunk/qpid/cpp/src/qpid/broker/Exchange.cpp
<https://reviews.apache.org/r/18900/#comment67496>
Perhaps:
if (autodelete) {
if (isControllingLink) {
if (broker) broker->getExchanges().destroy(name);
} else if (!inUse() && !hasBindings()) {
checkAutoDelete();
}
}
is a bit clearer(?), but that is likely subjective.
- Gordon Sim
On March 7, 2014, 9:49 a.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18900/
> -----------------------------------------------------------
>
> (Updated March 7, 2014, 9:49 a.m.)
>
>
> Review request for qpid, Gordon Sim and Kenneth Giusti.
>
>
> Bugs: QPID-5608
> https://issues.apache.org/jira/browse/QPID-5608
>
>
> Repository: qpid
>
>
> Description
> -------
>
> The fix follows how IncomingToQueue class behave here for queues. Potential issues with the fix (causing me to raise the review request before committing it):
>
> 1) change in Exchange::decOtherUsers - cant it be done in more elegant way?
> 2) can't the petch in general cause problems to some other lifetime policies?
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Exchange.h 1574030
> /trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1574030
> /trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1574030
>
> Diff: https://reviews.apache.org/r/18900/diff/
>
>
> Testing
> -------
>
> automated tests seem passed, the reproducer from JIRA is fixed
>
>
> Thanks,
>
> Pavel Moravec
>
>