[GitHub] activemq-artemis pull request #:

[jdanekrh]

Github user jdanekrh commented on the pull request:


https://github.com/apache/activemq-artemis/commit/63e0c0d310850fb59f800d2cc5cf9c5cfc0060ec#commitcomment-27458675
  
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java:
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
 on line 282:
>>> CID 1465080:  Concurrent data access violations  
(GUARDED_BY_VIOLATION)
>>> Accessing "message" without holding lock "PagedReferenceImpl.this". 
Elsewhere, 
"org.apache.activemq.artemis.core.paging.cursor.PagedReferenceImpl.message" is 
accessed with "PagedReferenceImpl.this" held 3 out of 4 times.

https://scan.coverity.com/projects/apacheactivemqartemis


---

[GitHub] activemq-artemis pull request #:

[jdanekrh]

Github user jdanekrh commented on the pull request:


https://github.com/apache/activemq-artemis/commit/63e0c0d310850fb59f800d2cc5cf9c5cfc0060ec#commitcomment-27458655
  
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java:
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java
 on line 285:
>>> CID 1465081:  Null pointer dereferences  (FORWARD_NULL)
>>> Unboxing null object "largeMessage".

https://scan.coverity.com/projects/apacheactivemqartemis


---

[GitHub] activemq-artemis pull request #1852: [ARTEMIS-1662] Reduce log of "INFO [org...

[michaelandrepearce]

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1852#discussion_r167332364
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java
 ---
@@ -64,7 +64,7 @@
@Message(id = 211001, value = "session created", format = 
Message.Format.MESSAGE_FORMAT)
void dumpingSessionStack(@Cause Exception e);
 
-   @LogMessage(level = Logger.Level.INFO)
+   @LogMessage(level = Logger.Level.DEBUG)
--- End diff --

disagree still but as its 2 v 1 and its logging at the end of the day 
(bigger issues to really worry on) ill make it a -0. Ive merged the PR as is.


---

[GitHub] activemq-artemis pull request #1852: [ARTEMIS-1662] Reduce log of "INFO [org...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1862: ARTEMIS-1676 Support overriding of JAVA...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1862#discussion_r167280403
  
--- Diff: 
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/artemis.profile
 ---
@@ -21,14 +21,20 @@ ARTEMIS_INSTANCE='${artemis.instance}'
 # The logging config will need an URI
 # this will be encoded in case you use spaces or special characters
 # on your directory structure
-ARTEMIS_INSTANCE_URI='${artemis.instance.uri}'
 
-# Cluster Properties: Used to pass arguments to ActiveMQ Artemis which can 
be referenced in broker.xml
-#ARTEMIS_CLUSTER_PROPS="-Dactivemq.remoting.default.port=61617 
-Dactivemq.remoting.amqp.port=5673 -Dactivemq.remoting.stomp.port=61614 
-Dactivemq.remoting.hornetq.port=5446"
+if [ -z "$ARTEMIS_INSTANCE_URI" ]; then
+ARTEMIS_INSTANCE_URI='${artemis.instance.uri}'
+fi
 
+# Cluster Properties: Used to pass arguments to ActiveMQ Artemis which can 
be referenced in broker.xml
+if [ -z "$ARTEMIS_CLUSTER_PROPS" ]; then
+#ARTEMIS_CLUSTER_PROPS="-Dactivemq.remoting.default.port=61617 
-Dactivemq.remoting.amqp.port=5673 -Dactivemq.remoting.stomp.port=61614 
-Dactivemq.remoting.hornetq.port=5446"
--- End diff --

what's the point of adding this commented out? the user would have to add 
it anyways?


---

[GitHub] activemq-artemis pull request #1862: ARTEMIS-1676 Support overriding of JAVA...

[mtaylor]

GitHub user mtaylor opened a pull request:

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

ARTEMIS-1676 Support overriding of JAVA_ARGS via env variable



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

$ git pull https://github.com/mtaylor/activemq-artemis ARTEMIS-1676

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

https://github.com/apache/activemq-artemis/pull/1862.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 #1862


commit 62d904f47979afae105981144053f30430ecd310
Author: Martyn Taylor 
Date:   2018-02-09T16:29:44Z

ARTEMIS-1676 Support overriding of JAVA_ARGS via env variable




---

[GitHub] activemq-artemis issue #1861: ARTEMIS-1675 Adding --safe option on print-dat...

[clebertsuconic]

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/1861
  
@michaelandrepearce I think you would like this one. I talked to you in 
person about it.


---

[GitHub] activemq-artemis pull request #1861: ARTEMIS-1675 Adding --safe option on pr...

[clebertsuconic]

GitHub user clebertsuconic opened a pull request:

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

ARTEMIS-1675 Adding --safe option on print-data

This is good when you are a customer and an artemis engineer (e.g. me) asks 
your journal print-data but you can't do it because that would expose your 
user's data. If you do artemis data print --safe, that will only expose the 
journal structure without exposing user's data and eliminate any liability 
between the engineer and users.

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

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

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

https://github.com/apache/activemq-artemis/pull/1861.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 #1861


commit 5c61420c6964924a64d68bdcbf8ce9c8480f0163
Author: Clebert Suconic 
Date:   2018-02-09T16:09:57Z

ARTEMIS-1675 Adding --safe option on print-data

