activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glen Mazza <glen.ma...@verizon.net>
Subject Re: svn commit: r613830 - in /activemq/trunk/activemq-core/src/main/java/org/apache/activemq: broker/jmx/ broker/region/ broker/region/policy/ command/
Date Tue, 22 Jan 2008 02:51:56 GMT

Am Montag, den 21.01.2008, 10:31 +0000 schrieb rajdavies@apache.org:
> Author: rajdavies
> Date: Mon Jan 21 02:31:22 2008
> New Revision: 613830
> 
> URL: http://svn.apache.org/viewvc?rev=613830&view=rev
> Log:
> Fix for https://issues.apache.org/activemq/browse/AMQ-1510
> 
> Modified:
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerViewMBean.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/DestinationViewMBean.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/BaseDestination.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/Destination.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DestinationFactoryImpl.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DestinationFilter.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/Queue.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/RegionBroker.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/Topic.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/TopicRegion.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/policy/PolicyEntry.java
>     activemq/trunk/activemq-core/src/main/java/org/apache/activemq/command/Message.java
> 
> Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java
> URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java?rev=613830&r1=613829&r2=613830&view=diff
> ==============================================================================
> --- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java
(original)
> +++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java
Mon Jan 21 02:31:22 2008
> @@ -104,6 +104,32 @@

Something seems wrong with this class.  Looking at the four methods
here:

    public ObjectName[] getQueueSubscribers() {
        return broker.getQueueSubscribers();
    }

    public ObjectName[] getTemporaryQueueSubscribers() {
        return broker.getTemporaryQueueSubscribers();
    }

    public ObjectName[] getTopicSubscribers() {
        return broker.getTemporaryTopicSubscribers();
    }

    public ObjectName[] getTemporaryTopicSubscribers() {
        return broker.getTemporaryTopicSubscribers();
    }

I think the third needs to be:
"return broker.getTopicSubscribers()"



> +
> +    public int getStorePercentageUsed() {
> +        return brokerService.getSystemUsage().getStoreUsage().getPercentUsage();
> +    }
> +
> +    public int getTmpPercentageUsed() {
> +       return brokerService.getSystemUsage().getTempUsage().getPercentUsage();
> +    }
> 

There is a switch between "Usage"/"Used" naming here.  I can't recommend
a preference, but if those brokerServer.getXXXXUsage() methods are
externally available you may wish to standardize on one or the other.


> -    
>  }
> 
> Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerViewMBean.java
> URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerViewMBean.java?rev=613830&r1=613829&r2=613830&view=diff
> ==============================================================================
> --- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerViewMBean.java
(original)
> +++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/BrokerViewMBean.java
Mon Jan 21 02:31:22 2008
> @@ -20,6 +20,7 @@
>  
>  import org.apache.activemq.Service;
>  
> +
>  /**
>   * @author David Martin Clavo david(dot)martin(dot)clavo(at)gmail.com (for the reloadLog4jProperties
method)
>   * @version $Revision$
> @@ -65,6 +66,18 @@
>      long getMemoryLimit();
>  
>      void setMemoryLimit(long limit);
> +        
> +    int getStorePercentageUsed();
> +
> +    long getStoreLimit();
> +
> +    void setStoreLimit(long limit);
> +    
> +    int getTmpPercentageUsed();
> +
> +    long getTmpLimit();
> +
> +    void setTmpLimit(long limit);
>      

Normally, "Temp" is the abbreviation--are you sure you wish to go with
"Tmp" here?  No big deal, just wish to confirm.



>      boolean isPersistent();
>  
> 
> Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java
> URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java?rev=613830&r1=613829&r2=613830&view=diff
> ==============================================================================
> --- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java
(original)
> +++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java
Mon Jan 21 02:31:22 2008
> @@ -94,15 +94,15 @@
>      }
>  
>      public int getMemoryPercentageUsed() {
> -        return destination.getBrokerMemoryUsage().getPercentUsage();
> +        return destination.getMemoryUsage().getPercentUsage();
>      }
>  

Same usage/used comment as above.


>      public long getMemoryLimit() {
> -        return destination.getBrokerMemoryUsage().getLimit();
> +        return destination.getMemoryUsage().getLimit();
>      }
>  
>      public void setMemoryLimit(long limit) {
> -        destination.getBrokerMemoryUsage().setLimit(limit);
> +        destination.getMemoryUsage().setLimit(limit);
>      }
>  

There were two other methods added to this class that might cause
confusion with the above two methods:

> +    public float getMemoryLimitPortion() {
> +        return destination.getMemoryUsage().getUsagePortion();
> +    }
> +
> +    
> +    public void setMemoryLimitPortion(float value) {
> +        destination.getMemoryUsage().setUsagePortion(value);
> +    }
> +
> 

Might you have meant "getMemoryUsagePortion()"?



> Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java
> URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java?rev=613830&r1=613829&r2=613830&view=diff
> ==============================================================================
> --- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java
(original)
> +++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java
Mon Jan 21 02:31:22 2008
> @@ -17,11 +17,12 @@

DurableTopicSubscription exposes both the SubscriptionKey and methods
that access the two main values of SubscriptionKey:

    public SubscriptionKey getSubscriptionKey() {
        return subscriptionKey;
    }

    public String getClientId() {
        return subscriptionKey.getClientId();
    }

    public String getSubscriptionName() {
        return subscriptionKey.getSubscriptionName();
    }

Searching is showing that these methods are very seldom called, I would
guess the latter two can be removed, as that information is already
available from the first method.  (Alternatively, if it is desired to
encapsulate the key, to just remove the first method.)

Regards,
Glen


Mime
View raw message