Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
AM-19 commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2050429370 > Please add a quick unit test Have Added the same, Please have a look. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
AM-19 commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2050149863 > Please add a quick unit test Sure, Let me work on that -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
jbonofre commented on code in PR #1194:
URL: https://github.com/apache/activemq/pull/1194#discussion_r1560916109
##
activemq-broker/src/main/java/org/apache/activemq/broker/jmx/HealthView.java:
##
@@ -167,6 +169,25 @@ public List healthList() throws Exception {
}
}
+/**
+ * Check The Transport Connector limits
+ */
+
+if (brokerService != null &&
!brokerService.getTransportConnectors().isEmpty()) {
+ for(TransportConnector tc:
brokerService.getTransportConnectors()) {
+ if(tc.getServer() instanceof TcpTransportServer) {
+ int connectionUsage = (int)
(((TcpTransportServer)tc.getServer()).getCurrentTransportCount().get() * 100 )
/ ((TcpTransportServer)tc.getServer()).getMaximumConnections();
+
if(((TcpTransportServer)tc.getServer()).getCurrentTransportCount().get() >=
((TcpTransportServer)tc.getServer()).getMaximumConnections()) {
+ String message = "Exceeded the maximum
number of allowed client connections: " +
((TcpTransportServer)tc.getServer()).getMaximumConnections();
Review Comment:
Agree, just the limit is enough.
We can relay on the existing metrics for that.
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
jbonofre commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2049540751 > It seems we already have a check for File system usage. We have a check on the system usage, but not on the cursor. It's not super critical. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
mattrpav commented on code in PR #1194:
URL: https://github.com/apache/activemq/pull/1194#discussion_r1560887925
##
activemq-broker/src/main/java/org/apache/activemq/broker/jmx/HealthView.java:
##
@@ -167,6 +169,25 @@ public List healthList() throws Exception {
}
}
+/**
+ * Check The Transport Connector limits
+ */
+
+if (brokerService != null &&
!brokerService.getTransportConnectors().isEmpty()) {
+ for(TransportConnector tc:
brokerService.getTransportConnectors()) {
+ if(tc.getServer() instanceof TcpTransportServer) {
+ int connectionUsage = (int)
(((TcpTransportServer)tc.getServer()).getCurrentTransportCount().get() * 100 )
/ ((TcpTransportServer)tc.getServer()).getMaximumConnections();
+
if(((TcpTransportServer)tc.getServer()).getCurrentTransportCount().get() >=
((TcpTransportServer)tc.getServer()).getMaximumConnections()) {
+ String message = "Exceeded the maximum
number of allowed client connections: " +
((TcpTransportServer)tc.getServer()).getMaximumConnections();
Review Comment:
This check can’t happen since the connector will close a connection when it
reaches the limit. An exceeded count isn’t possible.
I think this branch of logic should just be removed and simply go with the
limit check
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
AM-19 commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2049205082 It seems we already have a check for File system usage. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
jbonofre commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2049196650 For JDBC yes, but no test on KahaDB. We can check the system usage for instance to see if the filesystem is not almost full. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
AM-19 commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2049172132 > This first step checking the transports looks good to me. I would also check the storage service. Ideally the above mentioned code for JDBC Persistence check will work, with the appropriate imports. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
jbonofre commented on PR #1194: URL: https://github.com/apache/activemq/pull/1194#issuecomment-2049061906 This first step checking the transports looks good to me. I would also check the storage service. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
AM-19 commented on PR #1192: URL: https://github.com/apache/activemq/pull/1192#issuecomment-2028855845 Hello @jbonofre Have Re-opened as #1194 Sorry for the inconvenience. Thanks Anubhav -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
AM-19 closed pull request #1192: [AMQ-9447] Added Logic to check underlying transport connector limits URL: https://github.com/apache/activemq/pull/1192 -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [AMQ-9447] Added Logic to check underlying transport connector limits [activemq]
jbonofre commented on PR #1192: URL: https://github.com/apache/activemq/pull/1192#issuecomment-2028824352 That's a good start, on Tcp transport. I will add additional more "specific" transport (like http/https/ws transport). -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