This is good when you are a customer and an artemis engineer (e.g. me) asks 
your journal print-data but you can't do it because that would expose your 
user's data. If you do artemis data print --safe, that will only expose the 
journal structure without exposing user's data and eliminate any liability 
between the engineer and users.




---

[GitHub] activemq-artemis pull request #1852: [ARTEMIS-1662] Reduce log of "INFO [org...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1852#discussion_r167269720
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java
 ---
@@ -64,7 +64,7 @@
@Message(id = 211001, value = "session created", format = 
Message.Format.MESSAGE_FORMAT)
void dumpingSessionStack(@Cause Exception e);
 
-   @LogMessage(level = Logger.Level.INFO)
+   @LogMessage(level = Logger.Level.DEBUG)
--- End diff --

@michaelandrepearce actually this is getting too verbose on clients.
We can use log.debug on the client... and you can increase logger if you 
really want it.

Some users will complain the opposite.


---

[GitHub] activemq-artemis pull request #1852: [ARTEMIS-1662] Reduce log of "INFO [org...

[michaelandrepearce]

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1852#discussion_r167261076
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java
 ---
@@ -64,7 +64,7 @@
@Message(id = 211001, value = "session created", format = 
Message.Format.MESSAGE_FORMAT)
void dumpingSessionStack(@Cause Exception e);
 
-   @LogMessage(level = Logger.Level.INFO)
+   @LogMessage(level = Logger.Level.DEBUG)
--- End diff --

Agreed server side, but client side this is far too useful to lose. 



---

[GitHub] activemq-artemis pull request #1852: [ARTEMIS-1662] Reduce log of "INFO [org...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1852#discussion_r167242748
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java
 ---
@@ -64,7 +64,7 @@
@Message(id = 211001, value = "session created", format = 
Message.Format.MESSAGE_FORMAT)
void dumpingSessionStack(@Cause Exception e);
 
-   @LogMessage(level = Logger.Level.INFO)
+   @LogMessage(level = Logger.Level.DEBUG)
--- End diff --

@michaelandrepearce I was actually considering doing this myself... the 
clients are too verbose.. some users will open and create connections all the 
time.. this becomes too verbose.


---

[GitHub] activemq-artemis pull request #1860: [ARTEMIS-1670] NPE was found in when dr...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1859: ARTEMIS-1659 - Only reload configuratio...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #:

[jdanekrh]

Github user jdanekrh commented on the pull request:


https://github.com/apache/activemq-artemis/commit/dc41f3ca491e96e199290a225fdaa07ac05d66df#commitcomment-27450542
  
There are ways. Suppressions can be provided, in multiple ways (special 
form commits in source, or what Coverity calls "modeling files"), the daily 
report e-mail could be copied to project mailinglist for quick triage, ... 
FindBugs/SpotBugs can be executed locally on dev machines.

There is fairly massive backlog of things in Coverity already (over 1000 
items) and that can be used to get an idea of things it flags and to estimate 
future false positive rate.


---

[GitHub] activemq-artemis pull request #:

[michaelandrepearce]

Github user michaelandrepearce commented on the pull request:


https://github.com/apache/activemq-artemis/commit/dc41f3ca491e96e199290a225fdaa07ac05d66df#commitcomment-27449579
  
@jdanekrh its a good idea def good to catch it whilst code is still fresh 
in the authors head. FYI i flagged you on the commit, but i fixed the ones i 
believe were caused by this commit.

The only bit will be ensuring you dont get too many false positives (will 
annoy people), and like wise doing it consistently.


---

[GitHub] activemq-artemis pull request #:

[jdanekrh]

Github user jdanekrh commented on the pull request:


https://github.com/apache/activemq-artemis/commit/dc41f3ca491e96e199290a225fdaa07ac05d66df#commitcomment-27446263
  
Issues created and linked onto 
https://issues.apache.org/jira/browse/ARTEMIS-1400.


---

[GitHub] activemq-artemis pull request #:

[jdanekrh]

Github user jdanekrh commented on the pull request:


https://github.com/apache/activemq-artemis/commit/dc41f3ca491e96e199290a225fdaa07ac05d66df#commitcomment-27446067
  
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
 on line 2753:
It is the right line. It was preexisting, after closer examination. 
(Coverity marked the previous item it had for this as fixed and created a new 
item, which is absolutely identical, barring the item id. Apparently, the item 
IDs are not completely stable between scan runs, if the code around changes.)


---

[GitHub] activemq-artemis pull request #:

[jdanekrh]

Github user jdanekrh commented on the pull request:


https://github.com/apache/activemq-artemis/commit/dc41f3ca491e96e199290a225fdaa07ac05d66df#commitcomment-27446032
  
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
In 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
 on line 2862:
bq. If you want to flag coverity issues in code suggest raising it as 
sepate jira and issue

I am usually doing that, after I've investigated and decided it is a real 
issue. I am subscribed to the daily Coverity email, which includes any new 
items it found since the previous day. (With the day boundary at 20:00 CET.) 
This time I decided to try to go to the commits and try to "move the 
investigative burden" onto the authors, just to see what happens... Also, the 
Boolean comparisons seemed clear cut.

This one is not a clear cut, I must admit. If I do investigate it and think 
it is a real problem, I will report it.


---