activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rob Davies <rajdav...@gmail.com>
Subject Re: Advisory topic leakages (second theory)
Date Tue, 10 Jul 2007 11:43:40 GMT
Hi Manuel,

that's correct - it looks like there's leakage in trunk. Though  
before changing it back - I need to understand the implications of  
doing so
thanks for spotting this btw!

cheers,

Rob
On Jul 10, 2007, at 8:02 AM, Manuel Teira wrote:

> Rob Davies escribió:
>> hi Manuel,
>>
>> on the face of it - this does look like an issue was introduced by  
>> the change in trunk  you mentioned. I think the code was changed  
>> to help some concurrency issues. However, the broker has changed  
>> considerably since then, so I'm not sure it still applies. The  
>> best thing to do would be to apply your patch + rollback the  
>> change in AbstractBroker you highlighted, and see if anything  
>> breaks ;)
> Well, perhaps I didn't explain it correctly. The changes I was  
> talking about (destinations created inside the TopicRegion and not  
> being created from top) are only applied  in the trunk (509706),  
> not in  the 4.1 branch.
> The fact is that once I found the problem, and knowing that the  
> trunk was not suffering the leakage problem I'm behind, I wanted to  
> know the reason, and found these differences.
>
> What I would like to know is how is it now (now=trunk) supposed to  
> work. Changes in 509706 are, if I understand correctly, avoiding  
> the auto-created topics  to be exposed in the RegionBroker. So,  
> when the AdvisoryBroker tries to delete them, propagating the  
> removeDestination to the chained brokers, it won't find them. True?
>
> Regards.
>
>>
>> cheers,
>>
>> Rob
>>
>> On Jul 9, 2007, at 1:57 PM, Manuel Teira wrote:
>>
>>> And even worse, creating the destination in this way (inside the  
>>> TopicRegion) is not going to make the RegionBroker aware of it.  
>>> Since the AdvisoryBroker is relaying in the broker chain to try  
>>> to delete the consumer and producer advisory topics when a  
>>> destination is deleted, how and when are they supposed to be  
>>> deleted in the trunk code?
>>>
>>> Regards.
>>>
>>>
>>>
>>> Manuel Teira escribió:
>>>> Hi again.
>>>>
>>>> I'm afraid that the lookup code in AbstractRegion has being  
>>>> changed in the trunk (I was looking at 4.1 branch). Basically,  
>>>> instead of calling context.getBroker().addDestination 
>>>> (context,destination) to create the new destination,  
>>>> addDestination(context, destination) is used. This way, the  
>>>> advisory topic won't be created from top, but what happens with  
>>>> the advise in the code?
>>>>
>>>>                if(autoCreateDestinations){
>>>>                    // Try to auto create the destination... re- 
>>>> invoke broker from the
>>>>                    // top so that the proper security checks are  
>>>> performed.
>>>>                    try {
>>>>                        dest = addDestination(context, destination);
>>>>                        //context.getBroker().addDestination 
>>>> (context,destination);
>>>>                    }
>>>> I suppose that the assumption is no longer true.
>>>>
>>>> Also, the way to change this code (only commenting out the old  
>>>> one) makes me think about a not too mature change?
>>>>
>>>> If you can verify that this is the right way to proceed,I will  
>>>> like to prepare a patch against 4.1 branch.
>>>>
>>>>
>>>> Best regards.
>>>>
>>>>
>>>> Manuel Teira escribió:
>>>>> Hello again. Digging into the problem I've found another thing  
>>>>> related with an asymmetry in the way an advisory topic  is  
>>>>> created and destroyed.
>>>>>
>>>>> I'm analizying the way the Consumer and Producer advisory  
>>>>> topics for temporary queues are created and destroyed:
>>>>>
>>>>> An advisory topic is actually created when the AdvisoryBroker  
>>>>> fireAdvisory method is eventually sending the message. This is  
>>>>> happening in AbstractRegion lookup method, as the advisory  
>>>>> topic doesn't exist yet:
>>>>>
>>>>>    protected Destination lookup(ConnectionContext context,  
>>>>> ActiveMQDestination destination) throws Exception {
>>>>>        synchronized(destinationsMutex){
>>>>>            Destination dest=(Destination) destinations.get 
>>>>> (destination);
>>>>>            if(dest==null){
>>>>>                if(autoCreateDestinations){
>>>>>                    // Try to auto create the destination... re- 
>>>>> invoke broker from the
>>>>>                    // top so that the proper security checks  
>>>>> are performed.
>>>>>                    context.getBroker().addDestination 
>>>>> (context,destination);
>>>>>                    // We should now have the dest created.
>>>>>                    dest=(Destination) destinations.get 
>>>>> (destination);
>>>>>                }
>>>>>                if(dest==null){
>>>>>                    throw new JMSException("The destination  
>>>>> "+destination+" does not exist.");
>>>>>                }
>>>>>            }
>>>>>            return dest;
>>>>>        }
>>>>>    }
>>>>>
>>>>> Hence, the whole Broker chain is called to create a destination  
>>>>> (context.getBroker().addDestination), this, in a common  
>>>>> environment, involves calling:
>>>>>
>>>>> MutableBrokerFilter.addDestination - Just pass the request to  
>>>>> the next chained BrokerFilter
>>>>> [
>>>>>   Here the configured plugins
>>>>> }
>>>>> CompositeDestinationBroker. No implementation, so it passes the  
>>>>> request to the next chained object.
>>>>> AdvisoryBroker. Fires an advisory to the destination advisory  
>>>>> topic, and adds the destination to its own destinations map.
>>>>> TransactionBroker.  No implementation, passes the request to  
>>>>> the next chained object.
>>>>> RegionBroker. Delegates in the TopicRegion.addDestination to  
>>>>> create the given advisory topic.
>>>>>
>>>>> On the other way, this advisory topic is destroyed when the  
>>>>> advised destination is removed, in AdvisoryTopic.  
>>>>> removeDestinationInfo. But here, the way to do it is:
>>>>>
>>>>>    public void removeDestinationInfo(ConnectionContext context,  
>>>>> DestinationInfo destInfo) throws Exception{
>>>>>        next.removeDestinationInfo(context, destInfo);
>>>>>        DestinationInfo info = (DestinationInfo)  
>>>>> destinations.remove(destInfo.getDestination());
>>>>>
>>>>>        if( info !=null ) {
>>>>>            info.setDestination(destInfo.getDestination());
>>>>>            info.setOperationType 
>>>>> (DestinationInfo.REMOVE_OPERATION_TYPE);
>>>>>            ActiveMQTopic topic =  
>>>>> AdvisorySupport.getDestinationAdvisoryTopic 
>>>>> (destInfo.getDestination());
>>>>>            fireAdvisory(context, topic, info);
>>>>>            try {
>>>>>                next.removeDestination(context,  
>>>>> AdvisorySupport.getConsumerAdvisoryTopic(info.getDestination 
>>>>> ()), -1);
>>>>>            } catch (Exception  
>>>>> expectedIfDestinationDidNotExistYet) {
>>>>>            }
>>>>>            try {
>>>>>                next.removeDestination(context,  
>>>>> AdvisorySupport.getProducerAdvisoryTopic(info.getDestination 
>>>>> ()), -1);
>>>>>            } catch (Exception  
>>>>> expectedIfDestinationDidNotExistYet) {
>>>>>            }
>>>>>        }
>>>>>
>>>>>    }
>>>>>
>>>>>
>>>>> So, only the next chained broker components to AdvisoryBroker  
>>>>> are called to remove the consumer and producer advisory topics.  
>>>>> This way to proceed suggests me two problems:
>>>>>
>>>>> 1.-The advisory broker itself is not aware of the deletion of  
>>>>> those topics (remember that it had registered them when the  
>>>>> whole broker chain was called to create the topic). I think  
>>>>> that this is the leakage I'm suffering.
>>>>> 2.-Any plugin (or component in the chain preceding the  
>>>>> AdvisoryBroker) that could be creating and retaining objects  
>>>>> related with these advisory topics won't never be able to  
>>>>> release them.
>>>>>
>>>>> Perhaps this way to proceed could be related with the fix of  
>>>>> AMQ-677.
>>>>>
>>>>> Did I miss anything?
>>>>>
>>>>> Regards.
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>>
>> Rob Davies
>> 'Go further faster with Apache Camel!'
>> http://rajdavies.blogspot.com/
>>
>>
>>
>>
>


Mime
View raw message