You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/05/04 21:35:00 UTC

[GitHub] [activemq-artemis] mtaylor opened a new pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

mtaylor opened a new pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] mtaylor commented on pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
mtaylor commented on pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#issuecomment-832668593


   retest


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] mtaylor commented on pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
mtaylor commented on pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#issuecomment-842083319


   @jbertram Hey Justin.  I added a section to address-model.md.  The slow-consumers.md did not seem appropriate.  Could you take a look.
   
   Thank you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit closed pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#discussion_r633907940



##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -3638,12 +3638,29 @@
             <xsd:element name="slow-consumer-threshold" type="xsd:long" maxOccurs="1" minOccurs="0">
                <xsd:annotation>
                   <xsd:documentation>
-                     The minimum rate of message consumption allowed before a consumer is considered "slow." Measured
-                     in messages-per-second.
+                     The minimum rate of message consumption allowed before a consumer is considered "slow."  Measurement
+                     unit is defined by the slow-consumer-threshold-measurement-unit parameter.  By default this is
+                     messages-per-seconds
                   </xsd:documentation>
                </xsd:annotation>
             </xsd:element>
 
+            <xsd:element name="slow-consumer-threshold-measurement-unit" maxOccurs="1" minOccurs="0">
+               <xsd:annotation>
+                  <xsd:documentation>
+                     The units used to measure the slow consumer threshold.  Default is messages-per-second.
+                  </xsd:documentation>
+               </xsd:annotation>
+               <xsd:simpleType>
+                  <xsd:restriction base="xsd:string">

Review comment:
       @clebertsuconic / @mtaylor this looks very similar to the period unit that is in the journal history, i assume we're just using TimeUnit under the covers, should we maybe extract the type and simply share it....




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] mtaylor commented on pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
mtaylor commented on pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#issuecomment-842083319


   @jbertram Hey Justin.  I added a section to address-model.md.  The slow-consumers.md did not seem appropriate.  Could you take a look.
   
   Thank you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#discussion_r633907940



##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -3638,12 +3638,29 @@
             <xsd:element name="slow-consumer-threshold" type="xsd:long" maxOccurs="1" minOccurs="0">
                <xsd:annotation>
                   <xsd:documentation>
-                     The minimum rate of message consumption allowed before a consumer is considered "slow." Measured
-                     in messages-per-second.
+                     The minimum rate of message consumption allowed before a consumer is considered "slow."  Measurement
+                     unit is defined by the slow-consumer-threshold-measurement-unit parameter.  By default this is
+                     messages-per-seconds
                   </xsd:documentation>
                </xsd:annotation>
             </xsd:element>
 
+            <xsd:element name="slow-consumer-threshold-measurement-unit" maxOccurs="1" minOccurs="0">
+               <xsd:annotation>
+                  <xsd:documentation>
+                     The units used to measure the slow consumer threshold.  Default is messages-per-second.
+                  </xsd:documentation>
+               </xsd:annotation>
+               <xsd:simpleType>
+                  <xsd:restriction base="xsd:string">

Review comment:
       @clebertsuconic / @mtaylor this looks very similar to the period unit that is in the journal history work Clebert has in PR, i assume we're just using TimeUnit under the covers, should we maybe extract the type and simply share it....




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#discussion_r633910939



##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -3638,12 +3638,29 @@
             <xsd:element name="slow-consumer-threshold" type="xsd:long" maxOccurs="1" minOccurs="0">
                <xsd:annotation>
                   <xsd:documentation>
-                     The minimum rate of message consumption allowed before a consumer is considered "slow." Measured
-                     in messages-per-second.
+                     The minimum rate of message consumption allowed before a consumer is considered "slow."  Measurement
+                     unit is defined by the slow-consumer-threshold-measurement-unit parameter.  By default this is
+                     messages-per-seconds
                   </xsd:documentation>
                </xsd:annotation>
             </xsd:element>
 
+            <xsd:element name="slow-consumer-threshold-measurement-unit" maxOccurs="1" minOccurs="0">
+               <xsd:annotation>
+                  <xsd:documentation>
+                     The units used to measure the slow consumer threshold.  Default is messages-per-second.
+                  </xsd:documentation>
+               </xsd:annotation>
+               <xsd:simpleType>
+                  <xsd:restriction base="xsd:string">

