You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/11/02 22:58:18 UTC
[GitHub] [james-project] ouvtam opened a new pull request, #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
ouvtam opened a new pull request, #1290:
URL: https://github.com/apache/james-project/pull/1290
…trieving ActiveMQ broker stats
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033354190
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
Yes sure. The overall feedback here is that it is (according to me) the metric stack job to store metrics state and that we do not need to store it ourselves.
I had a deeper look at dropwizard library. There is a handy object: `DefaultSettableGauge`...
```
metricRegistry.register("", metric);
metric.setValue(36L);
metric.setValue(2L);
```
That would be very handy to me to design a little wrapper around this class in `GaugeRegistry`:
```
public interface GaugeRegistry {
interface SettableGauge<T> {
void setValue(T t);
}
<T> GaugeRegistry register(String name, Gauge<T> gauge);
<T> SettableGauge<T> settableGauge(String name);
}
```
Then
```
public class DropWizardGaugeRegistry implements GaugeRegistry {
private final MetricRegistry metricRegistry;
@Inject
public DropWizardGaugeRegistry(MetricRegistry metricRegistry) {
this.metricRegistry = metricRegistry;
}
@Override
public <T> GaugeRegistry register(String name, Gauge<T> gauge) {
metricRegistry.gauge(name, () -> gauge::get);
return this;
}
@PreDestroy
public void shutDown() {
metricRegistry.getGauges().keySet().forEach(metricRegistry::remove);
}
@Override
public <T> SettableGauge<T> settableGauge(String name) {
DefaultSettableGauge<T> gauge = new DefaultSettableGauge<>();
metricRegistry.register(name, gauge);
return gauge::setValue;
}
}
```
Then in your AMQP metrics we could *directly* use these `SettableGauge` instead of the `Atomic*` fields...
What do you think about such a proposal?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1301590131
From a design perspective I do not like correlating metric collection with the healthcheck.
There are ways to obtain metrics directly from the broker (statistics on, JMX on):
```
System.out.println("memoryPercentUsage " + brokerService.getAdminView().getMemoryPercentUsage());
System.out.println("tempPercentUsage " + brokerService.getAdminView().getTempPercentUsage());
System.out.println("storePercentUsage " + brokerService.getAdminView().getStorePercentUsage());
System.out.println("memoryLimit " + brokerService.getAdminView().getMemoryLimit());
System.out.println("storeLimit " + brokerService.getAdminView().getStoreLimit());
System.out.println("tempLimit " + brokerService.getAdminView().getTempLimit());
System.out.println("dequeueCount " + brokerService.getAdminView().getTotalDequeueCount());
System.out.println("enqueueCount " + brokerService.getAdminView().getTotalEnqueueCount());
System.out.println("averageMessageSize " + brokerService.getAdminView().getAverageMessageSize());
```
Will yield:
```
memoryPercentUsage 1
tempPercentUsage 0
storePercentUsage 0
memoryLimit 1073741824
storeLimit 107374182400
tempLimit 53687091200
dequeueCount 1119
enqueueCount 4156
averageMessageSize 4097
```
This way you would not need to alter the health check to base the metric collection on it: the two would be completely indepandant, which is cleaner.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033407635
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
> I have to check if gauge::setValue imposes any chance for blocking code.
Blockhound will tell you but my bet is that no. Because dropwizrd relies on in-memory datastructures.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1034660235
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQMetricCollectorImpl.java:
##########
@@ -0,0 +1,178 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.queue.api.MailQueueName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import reactor.core.Disposable;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+public class ActiveMQMetricCollectorImpl implements ActiveMQMetricCollector {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(ActiveMQMetricCollectorImpl.class);
+
+ public static final Duration REFRESH_DELAY = Duration.ofSeconds(2);
+ public static final Duration REFRESH_INTERVAL = Duration.ofSeconds(5);
+ public static final Duration RECEIVE_TIMEOUT = Duration.ofSeconds(1);
+ public static final Duration REFRESH_TIMEOUT = RECEIVE_TIMEOUT.multipliedBy(2);
Review Comment:
Configuration option is available through conf/activemq.properties
Some thoughts about configuring `EmbeddedActiveMQ`:
- It might be possible to use an external ActiveMQ instance, but enable the embedded one by default `embedded=true`
- File locations for KahaDB and blobs
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1301744057
> Another option would be to separate collecting statistics and health check.
Where would be the best place to collect the broker stats? Add a broker stats collector via Guice ActiveMQQueueModule that runs in its own thread?
Should the stats collector also collecting stats for each "registered" queue (e.g. spool, outgoing)? For retrieving the size of a queue (see ActiveMQCacheableMailQueue#getSize) there will be also sent a message to the broker that might block. The getSize method is used by WebAdmin that exposes it via /mailQueues -> IMO this should also be decoupled...
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1301753256
> Where would be the best place to collect the broker stats? Add a broker stats collector via Guice ActiveMQQueueModule that runs in its own thread?
Just let the Gauge and the underlying metric library decide when to collect those metrics.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033355355
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQMetricCollectorImpl.java:
##########
@@ -0,0 +1,178 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.queue.api.MailQueueName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import reactor.core.Disposable;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+public class ActiveMQMetricCollectorImpl implements ActiveMQMetricCollector {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(ActiveMQMetricCollectorImpl.class);
+
+ public static final Duration REFRESH_DELAY = Duration.ofSeconds(2);
+ public static final Duration REFRESH_INTERVAL = Duration.ofSeconds(5);
+ public static final Duration RECEIVE_TIMEOUT = Duration.ofSeconds(1);
+ public static final Duration REFRESH_TIMEOUT = RECEIVE_TIMEOUT.multipliedBy(2);
Review Comment:
If that is the way we go down to, we could take time to identify other properties of `EmbeddedActiveMQ` worth configuring...
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1034656262
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
done!
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1301724865
It depends what health means. For instance, is ActiveMQ healthy when you can initiate a connection (old behaviour), but ActiveMQ reached a limit (memory, store, temp)?
In short, health before:
- Can connect to ActiveMQ
Health after this PR:
- Can connect to ActiveMQ
- Can send a message
- Receive a response in a specific timeout (currently 1s)
Furthermore, health check could be improved by checking the stats (e.g. tempPercentUsage < 90%, storePercentUsage < 90%, memoryPercentUsage < 90%).
Another option would be to separate collecting statistics and health check. However, the health check could access the information in the stats to determine if healthy.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033408040
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
> Should I apply your metrics-api change?
If you feel motivated to do this, sure, go for 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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033071022
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
If you want my opinion, maintaining ourselves a counter read by a gauge sounds overkill.
Dropwizard metrics comes with a `Histogram` that allows easily recording value.
This could be an idea of enhancement here.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033401753
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
That's what I thought. The metrics-api needs some additional interfaces. I did not want to change the metrics-api in the first place, that's why I used the Atomic* fields.
Overall your proposal looks good to me and some boilerplate code will be removed. I have to check if `gauge::setValue` imposes any chance for blocking code. Since I use `Mono#timeout` to limit `fetchAndUpdate` of the statistics/metrics.
Should I apply your metrics-api change?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1301731307
In itself I am OK with the healthcheck , however I do believe the gauge metric stuff should be independant from the health check. I am wishing to avoid temporal coupling here.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1328365536
- I reverted my changes to the health check in order to the separate collecting statistics and checking the health.
- ActiveMQMetricCollectorImpl takes care to collect statistics using reactor with an interval of 5 seconds and sane timeout values. TimerMetrics for collecting the statistics are also collected (*._time)
- Statistics about the broker and each created queue (i.e. outgoing, spool) will be collected
- Test coverage is available
- Please review ActiveMQMetricCollectorImpl#start carefully
Here is a sample output:
``` 23:06:06.262 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.averageEnqueueTime, value=1.2214895988112928
23:06:06.266 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.consumerCount, value=40
23:06:06.266 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.dequeueCount, value=0
23:06:06.266 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.dispatchCount, value=10508
23:06:06.266 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.enqueueCount, value=1487
23:06:06.266 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.expiredCount, value=0
23:06:06.267 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.inflightCount, value=10508
23:06:06.268 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.lastUpdate, value=1669590365575
23:06:06.268 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.maxEnqueueTime, value=143.0
23:06:06.268 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.memoryLimit, value=1073741824
23:06:06.268 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.memoryPercentUsage, value=0
23:06:06.269 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.memoryUsage, value=0
23:06:06.269 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.messagesCached, value=0
23:06:06.269 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.minEnqueueTime, value=1.0
23:06:06.269 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.producerCount, value=3
23:06:06.269 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.size, value=0
23:06:06.269 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.storeLimit, value=16339816520
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.storePercentUsage, value=0
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.storeUsage, value=135762
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.tempLimit, value=16339804160
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.tempPercentUsage, value=0
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Broker.tempUsage, value=0
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.averageEnqueueTime, value=0.0
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.consumerCount, value=1
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.dequeueCount, value=0
23:06:06.270 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.dispatchCount, value=0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.enqueueCount, value=0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.expiredCount, value=0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.inflightCount, value=0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.lastUpdate, value=1669590365533
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.maxEnqueueTime, value=0.0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.memoryLimit, value=1073741824
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.messagesCached, value=0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.minEnqueueTime, value=0.0
23:06:06.271 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.producerCount, value=1
23:06:06.272 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.outgoing.size, value=0
23:06:06.272 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.averageEnqueueTime, value=0.0
23:06:06.272 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.consumerCount, value=1
23:06:06.272 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.dequeueCount, value=0
23:06:06.272 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.dispatchCount, value=0
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.enqueueCount, value=0
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.expiredCount, value=0
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.inflightCount, value=0
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.lastUpdate, value=1669590365609
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.maxEnqueueTime, value=0.0
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.memoryLimit, value=1073741824
23:06:06.273 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.messagesCached, value=0
23:06:06.274 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.minEnqueueTime, value=0.0
23:06:06.276 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.producerCount, value=1
23:06:06.276 [DEBUG] o.a.j.metrics - type=GAUGE, name=ActiveMQ.Statistics.Destination.spool.size, value=0
23:06:06.340 [DEBUG] o.a.j.metrics - type=TIMER, name=ActiveMQ.Statistics.Broker._time, count=66, min=30.539776, max=289.406975, mean=51.27993406060606, stddev=31.783680823933842, p50=45.350911, p75=56.623103, p95=74.448895, p98=88.604671, p99=289.406975, p999=289.406975, m1_rate=0.20214070229262646, m5_rate=0.26901944823985086, m15_rate=0.34018704297266894, mean_rate=0.20265404074535695, rate_unit=events/second, duration_unit=milliseconds
23:06:06.341 [DEBUG] o.a.j.metrics - type=TIMER, name=ActiveMQ.Statistics.Destination.outgoing._time, count=66, min=30.015488, max=152.043519, mean=85.70023563636364, stddev=21.199560464617125, p50=85.458943, p75=98.041855, p95=125.304831, p98=135.266303, p99=152.043519, p999=152.043519, m1_rate=0.19920525480840734, m5_rate=0.199880531115036, m15_rate=0.1999834281769723, mean_rate=0.2025653469800763, rate_unit=events/second, duration_unit=milliseconds
23:06:06.343 [DEBUG] o.a.j.metrics - type=TIMER, name=ActiveMQ.Statistics.Destination.spool._time, count=66, min=26.345472, max=476.053503, mean=54.07315781818182, stddev=53.72771172053227, p50=46.137343, p75=55.050239, p95=66.584575, p98=96.468991, p99=476.053503, p999=476.053503, m1_rate=0.20177438323974942, m5_rate=0.26897689568049793, m15_rate=0.3401813951357432, mean_rate=0.2026923933224768, rate_unit=events/second, duration_unit=milliseconds```
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033402980
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQMetricCollectorImpl.java:
##########
@@ -0,0 +1,178 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.queue.api.MailQueueName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import reactor.core.Disposable;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+public class ActiveMQMetricCollectorImpl implements ActiveMQMetricCollector {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(ActiveMQMetricCollectorImpl.class);
+
+ public static final Duration REFRESH_DELAY = Duration.ofSeconds(2);
+ public static final Duration REFRESH_INTERVAL = Duration.ofSeconds(5);
+ public static final Duration RECEIVE_TIMEOUT = Duration.ofSeconds(1);
+ public static final Duration REFRESH_TIMEOUT = RECEIVE_TIMEOUT.multipliedBy(2);
Review Comment:
I am fine with this. I just wanted to keep this PR small. However, it totally makes sense to add some configuration options.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael merged pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
Arsnael merged PR #1290:
URL: https://github.com/apache/james-project/pull/1290
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033220106
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQMetricCollectorImpl.java:
##########
@@ -0,0 +1,178 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.queue.api.MailQueueName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import reactor.core.Disposable;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+public class ActiveMQMetricCollectorImpl implements ActiveMQMetricCollector {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(ActiveMQMetricCollectorImpl.class);
+
+ public static final Duration REFRESH_DELAY = Duration.ofSeconds(2);
+ public static final Duration REFRESH_INTERVAL = Duration.ofSeconds(5);
+ public static final Duration RECEIVE_TIMEOUT = Duration.ofSeconds(1);
+ public static final Duration REFRESH_TIMEOUT = RECEIVE_TIMEOUT.multipliedBy(2);
Review Comment:
> I think the intervals can be bigger (30s?)
I think the interval of 5s seems ok, since the mean time to retrieve those metrics is about 50-80ms when James is not under load.
> We could imagine also enabling configuring them (ENV variables?)
Sounds good. How about `conf/activemq.properties` to streamline to other modules (e.g. healthcheck.properties, jvm.properties, etc.)
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033502430
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
> If you feel motivated to do this, sure, go for it!
Ok, I will do so as part of this PR.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033223924
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQQueueStatistics.java:
##########
@@ -0,0 +1,155 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+
+import com.google.common.util.concurrent.AtomicDouble;
+
+public class ActiveMQQueueStatistics implements ActiveMQStatistics {
+ protected static final String MEMORY_LIMIT = "memoryLimit";
+
+ protected static final String SIZE = "size";
+ protected static final String ENQUEUE_COUNT = "enqueueCount";
+ protected static final String DEQUEUE_COUNT = "dequeueCount";
+ protected static final String INFLIGHT_COUNT = "inflightCount";
+ protected static final String PRODUCER_COUNT = "producerCount";
+ protected static final String CONSUMER_COUNT = "consumerCount";
+ protected static final String EXPIRED_COUNT = "expiredCount";
+ protected static final String DISPATCH_COUNT = "dispatchCount";
+ protected static final String MESSAGES_CACHED = "messagesCached";
+
+ protected static final String MIN_ENQUEUE_TIME = "minEnqueueTime";
+ protected static final String MAX_ENQUEUE_TIME = "maxEnqueueTime";
+ protected static final String AVERAGE_ENQUEUE_TIME = "averageEnqueueTime";
+
+ protected static final String LAST_UPDATE = "lastUpdate";
+
+ private final AtomicLong memoryLimit = new AtomicLong();
+
+ private final AtomicLong size = new AtomicLong();
+ private final AtomicLong enqueueCount = new AtomicLong();
+ private final AtomicLong dequeueCount = new AtomicLong();
+ private final AtomicLong inflightCount = new AtomicLong();
+ private final AtomicLong producerCount = new AtomicLong();
+ private final AtomicLong consumerCount = new AtomicLong();
+ private final AtomicLong expiredCount = new AtomicLong();
+ private final AtomicLong dispatchCount = new AtomicLong();
+ private final AtomicLong messagesCached = new AtomicLong();
+
+ private final AtomicDouble minEnqueueTime = new AtomicDouble();
+ private final AtomicDouble maxEnqueueTime = new AtomicDouble();
+ private final AtomicDouble averageEnqueueTime = new AtomicDouble();
Review Comment:
I see Dropwizard has such feature, but the metrics-api does not provide any interfaces yet. It only uses the histogram for timer metrics. Can you elaborate what you mean exactly?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1033055644
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQMetricCollectorImpl.java:
##########
@@ -0,0 +1,178 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.queue.activemq.metric;
+
+import java.time.Duration;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.JMSException;
+import javax.jms.MapMessage;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TemporaryQueue;
+
+import org.apache.james.metrics.api.GaugeRegistry;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.queue.api.MailQueueName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import reactor.core.Disposable;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+public class ActiveMQMetricCollectorImpl implements ActiveMQMetricCollector {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(ActiveMQMetricCollectorImpl.class);
+
+ public static final Duration REFRESH_DELAY = Duration.ofSeconds(2);
+ public static final Duration REFRESH_INTERVAL = Duration.ofSeconds(5);
+ public static final Duration RECEIVE_TIMEOUT = Duration.ofSeconds(1);
+ public static final Duration REFRESH_TIMEOUT = RECEIVE_TIMEOUT.multipliedBy(2);
Review Comment:
I think the intervals can be bigger (30s?)
We could imagine also enabling configuring them (ENV variables?)
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1290:
URL: https://github.com/apache/james-project/pull/1290#discussion_r1035458015
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/metric/ActiveMQMetricConfiguration.java:
##########
@@ -0,0 +1,94 @@
+package org.apache.james.queue.activemq.metric;
Review Comment:
Idem
##########
server/queue/queue-activemq/src/test/java/org/apache/james/queue/activemq/metric/ActiveMQMetricConfigurationTest.java:
##########
@@ -0,0 +1,72 @@
+package org.apache.james.queue.activemq.metric;
Review Comment:
Idem
##########
server/queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/ActiveMQConfiguration.java:
##########
@@ -0,0 +1,26 @@
+package org.apache.james.queue.activemq;
Review Comment:
This file is missing the license
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1331555846
We misses documentation for the added configuration (`src/site/xdoc/servers/config-activemq.xml` ?)
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] ouvtam commented on pull request #1290: [JAMES-3841] ActiveMQ: replace simple connection health check with re…
Posted by GitBox <gi...@apache.org>.
ouvtam commented on PR #1290:
URL: https://github.com/apache/james-project/pull/1290#issuecomment-1331713633
> We misses documentation for the added configuration (`src/site/xdoc/servers/config-activemq.xml` ?)
Thanks for the hint.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org