You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by ArthurFritz <gi...@git.apache.org> on 2018/10/26 17:40:57 UTC

[GitHub] activemq-artemis pull request #2398: [ARTEMIS-2150] Counts the number of del...

GitHub user ArthurFritz opened a pull request:

    https://github.com/apache/activemq-artemis/pull/2398

    [ARTEMIS-2150] Counts the number of delivering messages in this queue

    Create function this counts the number of delivering messages in this queue matching the specified filter.
    
    Create a other function this counts the number of delivering messages in this queue matching the specified filter, grouped by the given property field.
    
    [JIRA](https://issues.apache.org/jira/browse/ARTEMIS-2150)

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ArthurFritz/activemq-artemis feature/ARTEMIS-2150

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/2398.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2398
    
----
commit 1396ec6d825708994fa8d680f5c42480aacd410a
Author: Arthur Fritz Santiago <ar...@...>
Date:   2018-10-26T17:39:09Z

    [ARTEMIS-2150] Counts the number of delivering messages in this queue

----


---

[GitHub] activemq-artemis pull request #2398: [ARTEMIS-2150] Counts the number of del...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2398#discussion_r228623226
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java ---
    @@ -740,6 +733,56 @@ public String countMessages(final String filterStr, final String groupByProperty
           }
        }
     
    +   @Override
    +   public long countDeliveringMessages(final String filterStr) throws Exception {
    +      Long value = intenalCountDeliveryMessages(filterStr, null).get(null);
    +      return value == null ? 0 : value;
    +   }
    +
    +   @Override
    +   public String countDeliveringMessages(final String filterStr, final String groupByProperty) throws Exception {
    +      return JsonUtil.toJsonObject(intenalCountDeliveryMessages(filterStr, groupByProperty)).toString();
    +   }
    +
    +   private Map<String, Long> intenalCountDeliveryMessages(final String filterStr, final String groupByPropertyStr) throws Exception {
    +      checkStarted();
    +
    +      clearIO();
    +
    +      Map<String, Long> result = new HashMap<>();
    +      try {
    +         Filter filter = FilterImpl.createFilter(filterStr);
    +         SimpleString groupByProperty = SimpleString.toSimpleString(groupByPropertyStr);
    +         if (filter == null && groupByProperty == null) {
    +            result.put(null, Long.valueOf(getDeliveringCount()));
    +         } else {
    +            Map<String, List<MessageReference>> deliveringMessages = queue.getDeliveringMessages();
    --- End diff --
    
    actually on further looking i see benefit in what you're doing, it keeps management logic and features out the core queue and consumer code, so actually +1 your approach.


---

[GitHub] activemq-artemis pull request #2398: ARTEMIS-2150 Counts the number of deliv...

Posted by ArthurFritz <gi...@git.apache.org>.
Github user ArthurFritz closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/2398


---

[GitHub] activemq-artemis issue #2398: ARTEMIS-2150 Counts the number of delivering m...

Posted by ArthurFritz <gi...@git.apache.org>.
Github user ArthurFritz commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2398
  
    Okay, i created new Merge Request, adjusted commit message :smiley: 


---

[GitHub] activemq-artemis pull request #2398: [ARTEMIS-2150] Counts the number of del...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2398#discussion_r228619277
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java ---
    @@ -740,6 +733,56 @@ public String countMessages(final String filterStr, final String groupByProperty
           }
        }
     
    +   @Override
    +   public long countDeliveringMessages(final String filterStr) throws Exception {
    +      Long value = intenalCountDeliveryMessages(filterStr, null).get(null);
    +      return value == null ? 0 : value;
    +   }
    +
    +   @Override
    +   public String countDeliveringMessages(final String filterStr, final String groupByProperty) throws Exception {
    +      return JsonUtil.toJsonObject(intenalCountDeliveryMessages(filterStr, groupByProperty)).toString();
    +   }
    +
    +   private Map<String, Long> intenalCountDeliveryMessages(final String filterStr, final String groupByPropertyStr) throws Exception {
    +      checkStarted();
    +
    +      clearIO();
    +
    +      Map<String, Long> result = new HashMap<>();
    +      try {
    +         Filter filter = FilterImpl.createFilter(filterStr);
    +         SimpleString groupByProperty = SimpleString.toSimpleString(groupByPropertyStr);
    +         if (filter == null && groupByProperty == null) {
    +            result.put(null, Long.valueOf(getDeliveringCount()));
    +         } else {
    +            Map<String, List<MessageReference>> deliveringMessages = queue.getDeliveringMessages();
    --- End diff --
    
     getDeliveringMessages() doesn't return a thead safe reference, as such the list can change beneath. this will expose thread safety issues


---

[GitHub] activemq-artemis issue #2398: [ARTEMIS-2150] Counts the number of delivering...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2398
  
    @ArthurFritz LGTM (when build passes for checkstyle etc etc) could you merge the commits? Also could you tidy the commit message to be in the form:
    
    "ARTEMIS-XXXX You Commit Subject"
    



---