[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic can you take a look at the additional commit ?


---


[GitHub] activemq-artemis pull request #1803: ARTEMIS-1628 Limit pool size on artemis...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1803


---


[GitHub] activemq-artemis pull request #1800: ARTEMIS-1626 Disable thread leak check ...

2018-01-22 Thread gaohoward
GitHub user gaohoward reopened a pull request:

https://github.com/apache/activemq-artemis/pull/1800

ARTEMIS-1626 Disable thread leak check for failing tests

The ThreadLeakCheckRule is used to check thread leaks
after each test is finished. However when a test fails, it is
not necessary to check leaking threads because the test
failure should be fixed anyway. And leaking threads in a
failed test may well be a result of the failure (once the test
is fixed the thread leak may be gone).

If a failed test also leaks threads, it takes a long time before
the thread leak check finishes (60 seconds checking time),
thus it takes a long time to finish, especially when tests are
run in batches with failures.

So to improve this, it should be reasonable to just enable
the thread leaking check for each test passes, and disable
the check when a test fails.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/gaohoward/activemq-artemis fthread_rule

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1800.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1800


commit c8e25c72b9885bb08ec9dd5d8502c273b349d46a
Author: Howard Gao 
Date:   2018-01-22T14:39:23Z

ARTEMIS-1626 Disable thread leak check for failing tests

The ThreadLeakCheckRule is used to check thread leaks
after each test is finished. However when a test fails, it is
not necessary to check leaking threads because the test
failure should be fixed anyway. And leaking threads in a
failed test may well be a result of the failure (once the test
is fixed the thread leak may be gone).

If a failed test also leaks threads, it takes a long time before
the thread leak check finishes (60 seconds checking time),
thus it takes a long time to finish, especially when tests are
run in batches with failures.

So to improve this, it should be reasonable to just enable
the thread leaking check for each test passes, and disable
the check when a test fails.




---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
reopen for further discussion.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic 
regarding the thread leak logging, I don't think I got it right. Do you 
mean if test failed, we give the thread logging, what about test that pases? 
(just keep in mind if test failed we sure will check and log without this PR)


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
Right now it fails the test. If you changed it to log only if the test 
failed.  You got what you wanted. And what we wanted.  It’s a good 
compromise. 



---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
I think logging would eventually turn on the thread checking, that's the 
original behavior, no need to change anything, right?


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
Please don’t close those.  Just change it to log. 


---


[GitHub] activemq-artemis issue #1803: ARTEMIS-1628 Limit pool size on artemis journa...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1803
  
I can merge this.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
All we asked was logging instead. 


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
Why close it instead of logging it ?


---


[GitHub] activemq-artemis pull request #1800: ARTEMIS-1626 Disable thread leak check ...

2018-01-22 Thread gaohoward
Github user gaohoward closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1800


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
ok, I think it's better to take cautions as people worries about it.
Thanks you guys for the opinions !


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@clebertsuconic Np I've understood what you mean :) 
I've pushed another commit with another solution less "reactive" but more 
simlar to the original code: let me know if it seems better and I will squash 
the commits into this last one :+1: 


---


[GitHub] activemq-artemis pull request #1799: ARTEMIS-1625 fix moving messages

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1799


---


[GitHub] activemq-artemis pull request #1786: ARTEMIS-1616 OpenWire improvements

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1786


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@franz1981 typo on my last message.. I actually meant the autoRead stuff.


---


[GitHub] activemq-artemis issue #1793: ARTEMIS-1498: Openwire internal headers should...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1793
  
I ran the whole testsuite and it didn't complete.


it's a very nice start though.


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@franz1981 I told you it was ok to do it this way.. but I'm a bit concerned 
by the use of setResponse(true) and false through this.

is there a way to return the packet like it used to be done before? can you 
check the setRespond(true) and false usages? I'm concerned of a possible race 
making the server unavailable eventually.


---


[GitHub] activemq-artemis pull request #1805: ARTEMIS-609 fix interceptor XML docs

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1805


---


[GitHub] activemq-artemis pull request #1802: ARTEMIS-1504 Update Qpid JMS to 0.29.0 ...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1802


---


[GitHub] activemq-artemis pull request #1804: ARTEMIS-608 document adding runtime dep...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1804


---


[GitHub] activemq-artemis pull request #1803: ARTEMIS-1628 Limit pool size on artemis...

2018-01-22 Thread clebertsuconic
GitHub user clebertsuconic reopened a pull request:

https://github.com/apache/activemq-artemis/pull/1803

ARTEMIS-1628 Limit pool size on artemis journal



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/clebertsuconic/activemq-artemis journal

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1803.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1803


commit 12746dfd4e6752fd3fef02b0f2c18b17bdffe8d5
Author: Clebert Suconic 
Date:   2018-01-22T17:33:14Z

ARTEMIS-1628 Limit pool size on artemis journal




---


[GitHub] activemq-artemis pull request #1805: ARTEMIS-609 fix interceptor XML docs

2018-01-22 Thread jbertram
GitHub user jbertram opened a pull request:

https://github.com/apache/activemq-artemis/pull/1805

ARTEMIS-609 fix interceptor XML docs



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-609

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1805.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1805


commit e3561fde13d0cb05fb5c58fe7cca7843defa11de
Author: Justin Bertram 
Date:   2018-01-22T21:01:04Z

ARTEMIS-609 fix interceptor XML docs




---


[GitHub] activemq-artemis pull request #1801: ARTEMIS-1607 OpenWire is sending respon...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1801#discussion_r163065589
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
 ---
@@ -393,26 +395,47 @@ public void send(final ProducerInfo producerInfo,
  
this.connection.getContext().setDontSendReponse(false);
  connection.sendException(exceptionToSend);
   } else {
- if (sendProducerAck) {
-try {
-   ProducerAck ack = new 
ProducerAck(producerInfo.getProducerId(), messageSend.getSize());
-   connection.dispatchAsync(ack);
-} catch (Exception e) {
-   
this.connection.getContext().setDontSendReponse(false);
-   
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
-   connection.sendException(e);
+ 
server.getStorageManager().afterCompleteOperations(new IOCallback() {
+@Override
+public void done() {
+   if (sendProducerAck) {
+  try {
+ ProducerAck ack = new 
ProducerAck(producerInfo.getProducerId(), messageSend.getSize());
+ connection.dispatchAsync(ack);
+  } catch (Exception e) {
+ 
connection.getContext().setDontSendReponse(false);
+ 
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
+ connection.sendException(e);
+  }
+   } else {
+  
connection.getContext().setDontSendReponse(false);
+  try {
+ Response response = new Response();
+ 
response.setCorrelationId(messageSend.getCommandId());
+ connection.dispatchAsync(response);
+  } catch (Exception e) {
+ 
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
+ connection.sendException(e);
+  }
+   }
 }
- } else {
-connection.getContext().setDontSendReponse(false);
-try {
-   Response response = new Response();
-   
response.setCorrelationId(messageSend.getCommandId());
-   connection.dispatchAsync(response);
-} catch (Exception e) {
-   
ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
-   connection.sendException(e);
+
+@Override
+public void onError(int errorCode, String 
errorMessage) {
+   //failing here is severe and IO related
+   final Throwable criticalError = new 
IOException(errorMessage);
+   try {
+  //it is handled async and hopefully will be 
sent before the critical error shutdown the broker:
+  //it helps to fail fast the clients on 
critical errors
+  connection.serviceException(criticalError);
+   } catch (Exception e) {
+  ActiveMQServerLogger.LOGGER.debug(e);
+   } finally {
+  //it needs to be called ASAP: the broker 
isn't in a safe state
+  
server.getStorageManager().criticalError(criticalError);
--- End diff --

this is probably also called earlier by the IO layer.. but it doesn't hurt 
here.


---


[GitHub] activemq-artemis pull request #1804: ARTEMIS-608 document adding runtime dep...

2018-01-22 Thread jbertram
GitHub user jbertram opened a pull request:

https://github.com/apache/activemq-artemis/pull/1804

ARTEMIS-608 document adding runtime deps



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-608

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1804.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1804


commit b8239d4401e7a614067debd4b6c4d3d6b38abbe2
Author: Justin Bertram 
Date:   2018-01-22T20:47:53Z

ARTEMIS-608 document adding runtime deps




---


[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...

2018-01-22 Thread mtaylor
Github user mtaylor commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1783#discussion_r163036074
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/transformer/ServerMessageImpl.java
 ---
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.core.server.transformer;
+
+import org.apache.activemq.artemis.api.core.ICoreMessage;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.message.impl.CoreMessage;
+import org.apache.activemq.artemis.core.message.impl.MessageInternalImpl;
+import org.apache.activemq.artemis.core.paging.PagingStore;
+import org.apache.activemq.artemis.core.server.MessageReference;
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.server.ServerMessage;
+
+@Deprecated
--- End diff --

Can do.


---


[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...

2018-01-22 Thread mtaylor
Github user mtaylor commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1783#discussion_r163035962
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TypedProperties.java
 ---
@@ -23,4 +23,7 @@
 @Deprecated
 public class TypedProperties extends 
org.apache.activemq.artemis.utils.collections.TypedProperties {
 
+   public TypedProperties(final 
org.apache.activemq.artemis.utils.collections.TypedProperties other) {
--- End diff --

Good shout @michaelandrepearce.  I missed that.  Will update accordingly.


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
The errors on the completion are critical.  They are IOerrors.  I think the 
critical exception will be called anyways. 


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
@gtully Good point! I've added an async send of error that finally will end 
with the critical error as before: I hope (@clebertsuconic could help confirm 
that) that it is safe enough.
There could be the chance that the exception won't be never send back to 
the clients because the critical error handling will shutdown the server before 
the send will happen!


---


[GitHub] activemq-artemis pull request #1803: ARTEMIS-1628 Limit pool size on artemis...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1803


---


[GitHub] activemq-artemis issue #1803: ARTEMIS-1628 Limit pool size on artemis journa...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1803
  
Please do not merge this.. i will add a log.warn if -1


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward I undestand your points you've showed on my questions, but as 
@clebertsuconic has pointed 

> the ideal world is far from reality.. there are tests that will be 
failing forever on the testsuite and people will ignore them

I agree with that and I think we should try to keep more isolated than 
possible (if possible) tests (failing or not) just for this reason: just 
logging the leaks would be enough for me. 

While dealing with very long running tests who will read the test reports 
need to rely that 2 different runs are comparable somehow and hiding this 
information won't help doing it.
I understand the need to make the process more agile and to aim "0 tests 
failures" and I think that just logging would be great.
I hope to have explained better what I'm worried about 



---


[GitHub] activemq-artemis pull request #1786: ARTEMIS-1616 OpenWire improvements

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r163012542
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

@franz1981 just thinking on this is it worth making the other array list 
also 3? Just so both is consistent, maybe make it a constant, so if in future 
it changes again, single change would affect both.


---


[GitHub] activemq-artemis pull request #1803: ARTEMIS-1628 Limit pool size on artemis...

2018-01-22 Thread clebertsuconic
GitHub user clebertsuconic opened a pull request:

https://github.com/apache/activemq-artemis/pull/1803

ARTEMIS-1628 Limit pool size on artemis journal



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/clebertsuconic/activemq-artemis journal

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1803.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1803


commit 12746dfd4e6752fd3fef02b0f2c18b17bdffe8d5
Author: Clebert Suconic 
Date:   2018-01-22T17:33:14Z

ARTEMIS-1628 Limit pool size on artemis journal




---


[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1783#discussion_r163011302
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TypedProperties.java
 ---
@@ -23,4 +23,7 @@
 @Deprecated
 public class TypedProperties extends 
org.apache.activemq.artemis.utils.collections.TypedProperties {
 
+   public TypedProperties(final 
org.apache.activemq.artemis.utils.collections.TypedProperties other) {
--- End diff --

Should add the default empty constructor. By adding this without explicit 
defining the empty it is being removed.


---


[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1783#discussion_r163010905
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TypedProperties.java
 ---
@@ -23,4 +23,7 @@
 @Deprecated
 public class TypedProperties extends 
org.apache.activemq.artemis.utils.collections.TypedProperties {
 
+   public TypedProperties(final 
org.apache.activemq.artemis.utils.collections.TypedProperties other) {
--- End diff --

What about keeping the default empty constructor? 


---


[GitHub] activemq-artemis pull request #1791: ARTEMIS-1622 Reduce memory footprint of...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1791


---


[GitHub] activemq-artemis issue #1786: ARTEMIS-1616 OpenWire improvements

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1786
  
@franz1981 nudge, only thing holding me from merging this is the number of 
commits.
I guess really should be a @clebertsuconic nudge if he's asked for this on 
purpose.


---


[GitHub] activemq-artemis pull request #1792: ARTEMIS-1621 Make producerWindowSize co...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1792


---


[GitHub] activemq-artemis issue #1793: ARTEMIS-1498: Openwire internal headers should...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1793
  
Really good work, I like this, really good to see OpenWire getting this :) 

Have you run the full OpenWire test suite? I know not all run in the PR 
build, so worth checking. Might be worth running the full suite even if you can.




---


[GitHub] activemq-artemis pull request #1798: ARTEMIS-1624 Adding in condition for tu...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1798


---


[GitHub] activemq-artemis pull request #1793: ARTEMIS-1498: Openwire internal headers...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1793#discussion_r163007469
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenwireConverter.java
 ---
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.core.protocol.openwire;
+
+import org.apache.activemq.artemis.api.core.ICoreMessage;
+import 
org.apache.activemq.artemis.core.message.impl.CoreMessageObjectPools;
+import org.apache.activemq.artemis.spi.core.protocol.MessageConverter;
+
+public class OpenwireConverter implements 
MessageConverter {
--- End diff --

nit: But so it fits with other class naming where OpenWire seems to be 
itself camel cased elsewhere, could this be OpenWireConverter


---


[GitHub] activemq-artemis issue #1796: ARTEMIS-1623 ActiveMQServerPlugin impl for log...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1796
  
Could the logging events, have LOG codes, e.g. like ActiveMQServerLogger


---


[GitHub] activemq-artemis pull request #1795: [ARTEMIS-1609] Add distinct name/addres...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1795


---


[GitHub] activemq-artemis issue #1795: [ARTEMIS-1609] Add distinct name/address for J...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1795
  
Thanks.


---


[GitHub] activemq-artemis pull request #1802: ARTEMIS-1504 Update Qpid JMS to 0.29.0 ...

2018-01-22 Thread tabish121
GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1802

ARTEMIS-1504 Update Qpid JMS to 0.29.0 and proton-j to 0.25.0

Updates to latest Qpid JMS and the latest Proton-J release

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1504

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1802.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1802


commit e626b01243980a8194276760abbcf324880ddf8a
Author: Timothy Bish 
Date:   2018-01-22T17:09:07Z

ARTEMIS-1504 Update Qpid JMS to 0.29.0 and proton-j to 0.25.0

Updates to latest Qpid JMS and the latest Proton-J release




---


[GitHub] activemq-artemis issue #1796: ARTEMIS-1623 ActiveMQServerPlugin impl for log...

2018-01-22 Thread jbertram
Github user jbertram commented on the issue:

https://github.com/apache/activemq-artemis/pull/1796
  
Couple of things:

- Nice work!
- It's not clear to me why some things are logged at DEBUG vs. INFO.  
Perhaps the documentation could address this.
- Since you're using the "f" logging method variants it's really not 
necessary to do the isXEnabled() check before logging since those methods do 
that check already.
- In the places where you're logging either DEBUG or INFO and the format is 
the same you can use `org.jboss.logging.Logger#logf` and just pass in the level 
you want to use so you don't have duplicated code.


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread gtully
Github user gtully commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
think any error from the io completion callback should result in 
connection.sendException(...);


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward the idea world is far from reality.. there are tests that will 
be failing forever on the testsuite and people will ignore them.. a result will 
be a mess that people won't know where the leaks are coming from again.

The ThreadLeak Rule has prevented leaked threads to fail non related tests..

and the threadleak rule is not only logging, but it's also trying to 
interrupt these leaked threads... 


Lets not regress on that please!


We can do a faster check (not wait for a whole minute on failted tests.. 
and we would only logg. instead of fail).. it's a simple change.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic  re: "test1 failed.. leaked.. than test2 failed because 
there's a thread leak... if you don't report the leak from 1.. you won't know 
that test2 could fail later.."
In that case test1 should be looked at first. after test1 is corrected the 
thread check rule kicks in to make sure it doesn't leak, therefore won't 
pollute test2.
The harder case is test1 passed..leaked.. then test2 failed becasue of the 
leak. Even worse case is that test1 passed leaked and tests runs on until 
test(n) failed due to the test1's leak. This PR doesn't allow that happen 
because it checks every passing test for thread leaks.



---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward as I said.. I have been the one to fix these freaking issues 
when the testsuite is acting up... so... I would be okay with your changes if 
you at least logged the leakages without hiding the previous reason why the 
test failed.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
> @clebertsuconic I'm not convinced that it is that important to log a 
leakage on a failed test. If a test
> fails it should be fixed. With this PR you won't miss a real leakage.

What.. you don't even want to log a leakage? that's information we need to 
determine if subsequent tests may fail.

A test shouldn't leak threads.. even if it failed.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@franz1981 If the test fails, whether cleanly or not, we get reported and 
should fix it. The leakage is not important here. After you fix it and it begin 
to pass, the thread check rule will perform check on it, making sure it doesn't 
leak threads. Let's say there are 2 situations:

1) first test passes but secretly leaking threads and caused second test 
fail.
2) first test failed and also leaking thread and caused second test fail.

IMO the case 1) would be much harder to debug. So in case 1) the first test 
should be checked to make sure it doesn't pollute others. In case 2) however, 
you know the first test failed and you can be sure it fails on its own, there 
for a real failure. So no matter how many more failures follow, you will have 
to fix the first failure first. Once it passes, the thread check rule kicks in 
to make sure the leak won't happen.



---


[GitHub] activemq-artemis issue #1793: ARTEMIS-1498: Openwire internal headers should...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1793
  
Please take a look here:
```
2018-01-22 17:28:59,899 WARN  [org.apache.activemq.artemis.core.server] 
Error during message dispatch: java.lang.RuntimeException: null
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireMessage.toCore(OpenwireMessage.java:787)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireMessage.toCore(OpenwireMessage.java:779)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenWireMessageConverter.toAMQMessage(OpenWireMessageConverter.java:454)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenWireMessageConverter.createMessageDispatch(OpenWireMessageConverter.java:435)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.amq.AMQConsumer.handleDeliver(AMQConsumer.java:222)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.amq.AMQSession.sendMessage(AMQSession.java:276)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl.deliverStandardMessage(ServerConsumerImpl.java:1091)
 [artemis-server-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl.proceedDeliver(ServerConsumerImpl.java:460)
 [artemis-server-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.core.server.impl.QueueImpl.proceedDeliver(QueueImpl.java:2763)
 [artemis-server-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.core.server.impl.QueueImpl.deliver(QueueImpl.java:2247)
 [artemis-server-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.core.server.impl.QueueImpl.access$1900(QueueImpl.java:106)
 [artemis-server-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.core.server.impl.QueueImpl$DeliverRunner.run(QueueImpl.java:3021)
 [artemis-server-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:42)
 [artemis-commons-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:31)
 [artemis-commons-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:66)
 [artemis-commons-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) 
[rt.jar:1.8.0_102]
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) 
[rt.jar:1.8.0_102]
at java.lang.Thread.run(Thread.java:745) [rt.jar:1.8.0_102]
Caused by: java.lang.NumberFormatException: null
at java.lang.Integer.parseInt(Integer.java:542) [rt.jar:1.8.0_102]
at java.lang.Integer.valueOf(Integer.java:766) [rt.jar:1.8.0_102]
at 
org.apache.activemq.artemis.utils.collections.TypedProperties.getIntProperty(TypedProperties.java:225)
 [artemis-commons-2.5.0-SNAPSHOT.jar:2.5.0-SNAPSHOT]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireMessage.getIntProperty(OpenwireMessage.java:681)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireMessage.getIntProperty(OpenwireMessage.java:600)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireCoreConverter.toCore(OpenwireCoreConverter.java:74)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireConverter.toCore(OpenwireConverter.java:42)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
at 
org.apache.activemq.artemis.core.protocol.openwire.OpenwireMessage.toCore(OpenwireMessage.java:785)
 [artemis-openwire-protocol-2.5.0-SNAPSHOT.jar:]
... 17 more
```
I've built up a 1 producer 1 consumer test vs 1 not durable JMS queue and 
I'm getting it.
The idea seems good as @clebertsuconic as shown me but need to address 
these issues first :+1: 


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic I'm not convinced that it is that important to log a 
leakage on a failed test. If a test fails it should be fixed. With this PR you 
won't miss a real leakage.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward 
I understand, but often a failed test is expected to fail cleanly (without 
having leaking threads): having that information ignored could affect anyway 
the other tests and probably it is hiding leaks that is better to know (eg 
oracle JDBC has leaking driver clean threads that is better to have it reported 
on failed tests too: thay won't stop never even if the thread is killed 
manually).
Have I misunderstood the change?



---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward I'm ok on not reporting a failure.. but it should always log a 
leakage.. even for failed tests..


We could instead of failing on leakage.. log.warn instead after a test 
failed... but simply ignore a leakage is not good.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic @mtaylor @franz1981 We shouldn't disable the thread check 
for all tests. What this PR do it selectively disable the check for failing 
tests. If a test passes the check is certainly on, to make sure it silently 
leaking and affects other tests.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@mtaylor the opposite.. we could have it enabled on the dev and test 
profiles.. and have it off by default. just like checkstyle is off on dev 
profile.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward test1 failed.. leaked.. than test2 failed because there's a 
thread leak... if you don't report the leak from 1.. you won't know that test2 
could fail later..


so.. add a profile to disable the thread leak check would be enough.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic @gaohoward 
Some time ago has happened that I've written a test that was failing and I 
adming that having the thread leak check has helped me to find that a test 
wasn't failing gently ie letting the system be in the most resilient state.
I think that would be perfect to disable it just for rare cases where 
really the priority is just to pass the test


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread mtaylor
Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
We already have a dev profile.  Perhaps it could be disabled there. 


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic I don't think the PR make the debugging of thread leaking 
harder.
For example suppose we have 3 tests, test1, test2 and test3 running in this 
order.
Let's say test1 passes and test2 fails. You can be sure test2 is not caused 
by leaking threads from test1, because test1 passes and the thread checking 
rule is enabled on it.
So you just need to fix test2 (in which thread leaking factor is ruled out).

This is the same as before (without this PR), but the tests will be 
finished 60 second earlier.
This is good when  a PR is submitted and Jenkins starts a job. If the PR 
causes any failure,
jenkins will finish faster and the PR owner will quickly get the report and 
starting investigate.

Does that make sense? 






---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread mtaylor
Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
How about we just have a system variable to switch off the thread check 
rule for debug purposes.  Default being on.  -DapplyThreadCheckRule=true


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
so, I don't mean to be disrespectul.. your changes are nice technically.. 
but I don't want to lose visibility of leaking threads.. even if it's for a 
failed test.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
a failing test leaking is a double failure... it failed and then leaked.. 
you can fix that situation with finally blocks...


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@gaohoward  I have been there before... finding a test that's failing 
because of a previous leak it's not fun... I don't want to back into that...

If a thread is leaking a thread.. failing or not.. I would rather like to 
see the thread leakage reported.. so I would know where it's coming from...


I don't want to regress on that.. I had spent a week trying to find a 
correlation between tests before.. hence I'm being a bit strong opinionated 
here.


---


[GitHub] activemq-artemis issue #1801: ARTEMIS-1607 OpenWire is sending responses too...

2018-01-22 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1801
  
Possibly I will add a test but is tricky and I need to run CI on it too 
:+1: 


---


[GitHub] activemq-artemis pull request #1801: ARTEMIS-1607 OpenWire is sending respon...

2018-01-22 Thread franz1981
GitHub user franz1981 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1801

ARTEMIS-1607 OpenWire is sending responses too early with durable messages

AMQSession is sending response back without waiting past I/O tasks on 
StorageManager to be finished

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/franz1981/activemq-artemis open_wire_fix

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1801.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1801


commit 6ce9bee89e24cc865509f8ba26f9261ec4e5a106
Author: Michael André Pearce 
Date:   2018-01-13T19:47:58Z

ARTEMIS-1606 - Change AddressInfo RoutingType Set to use EnumSet

Change all use from Set to EnumSet
Deprecating any old exposed interfaces but keeping for back compatibility.
Address info to avoid iterator on getRoutingType hotpath, like wise can be 
avoided where single RoutingType is passed in.

commit e2683e11068a45a9dce08d806dfeb295cf91fd71
Author: Francesco Nigro 
Date:   2018-01-22T15:06:48Z

ARTEMIS-1607 OpenWire is sending responses too early with durable messages

AMQSession is sending response back without waiting past I/O tasks on 
StorageManager to be finished




---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
I think the more interesting case is that a 'passing' test that leaking 
threads. If that's the case it's very hard to debug. But for a failing test, 
leaking threads is not that meaningful. In batch mode, you need to fix the 
first failing test, which are sure 'not' caused by leaking threads and should 
be a real failure. Any other failing tests can be ignored until the first is 
fixed. 


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
We are supposed to write tests to cleanup resources.. even when it fails.


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
failed tests are also supposed to cleanup their resources...

example


try {
}
finally {
  close()
} 


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread gaohoward
Github user gaohoward commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
@clebertsuconic If a failing test causes thread leaking and causes other 
tests to fail, the thread checking can't fix it right? 


---


[GitHub] activemq-artemis issue #1793: ARTEMIS-1498: Openwire internal headers should...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1793
  
Let me see if I understand.. you implemented OpenwireMessage here?


---


[GitHub] activemq-artemis issue #1800: ARTEMIS-1626 Disable thread leak check for fai...

2018-01-22 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1800
  
-1 I don't agree with this... a thread leaking on a failing test will make 
another test to fail.

This is about raising awareness of leaked threads.


---


[GitHub] activemq-artemis pull request #1800: ARTEMIS-1626 Disable thread leak check ...

2018-01-22 Thread gaohoward
GitHub user gaohoward opened a pull request:

https://github.com/apache/activemq-artemis/pull/1800

ARTEMIS-1626 Disable thread leak check for failing tests

The ThreadLeakCheckRule is used to check thread leaks
after each test is finished. However when a test fails, it is
not necessary to check leaking threads because the test
failure should be fixed anyway. And leaking threads in a
failed test may well be a result of the failure (once the test
is fixed the thread leak may be gone).

If a failed test also leaks threads, it takes a long time before
the thread leak check finishes (60 seconds checking time),
thus it takes a long time to finish, especially when tests are
run in batches with failures.

So to improve this, it should be reasonable to just enable
the thread leaking check for each test passes, and disable
the check when a test fails.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/gaohoward/activemq-artemis fthread_rule

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1800.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1800


commit c8e25c72b9885bb08ec9dd5d8502c273b349d46a
Author: Howard Gao 
Date:   2018-01-22T14:39:23Z

ARTEMIS-1626 Disable thread leak check for failing tests

The ThreadLeakCheckRule is used to check thread leaks
after each test is finished. However when a test fails, it is
not necessary to check leaking threads because the test
failure should be fixed anyway. And leaking threads in a
failed test may well be a result of the failure (once the test
is fixed the thread leak may be gone).

If a failed test also leaks threads, it takes a long time before
the thread leak check finishes (60 seconds checking time),
thus it takes a long time to finish, especially when tests are
run in batches with failures.

So to improve this, it should be reasonable to just enable
the thread leaking check for each test passes, and disable
the check when a test fails.




---


[GitHub] activemq-artemis pull request #1799: ARTEMIS-1625 fix moving messages

2018-01-22 Thread stanlyDoge
GitHub user stanlyDoge opened a pull request:

https://github.com/apache/activemq-artemis/pull/1799

ARTEMIS-1625 fix moving messages



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stanlyDoge/activemq-artemis E986

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1799.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1799


commit 4c347fa49a2ee07b01b2d9571e82c3767ff913bf
Author: Stanislav Knot 
Date:   2018-01-22T12:33:30Z

ARTEMIS-1625 fix moving messages




---


[GitHub] activemq-artemis pull request #1785: [ARTEMIS-1030] add support for mapping ...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1785


---


[GitHub] activemq-artemis pull request #1785: [ARTEMIS-1030] add support for mapping ...

2018-01-22 Thread gtully
Github user gtully commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1785#discussion_r162907379
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/VirtualTopicToFQQNOpenWireTest.java
 ---
@@ -0,0 +1,89 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.openwire;
+
+import javax.jms.Connection;
+import javax.jms.Destination;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.util.Set;
+
+import org.apache.activemq.ActiveMQConnectionFactory;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import org.apache.activemq.artemis.core.config.Configuration;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
--- End diff --

@gaohoward  thanks. sorry for the wasted cycles. Pushed an update that 
rebases and fixes that violation.


---


[GitHub] activemq-artemis issue #1775: ARTEMIS-1587 Add setting to control the queue ...

2018-01-22 Thread jostbg
Github user jostbg commented on the issue:

https://github.com/apache/activemq-artemis/pull/1775
  
@michaelandrepearce All thoughts and ideas are appreciated :-) I could 
achieve what we need (despite it violates JMS spec) by changing the durable 
flag of a QueueConfig in beforeCreateQueue via reflection to false but 
especially since the durable field is final (which I can still change by 
removing the final flag via reflection first) this feels like a risky operation.

So if I could create the queue that I need programmatically in 
beforeCreateQueue() and the ActiveMQServerImpl would - instead of directly 
executing `queueFactory.createQueueWith(queueConfig);` after the broker plugins 
are called - check if the queue now already exists I would have all the 
flexibility I need.


---


[GitHub] activemq-artemis pull request #1794: [ARTEMIS-1552] differenciate empty fram...

2018-01-22 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1794


---


[GitHub] activemq-artemis issue #1794: [ARTEMIS-1552] differenciate empty frame from ...

2018-01-22 Thread andytaylor
Github user andytaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/1794
  
looks good will get it merged


---


[GitHub] activemq-artemis issue #1775: ARTEMIS-1587 Add setting to control the queue ...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1775
  
On the note of broker plugin, you do realise you can intercept 
beforeCreateAddress and beforeCreateQueue and a lot of other functions, this 
way you could do what you want without affecting the core broker.


---


[GitHub] activemq-artemis issue #1795: [ARTEMIS-1609] Add distinct name/address for J...

2018-01-22 Thread jmesnil
Github user jmesnil commented on the issue:

https://github.com/apache/activemq-artemis/pull/1795
  
Commit amended with a test


---


[GitHub] activemq-artemis issue #1775: ARTEMIS-1587 Add setting to control the queue ...

2018-01-22 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1775
  
As I stated there is already a global flag, if there is an address level 
flag then this should have the exact same behaviour. Also I’m not advocating 
changing the global flag behaviour.

We provided you an option you can achieve what you needed using multicast 
addresses (jms topic).

The plugin idea was just another that meant you don’t need the change and 
don’t affect other users of the broker as the plugin would be your own, if 
you are soo insistent on using but overriding the JMS queue logic in the amqp 
jms client.




---


[GitHub] activemq-artemis issue #1775: ARTEMIS-1587 Add setting to control the queue ...

2018-01-22 Thread jostbg
Github user jostbg commented on the issue:

https://github.com/apache/activemq-artemis/pull/1775
  
@michaelandrepearce I cannot modify the durable flag of AMQP messages as 
they are meant to be immutable and it would prevent using features like digital 
signatures AFAIK. What we are looking for - no matter at which level it is 
configured - is a way to opt-out of message persistence for a queues in a 
certain namespace (or even globally) since the SLA/QoS of our service does not 
guarantee message delivery in case of broker failure. But if we receive more 
messages than what can be stored in the heap or receive very large messages we 
would still want/need paging/large messages support to ensure stable operation.


---