Review comment:
       e.g. could just have a shared type in the xsd that defines timeUnit and simple share that between Cleberts journal history time unit / period and yours.
   
       <xsd:simpleType name="timeUnit">
           <xsd:restriction base="xsd:string">
               <xsd:enumeration value="DAYS"/>
               <xsd:enumeration value="HOURS"/>
               <xsd:enumeration value="MINUTES"/>
               <xsd:enumeration value="SECONDS"/>
           </xsd:restriction>
       </xsd:simpleType>




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] mtaylor commented on pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
mtaylor commented on pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#issuecomment-837043963


   Sure no problem.  Thanks Justin


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] mtaylor commented on a change in pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
mtaylor commented on a change in pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#discussion_r634340321



##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -3638,12 +3638,29 @@
             <xsd:element name="slow-consumer-threshold" type="xsd:long" maxOccurs="1" minOccurs="0">
                <xsd:annotation>
                   <xsd:documentation>
-                     The minimum rate of message consumption allowed before a consumer is considered "slow." Measured
-                     in messages-per-second.
+                     The minimum rate of message consumption allowed before a consumer is considered "slow."  Measurement
+                     unit is defined by the slow-consumer-threshold-measurement-unit parameter.  By default this is
+                     messages-per-seconds
                   </xsd:documentation>
                </xsd:annotation>
             </xsd:element>
 
+            <xsd:element name="slow-consumer-threshold-measurement-unit" maxOccurs="1" minOccurs="0">
+               <xsd:annotation>
+                  <xsd:documentation>
+                     The units used to measure the slow consumer threshold.  Default is messages-per-second.
+                  </xsd:documentation>
+               </xsd:annotation>
+               <xsd:simpleType>
+                  <xsd:restriction base="xsd:string">

Review comment:
       I did consider using a time unit.  However, the measurement unit is really messages-per-second, messages-per-minute, I wanted to make it clear hence went with the more verbose option.  
   
   I suppose it could be changed to mean "messages-per-*TimeUnit*" but imo that's not as intuitive.  It doesn't use TimeUnit under the covers here it instead uses a "MeasurementUnit" enum (as the calculations were very simple).
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] jbertram commented on pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#issuecomment-836845743


   @mtaylor, I was about to merge this and noticed there were no documentation updates. Could you add something to `address-model.md` and `slow-consumers.md`? Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] mtaylor commented on a change in pull request #3567: ARTEMIS-3283 Added SlowConsumerThreshold unit configuration option

Posted by GitBox <gi...@apache.org>.
mtaylor commented on a change in pull request #3567:
URL: https://github.com/apache/activemq-artemis/pull/3567#discussion_r634340321



##########
File path: artemis-server/src/main/resources/schema/artemis-configuration.xsd
##########
@@ -3638,12 +3638,29 @@
             <xsd:element name="slow-consumer-threshold" type="xsd:long" maxOccurs="1" minOccurs="0">
                <xsd:annotation>
                   <xsd:documentation>
-                     The minimum rate of message consumption allowed before a consumer is considered "slow." Measured
-                     in messages-per-second.
+                     The minimum rate of message consumption allowed before a consumer is considered "slow."  Measurement
+                     unit is defined by the slow-consumer-threshold-measurement-unit parameter.  By default this is
+                     messages-per-seconds
                   </xsd:documentation>
                </xsd:annotation>
             </xsd:element>
 
+            <xsd:element name="slow-consumer-threshold-measurement-unit" maxOccurs="1" minOccurs="0">
+               <xsd:annotation>
+                  <xsd:documentation>
+                     The units used to measure the slow consumer threshold.  Default is messages-per-second.
+                  </xsd:documentation>
+               </xsd:annotation>
+               <xsd:simpleType>
+                  <xsd:restriction base="xsd:string">

Review comment:
       I did consider using a time unit.  However, the measurement unit is really messages-per-second, messages-per-minute, I wanted to make it clear hence went with the more verbose option.  
   
   I suppose it could be changed to mean "messages-per-<TimeUnit>" but imo that's not as intuitive.  It doesn't use TimeUnit under the covers here it instead uses a "MeasurementUnit" enum (as the calculations were very simple).
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org