[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456724549 @franz1981 yup agree there are some improvements we could have done, but have left them out As right now, the main part is dealing with the behaviour/latency regression and still addressing 1451's concerns of sync blocks. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456724549 @franz1981 yup agree there are some improvements we could have done, but have left them out As right now, the main part is dealing with the behaviour/latency regression and still addressing 1451's concerns of sync blocks. As need this fixed also in 2.6.x branch as well. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456724549 @franz1981 yup agree there are some improvements we could have done, but have left them out. As right now, the main priority (and this PR) is dealing with the behaviour/latency regression and still addressing 1451's concerns of sync blocks. As need this fixed also in 2.6.x branch as well. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on a change in pull request #2514: ARTEMIS-2236 Address
Latency Impact caused by ARTEMIS-1451
URL: https://github.com/apache/activemq-artemis/pull/2514#discussion_r250105709
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadPoolExecutor.java
##
@@ -29,33 +29,129 @@
* and will be removed after idling for a specified keep time.
* But in contrast to a standard cached executor, tasks are queued if the
* maximum pool size if reached, instead of rejected.
+ *
+ * This is achieved by using a specialized blocking queue, which checks the
+ * state of the associated executor in the offer method to decide whether to
+ * queue a task or have the executor create another thread.
+ *
+ * Since the thread pool's execute method is reentrant, more than one caller
+ * could try to offer a task into the queue. There is a small chance that
+ * (a few) more threads are created as it should be limited by max pool size.
+ * To allow for such a case not to reject a task, the underlying thread pool
+ * executor is not limited. Only the offer method checks the configured limit.
*/
public class ActiveMQThreadPoolExecutor extends ThreadPoolExecutor {
- // Handler executed when a task is submitted and a new thread cannot be
created (because maxSize was reached)
- // It queues the task on the executors's queue (using the add() method, see
ThreadPoolQueue class below)
- private static final RejectedExecutionHandler QUEUE_EXECUTION_HANDLER = (r,
e) -> {
- if (!e.isShutdown()) {
- e.getQueue().add(r);
- }
- };
-
- // A specialized LinkedBlockingQueue that takes new elements by calling
add() but not offer()
- // This is to force the ThreadPoolExecutor to always create new threads and
never queue
+ @SuppressWarnings("serial")
private static class ThreadPoolQueue extends LinkedBlockingQueue {
+ private ActiveMQThreadPoolExecutor executor = null;
+
+ // keep track of the difference between the number of idle threads and
+ // the number of queued tasks. If the delta is > 0, we have more
+ // idle threads than queued tasks and can add more tasks into the queue.
+ // The delta is incremented if a thread becomes idle or if a task is
taken from the queue.
+ // The delta is decremented if a thread leaves idle state or if a task
is added to the queue.
+ private static final AtomicIntegerFieldUpdater
DELTA_UPDATER = AtomicIntegerFieldUpdater.newUpdater(ThreadPoolQueue.class,
"threadTaskDelta");
+ private volatile int threadTaskDelta = 0;
+
+ public void setExecutor(ActiveMQThreadPoolExecutor executor) {
+ this.executor = executor;
+ }
+
@Override
public boolean offer(Runnable runnable) {
- return false;
+ boolean retval = false;
+
+ if (threadTaskDelta > 0 || (executor.getPoolSize() >=
executor.getMaximumPoolSize())) {
+// A new task will be added to the queue if the maximum number of
threads has been reached
+// or if the delta is > 0, which means that there are enough idle
threads.
+
+retval = super.offer(runnable);
+
+// Only decrement the delta if the task has actually been added to
the queue
+if (retval)
+ DELTA_UPDATER.incrementAndGet(this);
+ }
+
+
+ return retval;
+ }
+
+ @Override
+ public Runnable take() throws InterruptedException {
+ // Increment the delta as a thread becomes idle
+ // by waiting for a task to take from the queue
+ DELTA_UPDATER.incrementAndGet(this);
+
+
+ Runnable runnable = null;
+
+ try {
+runnable = super.take();
+return runnable;
+ } finally {
+// Now the thread is no longer idle waiting for a task
+// If it had taken a task, the delta remains the same
+// (decremented by the thread and incremented by the taken task)
+// Only if no task had been taken, we have to decrement the delta.
+if (runnable == null) {
+ DELTA_UPDATER.decrementAndGet(this);
+
+}
+ }
}
@Override
- public boolean add(Runnable runnable) {
- return super.offer( runnable );
+ public Runnable poll(long arg0, TimeUnit arg2) throws
InterruptedException {
+ // Increment the delta as a thread becomes idle
+ // by waiting for a task to poll from the queue
+ DELTA_UPDATER.incrementAndGet(this);
+
+
+ Runnable runnable = null;
+
+ try {
+runnable = super.poll(arg0, arg2);
+ } finally {
+// Now the thread is no longer idle waiting for a task
+// If it had taken a task, the delta remains the same
+// (decremented by the thread and incremented by the taken task)
+if (runnable == null) {
+
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725073 @michaelandrepearce Yep, but consider that providing a lock-free implementation *is* an improvement that need to be reviewed: is not just a plain revert... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725467 @franz1981 no indeed, and thats not what im doing. Im reverting but also removed the need for the sync blocks, and using atomic updater to the threadTaskDelta which was the original 1451 goal. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on a change in pull request #2514: ARTEMIS-2236 Address
Latency Impact caused by ARTEMIS-1451
URL: https://github.com/apache/activemq-artemis/pull/2514#discussion_r250105709
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ActiveMQThreadPoolExecutor.java
##
@@ -29,33 +29,129 @@
* and will be removed after idling for a specified keep time.
* But in contrast to a standard cached executor, tasks are queued if the
* maximum pool size if reached, instead of rejected.
+ *
+ * This is achieved by using a specialized blocking queue, which checks the
+ * state of the associated executor in the offer method to decide whether to
+ * queue a task or have the executor create another thread.
+ *
+ * Since the thread pool's execute method is reentrant, more than one caller
+ * could try to offer a task into the queue. There is a small chance that
+ * (a few) more threads are created as it should be limited by max pool size.
+ * To allow for such a case not to reject a task, the underlying thread pool
+ * executor is not limited. Only the offer method checks the configured limit.
*/
public class ActiveMQThreadPoolExecutor extends ThreadPoolExecutor {
- // Handler executed when a task is submitted and a new thread cannot be
created (because maxSize was reached)
- // It queues the task on the executors's queue (using the add() method, see
ThreadPoolQueue class below)
- private static final RejectedExecutionHandler QUEUE_EXECUTION_HANDLER = (r,
e) -> {
- if (!e.isShutdown()) {
- e.getQueue().add(r);
- }
- };
-
- // A specialized LinkedBlockingQueue that takes new elements by calling
add() but not offer()
- // This is to force the ThreadPoolExecutor to always create new threads and
never queue
+ @SuppressWarnings("serial")
private static class ThreadPoolQueue extends LinkedBlockingQueue {
+ private ActiveMQThreadPoolExecutor executor = null;
+
+ // keep track of the difference between the number of idle threads and
+ // the number of queued tasks. If the delta is > 0, we have more
+ // idle threads than queued tasks and can add more tasks into the queue.
+ // The delta is incremented if a thread becomes idle or if a task is
taken from the queue.
+ // The delta is decremented if a thread leaves idle state or if a task
is added to the queue.
+ private static final AtomicIntegerFieldUpdater
DELTA_UPDATER = AtomicIntegerFieldUpdater.newUpdater(ThreadPoolQueue.class,
"threadTaskDelta");
+ private volatile int threadTaskDelta = 0;
+
+ public void setExecutor(ActiveMQThreadPoolExecutor executor) {
+ this.executor = executor;
+ }
+
@Override
public boolean offer(Runnable runnable) {
- return false;
+ boolean retval = false;
+
+ if (threadTaskDelta > 0 || (executor.getPoolSize() >=
executor.getMaximumPoolSize())) {
+// A new task will be added to the queue if the maximum number of
threads has been reached
+// or if the delta is > 0, which means that there are enough idle
threads.
+
+retval = super.offer(runnable);
+
+// Only decrement the delta if the task has actually been added to
the queue
+if (retval)
+ DELTA_UPDATER.incrementAndGet(this);
+ }
+
+
+ return retval;
+ }
+
+ @Override
+ public Runnable take() throws InterruptedException {
+ // Increment the delta as a thread becomes idle
+ // by waiting for a task to take from the queue
+ DELTA_UPDATER.incrementAndGet(this);
+
+
+ Runnable runnable = null;
+
+ try {
+runnable = super.take();
+return runnable;
+ } finally {
+// Now the thread is no longer idle waiting for a task
+// If it had taken a task, the delta remains the same
+// (decremented by the thread and incremented by the taken task)
+// Only if no task had been taken, we have to decrement the delta.
+if (runnable == null) {
+ DELTA_UPDATER.decrementAndGet(this);
+
+}
+ }
}
@Override
- public boolean add(Runnable runnable) {
- return super.offer( runnable );
+ public Runnable poll(long arg0, TimeUnit arg2) throws
InterruptedException {
+ // Increment the delta as a thread becomes idle
+ // by waiting for a task to poll from the queue
+ DELTA_UPDATER.incrementAndGet(this);
+
+
+ Runnable runnable = null;
+
+ try {
+runnable = super.poll(arg0, arg2);
+ } finally {
+// Now the thread is no longer idle waiting for a task
+// If it had taken a task, the delta remains the same
+// (decremented by the thread and incremented by the taken task)
+if (runnable == null) {
+
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725789 > I'm reverting but also removed the need for the sync blocks, and using atomic updater to the threadTaskDelta Let me take a look to the original code and I can tell you :+1: This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725467 @franz1981 no indeed, and thats not what im doing. Im reverting but also removed the need for the sync blocks(which was the original 1451 goal.), and using atomic updater to the threadTaskDelta This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725467 @franz1981 no indeed, and thats not what im doing. Im reverting but also removed the need for the sync blocks, and using atomic updater to the threadTaskDelta This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456709895 To show this, using a broker with persistance disabled (to remove external disk perf factors) Using CORE Producer -> Consume Result is latency in microseconds average per 1 messages (20 runs) CURRENT MASTER RESULT: 292 RESULT: 187 RESULT: 151 RESULT: 124 RESULT: 109 RESULT: 148 RESULT: 121 RESULT: 127 RESULT: 104 RESULT: 201 RESULT: 118 RESULT: 116 RESULT: 115 RESULT: 153 RESULT: 126 RESULT: 127 RESULT: 107 RESULT: 128 RESULT: 121 RESULT: 126 WITH REVERTING AND APPLYING THIS FIX RESULT: 249 RESULT: 173 RESULT: 150 RESULT: 120 RESULT: 127 RESULT: 124 RESULT: 140 RESULT: 122 RESULT: 106 RESULT: 100 RESULT: 96 RESULT: 104 RESULT: 103 RESULT: 96 RESULT: 92 RESULT: 94 RESULT: 100 RESULT: 91 RESULT: 101 RESULT: 100 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456708382 ARTEMIS-1451 removed synchronization in the class, by adding the overhead of creating a new thread until threadpool is full, this though means that instead of re-using an existing idle thread for new work, new threads are being created which is expensive, and which is the point of thread pooling. We have seen a 15-20% latency increase hit since 2.4.0 due to this. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456711246 Suggestion: i would use something more precise (with an higher spectrum of latencies) if possible to make things comparable: https://github.com/franz1981/artemis-load-generator Change the core version you need and run it with: ```java -jar destination-bench.jar --bytes 100 --protocol artemis --url tcp://localhost:61616 ---out /tmp/latencies.txt --name q --iterations 10 --runs 1 --warmup 10 --wait 2``` It will give you the latencies over an all out throughout: then you need to choose a sustainable throughput eg 10 and re-run the same test adding `--target 10`, then you will have comparable latencies. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456709895 To show this, using a broker with persistance disabled (to remove external disk perf factors), with localbroker but using netty (to remove network external factor) Using CORE Produce -> Consume Result is latency in microseconds average per 1 messages (20 runs) CURRENT MASTER RESULT: 292 RESULT: 187 RESULT: 151 RESULT: 124 RESULT: 109 RESULT: 148 RESULT: 121 RESULT: 127 RESULT: 104 RESULT: 201 RESULT: 118 RESULT: 116 RESULT: 115 RESULT: 153 RESULT: 126 RESULT: 127 RESULT: 107 RESULT: 128 RESULT: 121 RESULT: 126 REVERTING 1451 AND APPLYING THIS FIX RESULT: 249 RESULT: 173 RESULT: 150 RESULT: 120 RESULT: 127 RESULT: 124 RESULT: 140 RESULT: 122 RESULT: 106 RESULT: 100 RESULT: 96 RESULT: 104 RESULT: 103 RESULT: 96 RESULT: 92 RESULT: 94 RESULT: 100 RESULT: 91 RESULT: 101 RESULT: 100 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456711246 Suggestion: i would use something more precise (with an higher spectrum of latencies) if possible to make things comparable: https://github.com/franz1981/artemis-load-generator Change the core version you need on the pom and run it with: ```java -jar destination-bench.jar --bytes 100 --protocol artemis --url tcp://localhost:61616 ---out /tmp/latencies.txt --name q --iterations 10 --runs 1 --warmup 10 --wait 2``` It will give you the latencies over an all out throughout: then you need to choose a sustainable throughput eg 10 and re-run the same test adding `--target 10`, then you will have comparable latencies. The ideal would be to use a profiler (async-profiler) to measure the samples taken by awaking the idle threads: the point is, if you have too much threads sitting idle into the thread-pool you'll get a lower utilization. Just sizing the max thread pool with a reasonable value would be enough eg 1/2 of the real core count, given that netty is already stealing the rest (probably more). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456712653 @franz1981 we dont even need, that the % change is quite significant. Also note https://github.com/apache/activemq-artemis/pull/1577/commits/f8b758d14bc4dac7d613e1d15a65d31289f0a587 seems got merged very quickly without much perf critique. This is simply reverting that, and addressing the sync concerns in the original version. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456713968 @franz1981 we have been running investigations in a REAL environment going back an forth between versions on this for some time, even if we revert this class, then latency is improved. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456716247 @franz1981 its not down to sizing the thread pool. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456716626 @franz1981 This is what the old varient tried to avoid, was creating new threads... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456716626 @franz1981 This is what the old varient tried to avoid, was creating new threads... The performance difference is quite noticeable, as per the small bench mark we shared where thats over 1x20 so total of 200,000 (we have run many more during investigating this) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456718199 @franz1981 roughly correct The original one before 1451, tried to re-use an idle thread if possible before creating a new one, albiet using some sync blocks. As the idea is its meant to be semantically like cachedthreadpool, but where there is simply a max bound to that cache, and then tasks should be queued if all threads are not-idle This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456718199 @franz1981 roughly correct The original one before 1451, tried to re-use an idle thread if possible before creating a new one, albiet using some sync blocks. As the idea is its meant to be semantically like cachedthreadpool, but where there is simply a max bound to that cache, and then tasks should be queued This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456719291 correct. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456719095 @michaelandrepearce Ok and the old thread-pool, assuming max thread count already reached, was equally distributing the load across threads? > roughly correct If I've missed anything, please correct me, it helps me to understand ;) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456719095 @michaelandrepearce Ok and the old thread-pool, assuming max thread count already reached, was equally distributing the load across threads? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456719988 Ok, if that's the problem I agree with you that need to be addressed: is not the behaviour of a thread-pool that we expect to have, but it is mostly hitting when the system is got idle and suddenly start to process messages and some of the threads are already died. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456719988 Ok, if that's the problem I agree with you that need to be addressed: is not the behaviour of a thread-pool, that we expect to have, but it is mostly hitting when the system is got idle and suddenly start to process messages This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456721762 Now that you've helped me to understand the issue I believe that's correct: let me take a look to the PR code, because there is another thing I have on my "bag" re this one :) Well done to have found it, as you have said, that PR has stealthy entered into the code-base without having clue about the implications :O This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456721762 Now that you've helped me to understand the issue I believe that's correct: let me take a look to the current impl, because there is another thing I have on my "bag" re this one :) Well done to have found it, as you have said, that PR has stealthy entered into the code-base without having clue about the implications :O This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456722769 So all ive done, is revert that change of 1451 to put back semantics. Streamlined the code a tad, so then we can remove sync blocks, and can just make the updating of threadTaskDelta using an atomic updater rather than doing it inside a sync block. Which then re-addresses concern of 1451 which was sync blocks. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456722769 So all ive done, is revert that change of 1451 to put back semantics. Streamlined the code a tad, so then we can remove sync blocks, and can just make the updating of threadTaskDelta using an atomic updater rather than doing it inside a sync block. Which then re-addresses concern of 1451 which was perf overhead of the sync blocks. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
franz1981 commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456723336 Yep I see that there are a couple of things that could be addressed due to this + a possible improvement: I will propose the improvement separately, but I believe you will like it :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456724549 @franz1981 yup agree there are some improvements we could have done, but have left them out As right now, the main part is dealing with the behaviour/latency regression and still addressing 1451's concerns of sync blocks. As need this also in 2.6.x branch as well. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce edited a comment on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725981 I can un-squash and make it a two stage commit if it helps? e.g. revert commit, then remove sync block commit. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
michaelandrepearce commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456725981 I can make it a two stage commit if it helps? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ldebello commented on issue #2510: [ARTEMIS-2156] Message Duplication when using exclusive divert and clustering
ldebello commented on issue #2510: [ARTEMIS-2156] Message Duplication when using exclusive divert and clustering URL: https://github.com/apache/activemq-artemis/pull/2510#issuecomment-456842951 @clebertsuconic sorry just one question to understand the workflow is common to see the PR close which indicates `branch has unmerged commits.` but in master I see the changes This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2490: ARTEMIS-196 Implement Consumer Priority
michaelandrepearce commented on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-456881694 @clebertsuconic some of those variables are used but accesed by atomic updaters. Have commented on your commit This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit merged pull request #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
asfgit merged pull request #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2510: [ARTEMIS-2156] Message Duplication when using exclusive divert and clustering
asfgit closed pull request #2510: [ARTEMIS-2156] Message Duplication when using exclusive divert and clustering URL: https://github.com/apache/activemq-artemis/pull/2510 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority
clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-456877360 I added one change on top of yours during merge... to remove unused variables.. merged! you are the man! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2451: ARTEMIS-2192 LegacyLDAPSecuritySettingPlugin uses hard-coded RDN types
asfgit closed pull request #2451: ARTEMIS-2192 LegacyLDAPSecuritySettingPlugin uses hard-coded RDN types URL: https://github.com/apache/activemq-artemis/pull/2451 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority
clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-456656799 @michaelandrepearce I'm pretty sure I will be able to merge this tomorrow. Testsuite had some unrelated failures. I just want to double check.. if there's any issues I'm sure i will be able to figure out. will let you know tomorrow. I got the results pretty late after I finished. leave it with me for tomorrow first thing please? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451
clebertsuconic commented on issue #2514: ARTEMIS-2236 Address Latency Impact caused by ARTEMIS-1451 URL: https://github.com/apache/activemq-artemis/pull/2514#issuecomment-456837511 Oh shoot.. I don't know how I missed this.. I'm the one who merged it.. I wouldn't have merged it if I had fully understood the change. I'm merging it now, cherry-picking it into 2.6.x This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2490: ARTEMIS-196 Implement Consumer Priority
asfgit closed pull request #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
michaelandrepearce commented on issue #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods URL: https://github.com/apache/activemq-artemis/pull/2427#issuecomment-456947342 I happy to merge this if you are ready. Have been waiting for your ci build check. But from the checks i did it seems good This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
michaelandrepearce edited a comment on issue #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods URL: https://github.com/apache/activemq-artemis/pull/2427#issuecomment-456947342 @franz1981 I happy to merge this if you are ready. Have been waiting for your ci build check. But from the checks i did it seems good This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2199: ARTEMIS-1996 MappedSequentialFileFactory may cause DirectByteBuffer off-heap memory leaks
michaelandrepearce commented on issue #2199: ARTEMIS-1996 MappedSequentialFileFactory may cause DirectByteBuffer off-heap memory leaks URL: https://github.com/apache/activemq-artemis/pull/2199#issuecomment-456949661 @morefuntang ping. I will close this in a couple days as no response for 3 months and it seems another pr addressed the issue. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
michaelandrepearce edited a comment on issue #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods URL: https://github.com/apache/activemq-artemis/pull/2427#issuecomment-456947342 @franz1981 I happy to merge this if you are ready. Have been waiting for you to report back on the ci build check you wanted todo. But from the checks i did it seems good This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2515: ARTEMIS-1867 fix FQQNOpenWireTest
asfgit closed pull request #2515: ARTEMIS-1867 fix FQQNOpenWireTest URL: https://github.com/apache/activemq-artemis/pull/2515 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram opened a new pull request #2515: ARTEMIS-1867 fix FQQNOpenWireTest
jbertram opened a new pull request #2515: ARTEMIS-1867 fix FQQNOpenWireTest URL: https://github.com/apache/activemq-artemis/pull/2515 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority
clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-456913156 @michaelandrepearce duh.. thanks This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #1293: ARTEMIS-1194: SOCKS proxy support
michaelandrepearce commented on issue #1293: ARTEMIS-1194: SOCKS proxy support URL: https://github.com/apache/activemq-artemis/pull/1293#issuecomment-456950321 @clebertsuconic whats occuring on this, we ok to merge? And simply mark feature beta until some decent tests? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] tabish121 commented on issue #17: AMQCPP-643: Add an option to time out connection attempts when blocked in ensureConnectionInfoSent
tabish121 commented on issue #17: AMQCPP-643: Add an option to time out connection attempts when blocked in ensureConnectionInfoSent URL: https://github.com/apache/activemq-cpp/pull/17#issuecomment-456951888 This is targeted at 3.9.x but would also need to go into master, can you target this at master so it can go into there and then be cherry picked to the patch branches This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #1994: ARTEMIS-1787 Openwire message should not contain internal property
michaelandrepearce commented on issue #1994: ARTEMIS-1787 Openwire message should not contain internal property URL: https://github.com/apache/activemq-artemis/pull/1994#issuecomment-456950879 As noted effort should go to native openwire message support #1793. Can we close this. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2274: ARTEMIS-2059 NettyWritable should use UTF-8 exact length to encode strings
michaelandrepearce commented on issue #2274: ARTEMIS-2059 NettyWritable should use UTF-8 exact length to encode strings URL: https://github.com/apache/activemq-artemis/pull/2274#issuecomment-456951676 @franz1981 Can we close and reopen later once some more progress or discussion is done. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2480: ARTEMIS-2212 Avoid using CLQ on ServerConsumerImpl
clebertsuconic commented on a change in pull request #2480: ARTEMIS-2212 Avoid
using CLQ on ServerConsumerImpl
URL: https://github.com/apache/activemq-artemis/pull/2480#discussion_r248709539
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java
##
@@ -509,21 +518,16 @@ public synchronized void close(final boolean failed)
throws Exception {
removeItself();
- LinkedList refs = cancelRefs(failed, false, null);
-
- Iterator iter = refs.iterator();
+ List refs = cancelRefs(failed, false, null);
Transaction tx = new TransactionImpl(storageManager);
- while (iter.hasNext()) {
- MessageReference ref = iter.next();
-
+ refs.forEach(ref -> {
Review comment:
is this atomic?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2480: ARTEMIS-2212 Avoid using CLQ on ServerConsumerImpl
clebertsuconic commented on a change in pull request #2480: ARTEMIS-2212 Avoid
using CLQ on ServerConsumerImpl
URL: https://github.com/apache/activemq-artemis/pull/2480#discussion_r24870
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java
##
@@ -509,21 +518,16 @@ public synchronized void close(final boolean failed)
throws Exception {
removeItself();
- LinkedList refs = cancelRefs(failed, false, null);
-
- Iterator iter = refs.iterator();
+ List refs = cancelRefs(failed, false, null);
Transaction tx = new TransactionImpl(storageManager);
- while (iter.hasNext()) {
- MessageReference ref = iter.next();
-
+ refs.forEach(ref -> {
Review comment:
Ohhh... ok.. it's on the list from cancelRefs.. I confused as being the main
list.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] asfgit closed pull request #2480: ARTEMIS-2212 Avoid using CLQ on ServerConsumerImpl
asfgit closed pull request #2480: ARTEMIS-2212 Avoid using CLQ on ServerConsumerImpl URL: https://github.com/apache/activemq-artemis/pull/2480 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
franz1981 edited a comment on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl URL: https://github.com/apache/activemq-artemis/pull/2494#issuecomment-454709952 @qihongxu What is worrying me is this > at most time it obtained a 3k~6k tps If the same test (same configuration) is performed using just 1 consumer I'm expecting (best case scenario) that it would have (3k-6k)/200 tps if: 1. the tps are dependent by the most demanding operation in the CPU flamegraph you've posted ie `ConcurrentAppendOnlyList:.get(index)` 2. the `ConcurrentAppendOnlyList:.get(index)` was happening in parallel assuming to have 200 threads available ie bringing a 200 x speedup over a single consumer case I suppose that 2 can't be true, so it really depends on the number of cores/threads available to execute such queries to `LivePageCache` IF such queries are performed in parallel. Do you have any idea of what's happening here? Which assumptions are correct/wrong? Just guessing if this process could be made much better/scalable just answering those questions :+1: This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2498: ARTEMIS-2228 Large Messages over Management
michaelandrepearce commented on issue #2498: ARTEMIS-2228 Large Messages over Management URL: https://github.com/apache/activemq-artemis/pull/2498#issuecomment-454450983 Just fyi, Re your failed pr build since some recent merges to master ive been getting more intermittent build failures more regularly. I dont think its your pr This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2250: ARTEMIS-1996 MappedSequentialFileFactory may cause DirectByteBuffer off-heap memory leaks
franz1981 commented on issue #2250: ARTEMIS-1996 MappedSequentialFileFactory may cause DirectByteBuffer off-heap memory leaks URL: https://github.com/apache/activemq-artemis/pull/2250#issuecomment-454285382 @CNNJYB in this case it is wanted: compaction was leaking buffers big as as the file size, while with paging not. The point is that a thread local pool buffer is not a perfect solution, but is better then no pooling (using heap buffers and FileChannel means that FileChannel will leak a direct ByteBuffer and will perform an hidden copy) or a shared (among threads) pool. We can rethink pooling according to the lifecycle of who perform the page write: any proposal is welcome to solve it :+1 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2250: ARTEMIS-1996 MappedSequentialFileFactory may cause DirectByteBuffer off-heap memory leaks
franz1981 edited a comment on issue #2250: ARTEMIS-1996 MappedSequentialFileFactory may cause DirectByteBuffer off-heap memory leaks URL: https://github.com/apache/activemq-artemis/pull/2250#issuecomment-454285382 @CNNJYB in this case it is wanted: compaction was leaking buffers big as the file size, while with paging not. The point is that a thread local pool buffer is not a perfect solution, but is better then no pooling (using heap buffers and FileChannel means that FileChannel will leak a direct ByteBuffer and will perform an hidden copy) or a shared (among threads) pool. We can rethink pooling according to the lifecycle of who perform the page write: any proposal is welcome to solve it :+1 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2498: ARTEMIS-2228 Large Messages over Management
michaelandrepearce commented on a change in pull request #2498: ARTEMIS-2228
Large Messages over Management
URL: https://github.com/apache/activemq-artemis/pull/2498#discussion_r247781204
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##
@@ -1005,9 +1006,9 @@ public RemotingConnection getRemotingConnection() {
ByteBuffer buffer = ByteBuffer.allocate(8);
buffer.putLong(queue.getID());
message.putBytesProperty(Message.HDR_ROUTE_TO_IDS, buffer.array());
- postOffice.route(message, true);
+
server.getPostOffice().route(LargeServerMessageImpl.checkLargeMessage(message,
server.getStorageManager()), true);
Review comment:
Maybe rather than directly routing to the post office, should we not send
via serversession? This would then sort the large message issue but also ensure
the message is treated like others so in future if other bits should happen we
dont risk having two ways. Noted already that not going via serversession i
see we miss the broker plugin.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
franz1981 commented on a change in pull request #2494: ARTEMIS-2224 Reduce
contention on LivePageCacheImpl
URL: https://github.com/apache/activemq-artemis/pull/2494#discussion_r248691840
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/LivePageCacheImpl.java
##
@@ -16,30 +16,31 @@
*/
package org.apache.activemq.artemis.core.paging.cursor.impl;
-import java.util.LinkedList;
-import java.util.List;
-
import org.apache.activemq.artemis.core.paging.PagedMessage;
import org.apache.activemq.artemis.core.paging.cursor.LivePageCache;
import org.apache.activemq.artemis.core.paging.impl.Page;
import org.apache.activemq.artemis.core.server.LargeServerMessage;
+import
org.apache.activemq.artemis.utils.collections.ConcurrentAppendOnlyChunkedList;
import org.jboss.logging.Logger;
/**
* This is the same as PageCache, however this is for the page that's being
currently written.
*/
-public class LivePageCacheImpl implements LivePageCache {
+public final class LivePageCacheImpl implements LivePageCache {
private static final Logger logger =
Logger.getLogger(LivePageCacheImpl.class);
- private final List messages = new LinkedList<>();
+ private static final int CHUNK_SIZE = 32;
Review comment:
@qihongxu This should do the "trick" to improve things when is expected an
heavy usage of paging with small messages and big pages: if you tweak this in a
separate branch (taken from master too) into `CHUNK_SIZE = 64*1024` it would
save many cache misses and it should speedup in a visible way the paged message
queries.
You can give it a try and if it should work we can think to expose it or
just provide an cursor-like implementation instead of a generic query that
would give more hints about which access pattern is being used eg sequential I
suppose. Many thanks for the tests/efforts and intuitions! :+1:
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] asfgit closed pull request #2500: NO-JIRA Adding test to verify Openwire consuming from FQQN
asfgit closed pull request #2500: NO-JIRA Adding test to verify Openwire consuming from FQQN URL: https://github.com/apache/activemq-artemis/pull/2500 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] qihongxu commented on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
qihongxu commented on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl URL: https://github.com/apache/activemq-artemis/pull/2494#issuecomment-454686710 > 900M of page size and very small messages are not such a common use case TBH Agreed. Use this case just to make it stay at livePageCache longer. In production we use default 10M page size, which means that under high concurrency it's not easy for livePageCache to be read at the same time. In conclusion this patch works fine as described and performance benefits from it:) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
franz1981 commented on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl URL: https://github.com/apache/activemq-artemis/pull/2494#issuecomment-454709952 @qihongxu What is worrying me is this > at most time it obtained a 3k~6k tps If the same test (same configuration) is performed using just 1 consumer I'm expecting (best case scenario) that it would have (3k-6k)/200 tps if: 1. the tps are dependent by the most demanding operation in the CPU flamegraph you've posted ie `ConcurrentAppendOnlyList:.get(index)` 2. the `ConcurrentAppendOnlyList:.get(index)` can now happen in parallel and assuming to have 200 threads available it means a 200 x speedup I suppose that 2 can't be true, so it really depends on the number of cores/threads available to execute such queries to `LivePageCache` IF such queries are performed in parallel. Do you have any idea of what's happening here? Which assumptions are correct/wrong? Just guessing is this whole process could be made much better/scalable just answering those questions :+1: This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 edited a comment on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
franz1981 edited a comment on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl URL: https://github.com/apache/activemq-artemis/pull/2494#issuecomment-454709952 @qihongxu What is worrying me is this > at most time it obtained a 3k~6k tps If the same test (same configuration) is performed using just 1 consumer I'm expecting (best case scenario) that it would have (3k-6k)/200 tps if: 1. the tps are dependent by the most demanding operation in the CPU flamegraph you've posted ie `ConcurrentAppendOnlyList:.get(index)` 2. the `ConcurrentAppendOnlyList:.get(index)` can now happen in parallel and assuming to have 200 threads available it means a 200 x speedup I suppose that 2 can't be true, so it really depends on the number of cores/threads available to execute such queries to `LivePageCache` IF such queries are performed in parallel. Do you have any idea of what's happening here? Which assumptions are correct/wrong? Just guessing if this process could be made much better/scalable just answering those questions :+1: This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
franz1981 commented on issue #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl URL: https://github.com/apache/activemq-artemis/pull/2494#issuecomment-454338688 > @franz1981 thx for recommending this tool, much easier to use than jfr! I was sure you would have like it: is indeed "pragmatically better" for devs IMHO: just remember to enable debug non safepoints, but with JFR is the same, reading their docs! @qihongxu > At the end of test the perf boosted to 30k tps The current implementation (lock-free) of the live page cache suffer of the same performance limitation of a `LinkedList`: queries at the beginning and at the end of the list are faster ie `O(1)`, while in the middle are slower ie `O(n)`. To make things faster we need to exploit a better access pattern and provide a cursor-like implementation on it to be used on `QueueImpl::depage`: it should improve things at the point of having ~`O(1)` `getMessage` cost if the cursor will be used with a sequential access pattern, like an iterator. If you like the idea and it really seems to be important I can provide an implementaion for it ie 900M of page size and very small messages are not such a common use case TBH This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2479: ARTEMIS-2211 Avoid duplicate code for ByteBuffer pooling and alignment
clebertsuconic commented on a change in pull request #2479: ARTEMIS-2211 Avoid
duplicate code for ByteBuffer pooling and alignment
URL: https://github.com/apache/activemq-artemis/pull/2479#discussion_r248716417
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/BitUtil.java
##
@@ -14,19 +14,23 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.activemq.artemis.core.io.mapped;
+package org.apache.activemq.artemis.utils;
-import java.nio.ByteBuffer;
-
-import io.netty.util.internal.PlatformDependent;
-
-final class BytesUtils {
+/**
+ * Collection of bit-tricks
+ */
+public final class BitUtil {
Review comment:
We already have another class ByteUtil on the same package.
it will be confusing to have on ByteUtil and another BitsUtil.
Can you just move the methods to the same class with some documentation on
these methods?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] asfgit closed pull request #2478: ARTEMIS-2210 Fix PagingStore creation synchronization issue
asfgit closed pull request #2478: ARTEMIS-2210 Fix PagingStore creation synchronization issue URL: https://github.com/apache/activemq-artemis/pull/2478 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2474: [ARTEMIS-1536]: Incorrect Journal filesize calculation where specified size is lest that the block size when using AIO.
asfgit closed pull request #2474: [ARTEMIS-1536]: Incorrect Journal filesize calculation where specified size is lest that the block size when using AIO. URL: https://github.com/apache/activemq-artemis/pull/2474 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2474: [ARTEMIS-1536]: Incorrect Journal filesize calculation where specified size is lest that the block size when using AIO.
clebertsuconic commented on issue #2474: [ARTEMIS-1536]: Incorrect Journal filesize calculation where specified size is lest that the block size when using AIO. URL: https://github.com/apache/activemq-artemis/pull/2474#issuecomment-455216239 merged & cherry-picked into 2.6.x This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2473: ARTEMIS-1058 Jars in web tmp dir locked on Windows
clebertsuconic commented on a change in pull request #2473: ARTEMIS-1058 Jars
in web tmp dir locked on Windows
URL: https://github.com/apache/activemq-artemis/pull/2473#discussion_r248722829
##
File path:
artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
##
@@ -237,28 +236,39 @@ public void start() throws Exception {
public void internalStop() throws Exception {
server.stop();
if (webContexts != null) {
+
File tmpdir = null;
+ StringBuilder strBuilder = new StringBuilder();
+ boolean found = false;
for (WebAppContext context : webContexts) {
tmpdir = context.getTempDirectory();
-if (tmpdir != null && !context.isPersistTempDirectory()) {
+if (tmpdir != null && tmpdir.exists() &&
!context.isPersistTempDirectory()) {
//tmpdir will be removed by deleteOnExit()
- //somehow when broker is stopped and restarted quickly
- //this tmpdir won't get deleted sometimes
- boolean fileDeleted = TimeUtils.waitOnBoolean(false, 5000,
tmpdir::exists);
-
- if (!fileDeleted) {
- //because the execution order of shutdown hooks are
- //not determined, so it's possible that the deleteOnExit
- //is executed after this hook, in that case we force a
delete.
- FileUtil.deleteDirectory(tmpdir);
- logger.debug("Force to delete temporary file on shutdown: "
+ tmpdir.getAbsolutePath());
- if (tmpdir.exists()) {
- ActiveMQWebLogger.LOGGER.tmpFileNotDeleted(tmpdir);
- }
+ //However because the URLClassLoader never release/close its
opened
+ //jars the jar file won't be able to get deleted on Windows
platform
+ //until after the process fully terminated. To fix this here
arranges
+ //a separate process to try clean up the temp dir
+ FileUtil.deleteDirectory(tmpdir);
+ if (tmpdir.exists()) {
+ ActiveMQWebLogger.LOGGER.tmpFileNotDeleted(tmpdir);
+ strBuilder.append(tmpdir);
+ strBuilder.append(",");
+ found = true;
}
}
}
+
+ if (found) {
+String artemisHome = System.getProperty("artemis.home");
Review comment:
you're wrong actually,
WebServerComponent is getting the artemisInstance and artemisHome as
parameters.
It is important you don't use the variable here as you may break embedded
cases.
We use things like this on our testsuite as well. so, you can use a property
to store what's passed from the configure method, and use the variable directly
here instead.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] clebertsuconic commented on issue #2466: ARTEMIS-2206 The MQTT consumer reconnection caused the queue to not be cle…
clebertsuconic commented on issue #2466: ARTEMIS-2206 The MQTT consumer reconnection caused the queue to not be cle… URL: https://github.com/apache/activemq-artemis/pull/2466#issuecomment-455219150 @jbertram / @onlyMIT what's the status here? I was looking on PRs to merge but this one seems to be failing a test? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram commented on issue #2466: ARTEMIS-2206 The MQTT consumer reconnection caused the queue to not be cle…
jbertram commented on issue #2466: ARTEMIS-2206 The MQTT consumer reconnection caused the queue to not be cle… URL: https://github.com/apache/activemq-artemis/pull/2466#issuecomment-455219703 @clebertsuconic, this is still in progress. I haven't had any time to follow-up yet. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2459: ARTEMIS-2200 NPE while dropping/failing large messages on paging
clebertsuconic commented on a change in pull request #2459: ARTEMIS-2200 NPE
while dropping/failing large messages on paging
URL: https://github.com/apache/activemq-artemis/pull/2459#discussion_r248724979
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##
@@ -974,6 +974,7 @@ public void run() {
} catch (Throwable e) {
result.fail(e);
logger.error("appendDeleteRecord:" + e, e);
+ setErrorCondition(callback, null, e);
Review comment:
I understand you fixed a synchronization here, but I'm a bit concerned into
forcing a failure on the callback when non IO Errors have happened.
Do you really need this line?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2459: ARTEMIS-2200 NPE while dropping/failing large messages on paging
franz1981 commented on a change in pull request #2459: ARTEMIS-2200 NPE while
dropping/failing large messages on paging
URL: https://github.com/apache/activemq-artemis/pull/2459#discussion_r248726229
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##
@@ -974,6 +974,7 @@ public void run() {
} catch (Throwable e) {
result.fail(e);
logger.error("appendDeleteRecord:" + e, e);
+ setErrorCondition(callback, null, e);
Review comment:
It is handled like this on the other append calls, let me find one...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2459: ARTEMIS-2200 NPE while dropping/failing large messages on paging
franz1981 commented on a change in pull request #2459: ARTEMIS-2200 NPE while
dropping/failing large messages on paging
URL: https://github.com/apache/activemq-artemis/pull/2459#discussion_r248726803
##
File path:
artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java
##
@@ -974,6 +974,7 @@ public void run() {
} catch (Throwable e) {
result.fail(e);
logger.error("appendDeleteRecord:" + e, e);
+ setErrorCondition(callback, null, e);
Review comment:
https://github.com/apache/activemq-artemis/blob/47e17ee754044ec1b2bcf0f1712f71a994ed0009/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java#L860
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2459: ARTEMIS-2200 NPE while dropping/failing large messages on paging
clebertsuconic commented on a change in pull request #2459: ARTEMIS-2200 NPE
while dropping/failing large messages on paging
URL: https://github.com/apache/activemq-artemis/pull/2459#discussion_r248726923
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##
@@ -1354,13 +1354,15 @@ public void done() {
*/
private void confirmLargeMessageSend(Transaction tx, final Message message)
throws Exception {
LargeServerMessage largeServerMessage = (LargeServerMessage) message;
- if (largeServerMessage.getPendingRecordID() >= 0) {
Review comment:
this shouldn't be necessary. The only caller is synchronizing
largeServerMessage.
I would look for the real cause here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2438: ARTEMIS-2178 routing-type config for core bridge
michaelandrepearce edited a comment on issue #2438: ARTEMIS-2178 routing-type config for core bridge URL: https://github.com/apache/activemq-artemis/pull/2438#issuecomment-455354071 @jbertram thanks for this, like the feature!!, sorry been so long to merge it for you. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2438: ARTEMIS-2178 routing-type config for core bridge
asfgit closed pull request #2438: ARTEMIS-2178 routing-type config for core bridge URL: https://github.com/apache/activemq-artemis/pull/2438 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2438: ARTEMIS-2178 routing-type config for core bridge
michaelandrepearce commented on issue #2438: ARTEMIS-2178 routing-type config for core bridge URL: https://github.com/apache/activemq-artemis/pull/2438#issuecomment-455354071 @jbertram thanks for this, sorry been so long to merge it for you. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2470: Fixes for alerts from lgtm.com
asfgit closed pull request #2470: Fixes for alerts from lgtm.com URL: https://github.com/apache/activemq-artemis/pull/2470 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2451: ARTEMIS-2192 LegacyLDAPSecuritySettingPlugin uses hard-coded RDN types
clebertsuconic commented on a change in pull request #2451: ARTEMIS-2192
LegacyLDAPSecuritySettingPlugin uses hard-coded RDN types
URL: https://github.com/apache/activemq-artemis/pull/2451#discussion_r248871028
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java
##
@@ -330,25 +330,29 @@ private void processSearchResult(Map>
securityRoles,
String permissionType = null;
String destination = null;
String destinationType = "unknown";
- for (Rdn rdn : searchResultLdapName.getRdns()) {
- if (rdn.getType().equals("cn")) {
-logger.debug("\tPermission type: " + rdn.getValue());
-permissionType = rdn.getValue().toString();
- }
- if (rdn.getType().equals("uid")) {
-logger.debug("\tDestination name: " + rdn.getValue());
-destination = rdn.getValue().toString();
- }
- if (rdn.getType().equals("ou")) {
-String rawDestinationType = rdn.getValue().toString();
-if (rawDestinationType.toLowerCase().contains("queue")) {
- destinationType = "queue";
-} else if (rawDestinationType.toLowerCase().contains("topic")) {
- destinationType = "topic";
-}
-logger.debug("\tDestination type: " + destinationType);
- }
+ List rdns = searchResultLdapName.getRdns();
+ if (rdns.size() != 3) {
+ logger.debug("\tSkipping unexpected search result with " +
rdns.size() + " RDNs.");
+ return;
}
+ // we can count on the RNDs being in order from right to left
+ Rdn rdn = rdns.get(0);
+ String rawDestinationType = rdn.getValue().toString();
+ if (rawDestinationType.toLowerCase().contains("queue")) {
+ destinationType = "queue";
+ } else if (rawDestinationType.toLowerCase().contains("topic")) {
+ destinationType = "topic";
+ }
+ logger.debug("\tDestination type: " + destinationType);
+
+ rdn = rdns.get(1);
+ logger.debug("\tDestination name: " + rdn.getValue());
+ destination = rdn.getValue().toString();
+
+ rdn = rdns.get(2);
+ logger.debug("\tPermission type: " + rdn.getValue());
Review comment:
nit pik.. how often is this called? this should be using if
(loger.isDebugEnabled()) before the call on logger.debug since it has
concatenations in place here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] clebertsuconic commented on issue #2434: ARTEMIS-1867 FQQN for producers
clebertsuconic commented on issue #2434: ARTEMIS-1867 FQQN for producers URL: https://github.com/apache/activemq-artemis/pull/2434#issuecomment-455363730 this is conflicting. it will conflict even worse after we merge Consumers priority. Can we work on this after Consumers Priority since I'm already running tests on it? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2434: ARTEMIS-1867 FQQN for producers
michaelandrepearce edited a comment on issue #2434: ARTEMIS-1867 FQQN for producers URL: https://github.com/apache/activemq-artemis/pull/2434#issuecomment-455366517 @clebertsuconic ill hold on merging this, and the work on this with @jbertram after This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2445: ARTEMIS-2187 remove page from softcache before consumedpage
clebertsuconic commented on issue #2445: ARTEMIS-2187 remove page from softcache before consumedpage URL: https://github.com/apache/activemq-artemis/pull/2445#issuecomment-455366535 on this case I think it will be enough if the tests are passing.. will merge it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2434: ARTEMIS-1867 FQQN for producers
michaelandrepearce commented on issue #2434: ARTEMIS-1867 FQQN for producers URL: https://github.com/apache/activemq-artemis/pull/2434#issuecomment-455366517 @clebertsuconic ill hold. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2445: ARTEMIS-2187 remove page from softcache before consumedpage
michaelandrepearce commented on issue #2445: ARTEMIS-2187 remove page from softcache before consumedpage URL: https://github.com/apache/activemq-artemis/pull/2445#issuecomment-455370802 @clebertsuconic no worries, this is your area of expertise so will leave for your better judgement. If youre happy im happy :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2444: ARTEMIS-2186 Large message incomplete when server is crashed
michaelandrepearce commented on issue #2444: ARTEMIS-2186 Large message incomplete when server is crashed URL: https://github.com/apache/activemq-artemis/pull/2444#issuecomment-455373882 @clebertsuconic you happy with this one? I am. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit merged pull request #2501: ARTEMIS-2221 Fix Merge
asfgit merged pull request #2501: ARTEMIS-2221 Fix Merge URL: https://github.com/apache/activemq-artemis/pull/2501 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2427: ARTEMIS-2170 Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
franz1981 commented on a change in pull request #2427: ARTEMIS-2170 Optimized
CoreMessage's checkProperties and cleanupInternalProperties methods
URL: https://github.com/apache/activemq-artemis/pull/2427#discussion_r248889763
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java
##
@@ -318,6 +313,30 @@ public synchronized boolean containsProperty(final
SimpleString key) {
}
}
+ public synchronized boolean removeProperty(Predicate
propertyNamePredicate) {
Review comment:
Yep, then original version was using a slower method to remove a property
matching specific criteria and it was used in the hot path for every message.
This change has improved (for small binary message) the throughput of core
on my tests by a perceivable quantity.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] gaohoward opened a new pull request #2506: ARTEMIS-2230 Exception closing advisory consumers when supportAdvisory=false
gaohoward opened a new pull request #2506: ARTEMIS-2230 Exception closing advisory consumers when supportAdvisory=false URL: https://github.com/apache/activemq-artemis/pull/2506 When broker's advisory is disabled (supportAdvisory=false) any advisory consumer won't get created at broker and the advisory consumer ID won't be stored. Legacy openwire clients can have a reference of advisory consumer regardless broker's settings and therefore when it closes the advisory consumer the broker has no reference to it. Therefore broker throws an exception like: javax.jms.IllegalStateException: Cannot remove a consumer that had not been registered If the broker stores the consumer info (even it doesn't create it) the exception can be avoided. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] gaohoward commented on issue #2386: ARTEMIS-2135 Test multiple core consumers receiving amqp messages
gaohoward commented on issue #2386: ARTEMIS-2135 Test multiple core consumers receiving amqp messages URL: https://github.com/apache/activemq-artemis/pull/2386#issuecomment-455419691 closing this as it already merged. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] gaohoward closed pull request #2386: ARTEMIS-2135 Test multiple core consumers receiving amqp messages
gaohoward closed pull request #2386: ARTEMIS-2135 Test multiple core consumers receiving amqp messages URL: https://github.com/apache/activemq-artemis/pull/2386 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce opened a new pull request #2504: NO-JIRA Check data != null during encode
michaelandrepearce opened a new pull request #2504: NO-JIRA Check data != null during encode URL: https://github.com/apache/activemq-artemis/pull/2504 Picked up by code analysis checks This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority
clebertsuconic commented on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-455388517 Current list of failures: Test Name | Duration | Age -- | -- | -- org.apache.activemq.artemis.tests.integration.client.AutogroupIdTest.testGroupIdAutomaticallySetMultipleProducers | 37 ms | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testMultipleGroupingXACommit | 47 ms | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testLoadBalanceGroups | 21 ms | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testMultipleGroupingTXCommit | 0.25 sec | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testMultipleGroupingConsumeHalf | 0.79 sec | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testMultipleGroupingXARollback | 55 ms | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testMultipleGrouping | 14 ms | 1 org.apache.activemq.artemis.tests.integration.client.MessageGroupingTest.testMultipleGroupingTXRollback | 0.28 sec | 1 org.apache.activemq.artemis.tests.integration.cluster.failover.ReplicatedMultipleServerFailoverExtraBackupsTest.testStartLiveFirst | 1 min 8 sec | 1 org.apache.activemq.artemis.tests.integration.jms.client.GroupingTest.testManyGroups | 6 sec | 1 org.apache.activemq.artemis.tests.integration.jms.client.GroupingTest.testManyGroups | 6 sec | 1 org.apache.activemq.artemis.tests.integration.jmx.JmxConnectionTest.testJmxConnection | 1.1 sec | 1 org.apache.activemq.artemis.tests.integration.journal.AIOJournalImplTest.testDoubleDelete | 53 ms | 1 org.apache.activemq.artemis.tests.integration.journal.AIOUnbuferedJournalImplTest.testDoubleDelete | 39 ms | 1 org.apache.activemq.artemis.tests.integration.journal.MappedJournalImplTest.testDoubleDelete | 39 ms | 1 org.apache.activemq.artemis.tests.integration.journal.MappedUnbuferedJournalImplTest.testDoubleDelete | 43 ms | 1 org.apache.activemq.artemis.tests.integration.journal.NIOJournalImplTest.testDoubleDelete | 36 ms | 1 org.apache.activemq.artemis.tests.integration.journal.NIONoBufferJournalImplTest.testDoubleDelete | 18 ms | 1 org.apache.activemq.artemis.tests.integration.mqtt.MqttAcknowledgementTest.testAcknowledgementQOS1 | 5.6 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTFQQNTest.testSendAndReceiveMQTTSpecial1 | 11 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTOpenwireTest.testWildcards | 3.2 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTSecurityCRLTest.crlNotRevokedTest | 5.2 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testSendAndReceiveAtLeastOnce | 5.3 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testManagementQueueMessagesAreAckd | 5.2 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testMQTTRetainQoS | 7.3 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testSubscribeMultipleTopics | 5 min 0 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testSendAndReceiveExactlyOnceWithInterceptors | 6 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testSendAtLeastOnceReceiveAtMostOnce | 5.4 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testNoMessageReceivedAfterUnsubscribeMQTT | 5.4 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testMQTTPathPatterns | 5.3 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testUniqueMessageIds | 5.3 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testSendAndReceiveRetainedMessages | 5.4 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testRetainedMessage | 5.9 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testResendMessageId | 5.3 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testClientConnectionFailure | 11 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testSendAtMostOnceReceiveExactlyOnce | 5.5 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testAnycastAddressWorksWithMQTT | 3.5 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testAnycastPrefixWorksWithMQTT | 2.4 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testReceiveMessageSentWhileOffline | 5.4 sec | 1 org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.testAmbiguousRoutingWithMQTT | 3.2 sec | 1
[GitHub] michaelandrepearce commented on issue #2490: ARTEMIS-196 Implement Consumer Priority
michaelandrepearce commented on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-455391365 @clebertsuconic can you recheck this against master, i know things like testDoubleDelete are failing currently on master without this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2490: ARTEMIS-196 Implement Consumer Priority
michaelandrepearce edited a comment on issue #2490: ARTEMIS-196 Implement Consumer Priority URL: https://github.com/apache/activemq-artemis/pull/2490#issuecomment-455394066 @clebertsuconic the message group test ones are are genuine, should be fixed up in last push. I dont believe StompWebSocketMaxFrameTest is related, when i run this its erroring because io.netty.handler.codec.CorruptedFrameException: Max frame length of 65536 has been exceeded. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
