[ 
https://issues.apache.org/jira/browse/ARTEMIS-5121?focusedWorklogId=971590&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-971590
 ]

ASF GitHub Bot logged work on ARTEMIS-5121:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 03/Jun/25 04:47
            Start Date: 03/Jun/25 04:47
    Worklog Time Spent: 10m 
      Work Description: john9x commented on code in PR #5719:
URL: https://github.com/apache/activemq-artemis/pull/5719#discussion_r2122657528


##########
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java:
##########
@@ -207,16 +204,13 @@ public boolean addSubscription(MqttTopicSubscription 
subscription, WildcardConfi
 
          SubscriptionItem existingSubscription = 
subscriptions.get(subscription.topicFilter());
          if (existingSubscription != null) {
-            boolean updated = false;
-            if (subscription.qualityOfService().value() > 
existingSubscription.getSubscription().qualityOfService().value()) {
-               existingSubscription.setSubscription(subscription);
-               updated = true;
+            if (subscription.qualityOfService().value() > 
existingSubscription.getSubscription().qualityOfService().value()
+               || !Objects.equals(subscriptionIdentifier, 
existingSubscription.getId())) {
+               existingSubscription.update(subscription, 
subscriptionIdentifier);
+               return true;
+            } else {
+               return false;
             }
-            if (subscriptionIdentifier != null && 
!subscriptionIdentifier.equals(existingSubscription.getId())) {

Review Comment:
   @jbertram I'm not sure but I suspect there was a bug
   
   >[3.8.2.1.2] The Subscription Identifier is associated with any subscription 
created or modified as the result of this SUBSCRIBE packet. If there is a 
Subscription Identifier, it is stored with the subscription. If this property 
is not specified, then the absence of a Subscription Identifier is stored with 
the subscription.
   
   ```
   if (subscriptionIdentifier != null
   ```
   means that previous non-NULL ID will not to be replaced with NULL ID if new 
SUBSCRIBE does not contains subscription identifier.





Issue Time Tracking
-------------------

    Worklog Id:     (was: 971590)
    Time Spent: 1h  (was: 50m)

> Improve performance of SUBSCRIPTION_IDENTIFIER fetching from MQTT session 
> state
> -------------------------------------------------------------------------------
>
>                 Key: ARTEMIS-5121
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-5121
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>          Components: MQTT
>    Affects Versions: 2.37.0
>            Reporter: Adam Zyzak
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> Recently I performed stress test of a system using Artemis as a message 
> broker for MQTT and STOMP protocols. I noticed huge difference in performance 
> and CPU usage between these protocols.
> I made some jstacks and it shown that majority of time is spent in method 
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTSessionState#getMatchingSubscriptionIdentifiers
>  compiling regular expressions. 
> It would be helpful if performance of this method was improved, especially 
> since it is executed on every publish, either by caching compiled patterns, 
> adding special cases for simpler topic names (topic names without special 
> characters may be compared using equality, # subscriptions can be always 
> added and topic names having # at the end can be compared using startsWith) 
> or just adding option to configuration allowing to skip adding 
> SUBSCRIPTION_IDENTIFIER to publish message.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact


Reply via email to