[GitHub] activemq-artemis pull request #1532: ARTEMIS-1419 OpenWire advisory message ...

[gaohoward]

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

https://github.com/apache/activemq-artemis/pull/1532#discussion_r138784150
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/SimpleOpenWireTest.java
 ---
@@ -1585,6 +1586,47 @@ public void testXAResourceRolledBackRemoved() throws 
Exception {
   assertNull(transaction);
}
 
+   @Test
+   public void testTempQueueLeak() throws Exception {
+  final Connection[] connections = new Connection[20];
+
+  try {
+ for (int i = 0; i < connections.length; i++) {
+connections[i] = factory.createConnection();
+connections[i].start();
+ }
+
+ Session session = connections[0].createSession(false, 
Session.AUTO_ACKNOWLEDGE);
+
+ for (int i = 0; i < connections.length; i++) {
+TemporaryQueue temporaryQueue = session.createTemporaryQueue();
+temporaryQueue.delete();
+ }
+
+ Object[] addressResources = 
server.getManagementService().getResources(AddressControl.class);
+ AddressControl addressControl = null;
+
+ for (Object addressResource : addressResources) {
+
+if (((AddressControl) 
addressResource).getAddress().equals("ActiveMQ.Advisory.TempQueue")) {
+   addressControl = (AddressControl) addressResource;
+}
+ }
+
+ assertNotNull("addressControl for temp advisory", addressControl);
+
+ //sleep a bit to allow message count to go down.
+ Thread.sleep(50);
--- End diff --

Thanks! :)


---

Re: [DISCUSS] release process improvements

[Clebert Suconic]

Ok, fair enough... I can see this as a process improvement.

I wasn't just understanding what you were proposing clearly enough.

I just added this script here:
https://github.com/apache/activemq-artemis/blob/master/scripts/download-release.sh


I didn't update the RELEASE.md yet...


I would add that during the release, you use the download-release from
the staged mvn repo using that script into the dev area.
The vote would have the staged download on dev, and we just make a
simple copy from one place to the other.. and remove the previous
thing.


But I think this should be also done on ActiveMQ 5 releases.



The thing that threw me of was when you mentioned extra work.. there's
no extra work here :)
It's actually saving me from screwing up eventually, so I take it as
an improvement.


On Wed, Sep 13, 2017 at 1:19 PM, Robbie Gemmell
 wrote:
> Yes, thats essentially what I mean and do, I have a txt file I keep
> some comments in as notes, and can source as a script to download the
> various tars and signatures from nexus (though it could equally pull
> them from the maven local repo, verifying the Nexus ones is good I
> think), verify the signature, and generate new MD5+SHA512 checksum
> files that include the filename details (it could instead manipualte
> the MD5 one rather than create new). I execute that in a directory
> within a checkout of the dist dev, then commit the files after a
> little validation and open the vote.
>
> The process of putting the files in the dist dev area is mostly the
> same as what will be getting done now for the final release, it just
> uses a different subtree of the same parent dist svn repo, so for
> example you would use a subdir of
> https://dist.apache.org/repos/dist/dev/activemq/activemq-artemis/
> before the vote rather than of
> https://dist.apache.org/repos/dist/release/activemq/activemq-artemis/
> after the vote.
>
> To complete the example, had the files for the recent Artemis 2.3.0
> vote been in the dist dev area already you would just do something
> like this to complete the release once the vote had passed:
> svn cp -m "add files for activemq-artemis-2.3.0"
> https://dist.apache.org/repos/dist/dev/activemq/activemq-artemis/2.3.0-rc1
> https://dist.apache.org/repos/dist/release/activemq/activemq-artemis/2.3.0
>
> Robbie
>
> On 13 September 2017 at 17:52, Clebert Suconic
>  wrote:
>> I actually see how to make the copy into dev... let me play with it a
>> little bit
>>
>> On Wed, Sep 13, 2017 at 12:44 PM, Clebert Suconic
>>  wrote:
>>> what about this:
>>>
>>> Currently mvn release and mvn upload will always send the release to nexus,
>>>
>>> So what about:
>>>
>>> - we provide an script to artemis to download the correct bits of the
>>> release, the release manager would use that script to perform such
>>> download.
>>> - The release manager would place it on the dev repository Robbie is
>>> mentioning... (that means.. we wouldn't really have an extra step).
>>>
>>>
>>> On thing I'm not sure how to do is... how to upload it to the dev dist
>>> at https://dist.apache.org/repos/dist/dev/activemq/
>>>
>>> and how we would make the final move? just a regular copy?
>>>
>>>
>>>
>>>
>>>
>>> On Wed, Sep 13, 2017 at 9:49 AM, Robbie Gemmell
>>>  wrote:
 On 13 September 2017 at 14:35, Clebert Suconic
  wrote:
> On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
> wrote:
>
>> This was less about time, though there is some benefit in that regard,
>> with how much depending on how particular people actually verify the
>> checksums I guess.
>
>
>
> Actually this is kind of moot. nexus does that check for you.  You cannot
> upload a release with a checksum broken. It won't let you close.
>
> Like. Last week I had to restart the release once because MVN upload broke
> the checksum somewhere.
> --
> Clebert Suconic

 Whether the files in Nexus are ok isn't sufficient. The archives and
 checksum files in the dist repo are the mirrorer official release
 artifacts (and strictly only the source ones at that), and Nexus cant
 check those. There could be a problem deploying those bits for a
 variety of reasons, so we check they are ok. Users downloading the
 release archives also tend to grab the checksums from the dist repo
 because that is their official source, in order to verify downloads
 that have come from the third party mirrors which dont store the
 checksums for obvious reasons.

 Robbie
>>>
>>>
>>>
>>> --
>>> Clebert Suconic
>>
>>
>>
>> --
>> Clebert Suconic



-- 
Clebert Suconic

[GitHub] activemq-artemis pull request #1531: ARTEMIS-1410 remove redundant code proc...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1535: ARTEMIS-1421 duplicate queue possible i...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1532: ARTEMIS-1419 OpenWire advisory message ...

[clebertsuconic]

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

https://github.com/apache/activemq-artemis/pull/1532#discussion_r138755728
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/SimpleOpenWireTest.java
 ---
@@ -1585,6 +1586,47 @@ public void testXAResourceRolledBackRemoved() throws 
Exception {
   assertNull(transaction);
}
 
+   @Test
+   public void testTempQueueLeak() throws Exception {
+  final Connection[] connections = new Connection[20];
+
+  try {
+ for (int i = 0; i < connections.length; i++) {
+connections[i] = factory.createConnection();
+connections[i].start();
+ }
+
+ Session session = connections[0].createSession(false, 
Session.AUTO_ACKNOWLEDGE);
+
+ for (int i = 0; i < connections.length; i++) {
+TemporaryQueue temporaryQueue = session.createTemporaryQueue();
+temporaryQueue.delete();
+ }
+
+ Object[] addressResources = 
server.getManagementService().getResources(AddressControl.class);
+ AddressControl addressControl = null;
+
+ for (Object addressResource : addressResources) {
+
+if (((AddressControl) 
addressResource).getAddress().equals("ActiveMQ.Advisory.TempQueue")) {
+   addressControl = (AddressControl) addressResource;
+}
+ }
+
+ assertNotNull("addressControl for temp advisory", addressControl);
+
+ //sleep a bit to allow message count to go down.
+ Thread.sleep(50);
--- End diff --

I did it myself.. merged


---

[GitHub] activemq-artemis pull request #1532: ARTEMIS-1419 OpenWire advisory message ...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1527: ARTEMIS-1405: Fix PurgeOnNoConsumer que...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis issue #1535: ARTEMIS-1421 duplicate queue possible in XML

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1535
  
Good catch


---

[GitHub] activemq-artemis pull request #1535: ARTEMIS-1421 duplicate queue possible i...

[jbertram]

GitHub user jbertram opened a pull request:

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

ARTEMIS-1421 duplicate queue possible in XML



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

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

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

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


commit 44c05f10792ad9560e4499abd92c6f009f060e7d
Author: Justin Bertram 
Date:   2017-09-13T15:01:39Z

ARTEMIS-1421 duplicate queue possible in XML




---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[michaelandrepearce]

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

https://github.com/apache/activemq-artemis/pull/1534#discussion_r138720142
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
 ---
@@ -206,6 +207,8 @@
 
final AtomicBoolean warningPrinted = new AtomicBoolean(false);
 
+   private static int communicationTimeout = 30; //seconds
--- End diff --

I have done a very quick (not 100% complete) example of the way to better 
config this and how to have default (with disabled), and also how to add 
specific logger for the other review comment i left.

If you want to look at:


https://github.com/michaelandrepearce/activemq-artemis/commit/4e2026c0fce5a75dcb9551fc4237d3cba9abfb23



---

[GitHub] activemq-artemis issue #1527: ARTEMIS-1405: Fix PurgeOnNoConsumer queue conf...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1527
  
@mtaylor @franz1981 @clebertsuconic any review comments on this i need to 
address, or ok to merge?


---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[michaelandrepearce]

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

https://github.com/apache/activemq-artemis/pull/1534#discussion_r138716684
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
 ---
@@ -843,4 +847,12 @@ private Throwable getRootCause(Throwable throwable) {
  return (list.size() < 2 ? throwable : list.get(list.size() - 1));
   }
}
+
+   public static int getCommunicationTimeout() {
--- End diff --

as noted about should be configured by configuration per acceptor instance, 
these then are redundant.


---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[michaelandrepearce]

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

https://github.com/apache/activemq-artemis/pull/1534#discussion_r138716284
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/ActiveMQChannelHandler.java
 ---
@@ -89,6 +90,11 @@ public void exceptionCaught(final ChannelHandlerContext 
ctx, final Throwable cau
   if (!active) {
  return;
   }
+
+  if (cause instanceof ReadTimeoutException) {
+ ActiveMQClientLogger.LOGGER.error("Connection without 
communication timeout has expired.");
--- End diff --

Should probably have a specific error code defined in ActiveMQClientLogger 
and use that.


---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[michaelandrepearce]

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

https://github.com/apache/activemq-artemis/pull/1534#discussion_r138714254
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
 ---
@@ -206,6 +207,8 @@
 
final AtomicBoolean warningPrinted = new AtomicBoolean(false);
 
+   private static int communicationTimeout = 30; //seconds
--- End diff --

Also shouldn't be static, as you may want to configure different acceptors 
to different configs


---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[michaelandrepearce]

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

https://github.com/apache/activemq-artemis/pull/1534#discussion_r138714062
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
 ---
@@ -206,6 +207,8 @@
 
final AtomicBoolean warningPrinted = new AtomicBoolean(false);
 
+   private static int communicationTimeout = 30; //seconds
--- End diff --

This also should be config based with defaults declared as per how other 
bits are configured and defaulted for netty


---

[GitHub] activemq-artemis issue #1533: ARTEMIS-1418 AIO Shutdown on IOError and loggi...

[jbertram]

Github user jbertram commented on the issue:

https://github.com/apache/activemq-artemis/pull/1533
  
Looks like there was a compilation error in this PR which was merged.


---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[michaelandrepearce]

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

https://github.com/apache/activemq-artemis/pull/1534#discussion_r138713895
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
 ---
@@ -206,6 +207,8 @@
 
final AtomicBoolean warningPrinted = new AtomicBoolean(false);
 
+   private static int communicationTimeout = 30; //seconds
--- End diff --

This should be named read time out similar to netty naming


---

Re: [DISCUSS] release process improvements

[Robbie Gemmell]

Yes, thats essentially what I mean and do, I have a txt file I keep
some comments in as notes, and can source as a script to download the
various tars and signatures from nexus (though it could equally pull
them from the maven local repo, verifying the Nexus ones is good I
think), verify the signature, and generate new MD5+SHA512 checksum
files that include the filename details (it could instead manipualte
the MD5 one rather than create new). I execute that in a directory
within a checkout of the dist dev, then commit the files after a
little validation and open the vote.

The process of putting the files in the dist dev area is mostly the
same as what will be getting done now for the final release, it just
uses a different subtree of the same parent dist svn repo, so for
example you would use a subdir of
https://dist.apache.org/repos/dist/dev/activemq/activemq-artemis/
before the vote rather than of
https://dist.apache.org/repos/dist/release/activemq/activemq-artemis/
after the vote.

To complete the example, had the files for the recent Artemis 2.3.0
vote been in the dist dev area already you would just do something
like this to complete the release once the vote had passed:
svn cp -m "add files for activemq-artemis-2.3.0"
https://dist.apache.org/repos/dist/dev/activemq/activemq-artemis/2.3.0-rc1
https://dist.apache.org/repos/dist/release/activemq/activemq-artemis/2.3.0

Robbie

On 13 September 2017 at 17:52, Clebert Suconic
 wrote:
> I actually see how to make the copy into dev... let me play with it a
> little bit
>
> On Wed, Sep 13, 2017 at 12:44 PM, Clebert Suconic
>  wrote:
>> what about this:
>>
>> Currently mvn release and mvn upload will always send the release to nexus,
>>
>> So what about:
>>
>> - we provide an script to artemis to download the correct bits of the
>> release, the release manager would use that script to perform such
>> download.
>> - The release manager would place it on the dev repository Robbie is
>> mentioning... (that means.. we wouldn't really have an extra step).
>>
>>
>> On thing I'm not sure how to do is... how to upload it to the dev dist
>> at https://dist.apache.org/repos/dist/dev/activemq/
>>
>> and how we would make the final move? just a regular copy?
>>
>>
>>
>>
>>
>> On Wed, Sep 13, 2017 at 9:49 AM, Robbie Gemmell
>>  wrote:
>>> On 13 September 2017 at 14:35, Clebert Suconic
>>>  wrote:
 On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
 wrote:

> This was less about time, though there is some benefit in that regard,
> with how much depending on how particular people actually verify the
> checksums I guess.



 Actually this is kind of moot. nexus does that check for you.  You cannot
 upload a release with a checksum broken. It won't let you close.

 Like. Last week I had to restart the release once because MVN upload broke
 the checksum somewhere.
 --
 Clebert Suconic
>>>
>>> Whether the files in Nexus are ok isn't sufficient. The archives and
>>> checksum files in the dist repo are the mirrorer official release
>>> artifacts (and strictly only the source ones at that), and Nexus cant
>>> check those. There could be a problem deploying those bits for a
>>> variety of reasons, so we check they are ok. Users downloading the
>>> release archives also tend to grab the checksums from the dist repo
>>> because that is their official source, in order to verify downloads
>>> that have come from the third party mirrors which dont store the
>>> checksums for obvious reasons.
>>>
>>> Robbie
>>
>>
>>
>> --
>> Clebert Suconic
>
>
>
> --
> Clebert Suconic

Re: [DISCUSS] release process improvements

[Clebert Suconic]

I actually see how to make the copy into dev... let me play with it a
little bit

On Wed, Sep 13, 2017 at 12:44 PM, Clebert Suconic
 wrote:
> what about this:
>
> Currently mvn release and mvn upload will always send the release to nexus,
>
> So what about:
>
> - we provide an script to artemis to download the correct bits of the
> release, the release manager would use that script to perform such
> download.
> - The release manager would place it on the dev repository Robbie is
> mentioning... (that means.. we wouldn't really have an extra step).
>
>
> On thing I'm not sure how to do is... how to upload it to the dev dist
> at https://dist.apache.org/repos/dist/dev/activemq/
>
> and how we would make the final move? just a regular copy?
>
>
>
>
>
> On Wed, Sep 13, 2017 at 9:49 AM, Robbie Gemmell
>  wrote:
>> On 13 September 2017 at 14:35, Clebert Suconic
>>  wrote:
>>> On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
>>> wrote:
>>>
 This was less about time, though there is some benefit in that regard,
 with how much depending on how particular people actually verify the
 checksums I guess.
>>>
>>>
>>>
>>> Actually this is kind of moot. nexus does that check for you.  You cannot
>>> upload a release with a checksum broken. It won't let you close.
>>>
>>> Like. Last week I had to restart the release once because MVN upload broke
>>> the checksum somewhere.
>>> --
>>> Clebert Suconic
>>
>> Whether the files in Nexus are ok isn't sufficient. The archives and
>> checksum files in the dist repo are the mirrorer official release
>> artifacts (and strictly only the source ones at that), and Nexus cant
>> check those. There could be a problem deploying those bits for a
>> variety of reasons, so we check they are ok. Users downloading the
>> release archives also tend to grab the checksums from the dist repo
>> because that is their official source, in order to verify downloads
>> that have come from the third party mirrors which dont store the
>> checksums for obvious reasons.
>>
>> Robbie
>
>
>
> --
> Clebert Suconic



-- 
Clebert Suconic

Re: [DISCUSS] release process improvements

[Clebert Suconic]

what about this:

Currently mvn release and mvn upload will always send the release to nexus,

So what about:

- we provide an script to artemis to download the correct bits of the
release, the release manager would use that script to perform such
download.
- The release manager would place it on the dev repository Robbie is
mentioning... (that means.. we wouldn't really have an extra step).


On thing I'm not sure how to do is... how to upload it to the dev dist
at https://dist.apache.org/repos/dist/dev/activemq/

and how we would make the final move? just a regular copy?





On Wed, Sep 13, 2017 at 9:49 AM, Robbie Gemmell
 wrote:
> On 13 September 2017 at 14:35, Clebert Suconic
>  wrote:
>> On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
>> wrote:
>>
>>> This was less about time, though there is some benefit in that regard,
>>> with how much depending on how particular people actually verify the
>>> checksums I guess.
>>
>>
>>
>> Actually this is kind of moot. nexus does that check for you.  You cannot
>> upload a release with a checksum broken. It won't let you close.
>>
>> Like. Last week I had to restart the release once because MVN upload broke
>> the checksum somewhere.
>> --
>> Clebert Suconic
>
> Whether the files in Nexus are ok isn't sufficient. The archives and
> checksum files in the dist repo are the mirrorer official release
> artifacts (and strictly only the source ones at that), and Nexus cant
> check those. There could be a problem deploying those bits for a
> variety of reasons, so we check they are ok. Users downloading the
> release archives also tend to grab the checksums from the dist repo
> because that is their official source, in order to verify downloads
> that have come from the third party mirrors which dont store the
> checksums for obvious reasons.
>
> Robbie



-- 
Clebert Suconic

[GitHub] activemq-artemis pull request #1533: ARTEMIS-1418 AIO Shutdown on IOError an...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis issue #1534: ARTEMIS-1420 limit non-ssl connection hangs up...

[jbertram]

Github user jbertram commented on the issue:

https://github.com/apache/activemq-artemis/pull/1534
  
Can you elaborate on the use case where this is required?  The broker 
already has functionality to close dead connections.  By implementing this 
feature you are hard-coding a 30 timeout for *all* clients regardless of how 
they've configured their heart-beats.


---

[GitHub] activemq-artemis pull request #1534: ARTEMIS-1420 limit non-ssl connection h...

[stanlyDoge]

GitHub user stanlyDoge opened a pull request:

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

ARTEMIS-1420 limit non-ssl connection hangs up exceeding client(s)



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

$ git pull https://github.com/stanlyDoge/activemq-artemis-1 ARTEMIS-1420

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

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


commit 518e5c5491aa7261b6864b81617434aadd6e34c1
Author: Stanislav Knot 
Date:   2017-09-13T14:50:42Z

ARTEMIS-1420




---

[GitHub] activemq-artemis pull request #1532: ARTEMIS-1419 OpenWire advisory message ...

[clebertsuconic]

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

https://github.com/apache/activemq-artemis/pull/1532#discussion_r138630255
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/SimpleOpenWireTest.java
 ---
@@ -1585,6 +1586,47 @@ public void testXAResourceRolledBackRemoved() throws 
Exception {
   assertNull(transaction);
}
 
+   @Test
+   public void testTempQueueLeak() throws Exception {
+  final Connection[] connections = new Connection[20];
+
+  try {
+ for (int i = 0; i < connections.length; i++) {
+connections[i] = factory.createConnection();
+connections[i].start();
+ }
+
+ Session session = connections[0].createSession(false, 
Session.AUTO_ACKNOWLEDGE);
+
+ for (int i = 0; i < connections.length; i++) {
+TemporaryQueue temporaryQueue = session.createTemporaryQueue();
+temporaryQueue.delete();
+ }
+
+ Object[] addressResources = 
server.getManagementService().getResources(AddressControl.class);
+ AddressControl addressControl = null;
+
+ for (Object addressResource : addressResources) {
+
+if (((AddressControl) 
addressResource).getAddress().equals("ActiveMQ.Advisory.TempQueue")) {
+   addressControl = (AddressControl) addressResource;
+}
+ }
+
+ assertNotNull("addressControl for temp advisory", addressControl);
+
+ //sleep a bit to allow message count to go down.
+ Thread.sleep(50);
--- End diff --

Instead of this... please use Wait.waitFor() ?


---

Re: [DISCUSS] release process improvements

[Robbie Gemmell]

On 13 September 2017 at 14:35, Clebert Suconic
 wrote:
> On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
> wrote:
>
>> This was less about time, though there is some benefit in that regard,
>> with how much depending on how particular people actually verify the
>> checksums I guess.
>
>
>
> Actually this is kind of moot. nexus does that check for you.  You cannot
> upload a release with a checksum broken. It won't let you close.
>
> Like. Last week I had to restart the release once because MVN upload broke
> the checksum somewhere.
> --
> Clebert Suconic

Whether the files in Nexus are ok isn't sufficient. The archives and
checksum files in the dist repo are the mirrorer official release
artifacts (and strictly only the source ones at that), and Nexus cant
check those. There could be a problem deploying those bits for a
variety of reasons, so we check they are ok. Users downloading the
release archives also tend to grab the checksums from the dist repo
because that is their official source, in order to verify downloads
that have come from the third party mirrors which dont store the
checksums for obvious reasons.

Robbie

Re: [DISCUSS] release process improvements

[Timothy Bish]

On 09/13/2017 09:35 AM, Clebert Suconic wrote:

On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
wrote:


This was less about time, though there is some benefit in that regard,
with how much depending on how particular people actually verify the
checksums I guess.



Actually this is kind of moot. nexus does that check for you.  You cannot
upload a release with a checksum broken. It won't let you close.

Like. Last week I had to restart the release once because MVN upload broke
the checksum somewhere.


Just for clarification here the "official release" we are referring to 
here is the source and binary distributions that are uploaded to dist 
and linked on the website.  The bits in maven are not the official 
release, we don't need to do them in order to produce an official 
release of the project.  The bits Robbie is referring to are the one's 
uploaded to dist and are the one's you should be verifying when voting 
on a release.


The benefits gained then are that the checksum files we publish are 
easily validated by a review or you know by the people downloading the 
official release bits who want to validate that they match.  By 
producing a more correct checksum file it's much easier to use CLI tools 
to validate that.  The other benefit is that what gets voted on (the 
official thing, not the maven bits) is the same as what gets published 
to the download site because you just svn cp those to their end location.



--
Tim Bish
twitter: @tabish121
blog: http://timbish.blogspot.com/

Re: [DISCUSS] release process improvements

[Clebert Suconic]

On Wed, Sep 13, 2017 at 9:21 AM Robbie Gemmell 
wrote:

> This was less about time, though there is some benefit in that regard,
> with how much depending on how particular people actually verify the
> checksums I guess.



Actually this is kind of moot. nexus does that check for you.  You cannot
upload a release with a checksum broken. It won't let you close.

Like. Last week I had to restart the release once because MVN upload broke
the checksum somewhere.
-- 
Clebert Suconic

Re: [DISCUSS] release process improvements

[Robbie Gemmell]

This was less about time, though there is some benefit in that regard,
with how much depending on how particular people actually verify the
checksums I guess. That said, regardless of how much time difference
there actually was, given there is no real cost to doing it (adding
line[s] to an existing script), making things easier for testers and
more importantly end users eventually downloading the release to use
the checksum files for verifying their downloads seems worthwhile to
me.

A perhaps more important element is about removing a fragility in the
release process that doesn't need to be there. As you say its easy to
make mistakes, and using the dist dev repo makes it easier for us to
catch issues with the dist repo deployment while we are testing the
release, simply by moving some of the existing work a little earlier.

Also adopting the recommendation of the distribution policy seemed
beneficial to me, the checksums are there for a purpose and following
the recommendation to provide a SHA512 checksum improves their use at
no significant cost.

To be clear, there is no change around the use of Nexus itself, that
all stays the way it is, this is purely about basic changes to the
existing process of separately putting the official release bits in
the dist repo as used for the mirrors and download page etc.

Robbie

On 13 September 2017 at 13:30, Martyn Taylor  wrote:
> I don't think the responses were negative.  I don't think the benefit was
> made clear and people are questioning whether it's worth the effort.  How
> much time are we talking about saving here?  Seconds, minutes or hours?
> Would a reviewer script be helpful for some of this?
>
> IMO the Artemis release process seems to be working fine, there are quite a
> few steps to remember as a release manager and it's easy to make mistakes,
> so I would prefer not to change the process unless there's a clear benefit
> that warrants it.
>
> On Wed, Sep 13, 2017 at 10:42 AM, Robbie Gemmell 
> wrote:
>
>> I'm a bit surprised by the negative responses if I'm honest.
>>
>> There isn't really any significant difference in the amount of work
>> for the release manager here. The suggestion mainly just moves the
>> step of populating the dist repo to before the vote rather than after,
>> using the dev area rather than release area. As part of that entirely
>> scriptable action, the checksums could be improved by running some
>> commands, simplifying their eventual use both for testers and for end
>> users to apply them toward the purpose they exist in verifying
>> releases downloaded from the mirrors. Using the dev repo lets folks
>> test what is actually going to be released to the dist mirrors later,
>> much like the nexus staging repo allows testing the precise maven
>> artifacts before they are promoted to release. After the vote there is
>> then less moving parts with the release manger (or someone else if
>> ever needed) just doing "svn cp  " to complete
>> things, much like clicking the release button in Nexus.
>>
>> For what its worth, I based the suggestions on having followed similar
>> process at Qpid for several years as either release manager or a
>> tester. I've release managed a roughly similar number of releases in
>> recent years as all the ActiveMQ components combined have had in that
>> time, and don't find the suggestions in any way a burden, if anything
>> I think they make life easier for everyone including me.
>>
>> Robbie
>>
>> On 12 September 2017 at 22:43, Michael André Pearce
>>  wrote:
>> > I'm -0 on this.
>> >
>> > I'm not against it if people want it, but also I don't see if it adds
>> that much to warrant the effort.
>> >
>> >  e.g. If I had to do the work I prob wouldn't want to have the extra
>> burden. At most I would just update the checksum (though I don't think this
>> is end of the world right now either, I know plenty of org/projects still
>> using md5 hash only still)
>> >
>> > As someone who's been active since the new year and tested now a few
>> releases it hasn't been difficult getting binaries or artifacts and
>> validating them.
>> >
>> >
>> > Cheers
>> > Mike
>> >
>> >
>> >
>> >
>> >
>> > Sent from my iPhone
>> >
>> >> On 12 Sep 2017, at 22:03, Clebert Suconic 
>> wrote:
>> >>
>> >> What is the point on adding extra steps on verifying signatures and
>> hashes?
>> >> Nexus won't let us deploy anything if it has a bad hash?
>> >>
>> >> It seems additional burecractics that could be automated in Nexus. Or
>> >> whatever ever replaces it if you are concerned about the future.
>> >>
>> >> I have been following other projects and their processes are even leaner
>> >> than ours.
>> >>
>> >> On Tue, Sep 12, 2017 at 4:41 PM Clebert Suconic <
>> clebert.suco...@gmail.com>
>> >> wrote:
>> >>
>> >>> -1 to change the process.
>> >>>
>> >>> +1 to add scripts to the reviewer.
>> >>>
>> >>> That is we improve the process of 

[GitHub] activemq-artemis pull request #1533: ARTEMIS-1418 AIO Shutdown on IOError an...

[mtaylor]

GitHub user mtaylor opened a pull request:

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

ARTEMIS-1418 AIO Shutdown on IOError and logging



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

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

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

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


commit 966145392400a59a6e1d72303769229575bc3867
Author: Martyn Taylor 
Date:   2017-09-08T14:00:35Z

ARTEMIS-1418 AIO Shutdown on IOError and logging




---

[GitHub] activemq-artemis pull request #1532: ARTEMIS-1419 OpenWire advisory message ...

[gaohoward]

GitHub user gaohoward opened a pull request:

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

ARTEMIS-1419 OpenWire advisory message never deleted

By default, every openwire connection will create a queue
under the multicast address ActiveMQ.Advisory.TempQueue.
If a openwire client is create temporary queues these queues
will fill up with messages for as long as the associated
openwire connection is alive. It appears these messages
do not get consumed from the queues.

The reason behind is that advisory messages don't require
acknowledgement so the messages stay at the queue.

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

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

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

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


commit 853571229851ee2592aedf61e005b530987943fd
Author: Howard Gao 
Date:   2017-09-13T12:50:56Z

ARTEMIS-1419 OpenWire advisory message never deleted

By default, every openwire connection will create a queue
under the multicast address ActiveMQ.Advisory.TempQueue.
If a openwire client is create temporary queues these queues
will fill up with messages for as long as the associated
openwire connection is alive. It appears these messages
do not get consumed from the queues.

The reason behind is that advisory messages don't require
acknowledgement so the messages stay at the queue.




---

Re: [DISCUSS] release process improvements

[Martyn Taylor]

I don't think the responses were negative.  I don't think the benefit was
made clear and people are questioning whether it's worth the effort.  How
much time are we talking about saving here?  Seconds, minutes or hours?
Would a reviewer script be helpful for some of this?

IMO the Artemis release process seems to be working fine, there are quite a
few steps to remember as a release manager and it's easy to make mistakes,
so I would prefer not to change the process unless there's a clear benefit
that warrants it.

On Wed, Sep 13, 2017 at 10:42 AM, Robbie Gemmell 
wrote:

> I'm a bit surprised by the negative responses if I'm honest.
>
> There isn't really any significant difference in the amount of work
> for the release manager here. The suggestion mainly just moves the
> step of populating the dist repo to before the vote rather than after,
> using the dev area rather than release area. As part of that entirely
> scriptable action, the checksums could be improved by running some
> commands, simplifying their eventual use both for testers and for end
> users to apply them toward the purpose they exist in verifying
> releases downloaded from the mirrors. Using the dev repo lets folks
> test what is actually going to be released to the dist mirrors later,
> much like the nexus staging repo allows testing the precise maven
> artifacts before they are promoted to release. After the vote there is
> then less moving parts with the release manger (or someone else if
> ever needed) just doing "svn cp  " to complete
> things, much like clicking the release button in Nexus.
>
> For what its worth, I based the suggestions on having followed similar
> process at Qpid for several years as either release manager or a
> tester. I've release managed a roughly similar number of releases in
> recent years as all the ActiveMQ components combined have had in that
> time, and don't find the suggestions in any way a burden, if anything
> I think they make life easier for everyone including me.
>
> Robbie
>
> On 12 September 2017 at 22:43, Michael André Pearce
>  wrote:
> > I'm -0 on this.
> >
> > I'm not against it if people want it, but also I don't see if it adds
> that much to warrant the effort.
> >
> >  e.g. If I had to do the work I prob wouldn't want to have the extra
> burden. At most I would just update the checksum (though I don't think this
> is end of the world right now either, I know plenty of org/projects still
> using md5 hash only still)
> >
> > As someone who's been active since the new year and tested now a few
> releases it hasn't been difficult getting binaries or artifacts and
> validating them.
> >
> >
> > Cheers
> > Mike
> >
> >
> >
> >
> >
> > Sent from my iPhone
> >
> >> On 12 Sep 2017, at 22:03, Clebert Suconic 
> wrote:
> >>
> >> What is the point on adding extra steps on verifying signatures and
> hashes?
> >> Nexus won't let us deploy anything if it has a bad hash?
> >>
> >> It seems additional burecractics that could be automated in Nexus. Or
> >> whatever ever replaces it if you are concerned about the future.
> >>
> >> I have been following other projects and their processes are even leaner
> >> than ours.
> >>
> >> On Tue, Sep 12, 2017 at 4:41 PM Clebert Suconic <
> clebert.suco...@gmail.com>
> >> wrote:
> >>
> >>> -1 to change the process.
> >>>
> >>> +1 to add scripts to the reviewer.
> >>>
> >>> That is we improve the process of reviews. But I don't think we need to
> >>> change how this is released.
> >>>
> >>>
> >>>
> >>>
>  On Tue, Sep 12, 2017 at 12:36 PM Daniel Kulp 
> wrote:
> 
>  Just to be clear…
> 
>  This proposal creates more work for the release manager prior to
> starting
>  the vote but in hopes of reducing the work for the reviewers.   It’s
> a bit
>  more than a “mvn release:prepare ; man release:perform”.  Some of the
> extra
>  work can obviously be scripted, but it is still a bit more to do.
> 
>  That said,  script provided to the reviewer could accomplish the same
>  things using the current staging location/setup.
> 
>  Anyway, I’m -0 to the idea.Getting folks to actually be a release
>  manager is hard enough, why make it even more work.Since I
> haven’t been
>  a release manager for an ActiveMQ release in a while, I certainly
> wouldn’t
>  hold up the idea though.
> 
>  Dan
> 
> 
> 
> > On Sep 12, 2017, at 9:49 AM, Robbie Gemmell <
> robbie.gemm...@gmail.com>
>  wrote:
> >
> > Hi folks,
> >
> > I mentioned on the recent Artemis 2.3.0 vote that I had some
> suggested
> > changes for the release process improvements, not just for Artemis
> but
> > for other components too, and would send a mail later.
> >
> > The short version is there are three main things I'd like to suggest
> > as improvements, both for folks testing+voting, and end 

Re: [DISCUSS] release process improvements

[Clebert Suconic]

That's why I asked you the intention.  I had the impression it would be
something to double work what nexus already does on checksums.


Let me see how that works and I will put my feedback here.

On Wed, Sep 13, 2017 at 5:42 AM Robbie Gemmell 
wrote:

> I'm a bit surprised by the negative responses if I'm honest.
>
> There isn't really any significant difference in the amount of work
> for the release manager here. The suggestion mainly just moves the
> step of populating the dist repo to before the vote rather than after,
> using the dev area rather than release area. As part of that entirely
> scriptable action, the checksums could be improved by running some
> commands, simplifying their eventual use both for testers and for end
> users to apply them toward the purpose they exist in verifying
> releases downloaded from the mirrors. Using the dev repo lets folks
> test what is actually going to be released to the dist mirrors later,
> much like the nexus staging repo allows testing the precise maven
> artifacts before they are promoted to release. After the vote there is
> then less moving parts with the release manger (or someone else if
> ever needed) just doing "svn cp  " to complete
> things, much like clicking the release button in Nexus.
>
> For what its worth, I based the suggestions on having followed similar
> process at Qpid for several years as either release manager or a
> tester. I've release managed a roughly similar number of releases in
> recent years as all the ActiveMQ components combined have had in that
> time, and don't find the suggestions in any way a burden, if anything
> I think they make life easier for everyone including me.
>
> Robbie
>
> On 12 September 2017 at 22:43, Michael André Pearce
>  wrote:
> > I'm -0 on this.
> >
> > I'm not against it if people want it, but also I don't see if it adds
> that much to warrant the effort.
> >
> >  e.g. If I had to do the work I prob wouldn't want to have the extra
> burden. At most I would just update the checksum (though I don't think this
> is end of the world right now either, I know plenty of org/projects still
> using md5 hash only still)
> >
> > As someone who's been active since the new year and tested now a few
> releases it hasn't been difficult getting binaries or artifacts and
> validating them.
> >
> >
> > Cheers
> > Mike
> >
> >
> >
> >
> >
> > Sent from my iPhone
> >
> >> On 12 Sep 2017, at 22:03, Clebert Suconic 
> wrote:
> >>
> >> What is the point on adding extra steps on verifying signatures and
> hashes?
> >> Nexus won't let us deploy anything if it has a bad hash?
> >>
> >> It seems additional burecractics that could be automated in Nexus. Or
> >> whatever ever replaces it if you are concerned about the future.
> >>
> >> I have been following other projects and their processes are even leaner
> >> than ours.
> >>
> >> On Tue, Sep 12, 2017 at 4:41 PM Clebert Suconic <
> clebert.suco...@gmail.com>
> >> wrote:
> >>
> >>> -1 to change the process.
> >>>
> >>> +1 to add scripts to the reviewer.
> >>>
> >>> That is we improve the process of reviews. But I don't think we need to
> >>> change how this is released.
> >>>
> >>>
> >>>
> >>>
>  On Tue, Sep 12, 2017 at 12:36 PM Daniel Kulp 
> wrote:
> 
>  Just to be clear…
> 
>  This proposal creates more work for the release manager prior to
> starting
>  the vote but in hopes of reducing the work for the reviewers.   It’s
> a bit
>  more than a “mvn release:prepare ; man release:perform”.  Some of the
> extra
>  work can obviously be scripted, but it is still a bit more to do.
> 
>  That said,  script provided to the reviewer could accomplish the same
>  things using the current staging location/setup.
> 
>  Anyway, I’m -0 to the idea.Getting folks to actually be a release
>  manager is hard enough, why make it even more work.Since I
> haven’t been
>  a release manager for an ActiveMQ release in a while, I certainly
> wouldn’t
>  hold up the idea though.
> 
>  Dan
> 
> 
> 
> > On Sep 12, 2017, at 9:49 AM, Robbie Gemmell <
> robbie.gemm...@gmail.com>
>  wrote:
> >
> > Hi folks,
> >
> > I mentioned on the recent Artemis 2.3.0 vote that I had some
> suggested
> > changes for the release process improvements, not just for Artemis
> but
> > for other components too, and would send a mail later.
> >
> > The short version is there are three main things I'd like to suggest
> > as improvements, both for folks testing+voting, and end users
> > downloading the release later:
> > - Using the dist dev repo for publishing bits for folks to test and
>  vote on.
> > - Providing checksum files in the dist repo which verify more easily
> > with the related tools.
> > - Use SHA512 rather than SHA1 for checksums in the dist repo.
> 

[GitHub] activemq-artemis pull request #1530: ARTEMIS-1417 Failback not working on NF...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1530: ARTEMIS-1417 Failback not working on NF...

[mtaylor]

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

https://github.com/apache/activemq-artemis/pull/1530#discussion_r138584661
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
 ---
@@ -267,25 +293,27 @@ public final SimpleString readNodeId() throws 
ActiveMQIllegalStateException, IOE
   return getNodeId();
}
 
-   protected FileLock tryLock(final int lockPos) throws Exception {
+   protected FileLock tryLock(final long lockPos) throws IOException {
--- End diff --

+1 on this.  Merging.


---

Re: [DISCUSS] release process improvements

[Robbie Gemmell]

I'm a bit surprised by the negative responses if I'm honest.

There isn't really any significant difference in the amount of work
for the release manager here. The suggestion mainly just moves the
step of populating the dist repo to before the vote rather than after,
using the dev area rather than release area. As part of that entirely
scriptable action, the checksums could be improved by running some
commands, simplifying their eventual use both for testers and for end
users to apply them toward the purpose they exist in verifying
releases downloaded from the mirrors. Using the dev repo lets folks
test what is actually going to be released to the dist mirrors later,
much like the nexus staging repo allows testing the precise maven
artifacts before they are promoted to release. After the vote there is
then less moving parts with the release manger (or someone else if
ever needed) just doing "svn cp  " to complete
things, much like clicking the release button in Nexus.

For what its worth, I based the suggestions on having followed similar
process at Qpid for several years as either release manager or a
tester. I've release managed a roughly similar number of releases in
recent years as all the ActiveMQ components combined have had in that
time, and don't find the suggestions in any way a burden, if anything
I think they make life easier for everyone including me.

Robbie

On 12 September 2017 at 22:43, Michael André Pearce
 wrote:
> I'm -0 on this.
>
> I'm not against it if people want it, but also I don't see if it adds that 
> much to warrant the effort.
>
>  e.g. If I had to do the work I prob wouldn't want to have the extra burden. 
> At most I would just update the checksum (though I don't think this is end of 
> the world right now either, I know plenty of org/projects still using md5 
> hash only still)
>
> As someone who's been active since the new year and tested now a few releases 
> it hasn't been difficult getting binaries or artifacts and validating them.
>
>
> Cheers
> Mike
>
>
>
>
>
> Sent from my iPhone
>
>> On 12 Sep 2017, at 22:03, Clebert Suconic  wrote:
>>
>> What is the point on adding extra steps on verifying signatures and hashes?
>> Nexus won't let us deploy anything if it has a bad hash?
>>
>> It seems additional burecractics that could be automated in Nexus. Or
>> whatever ever replaces it if you are concerned about the future.
>>
>> I have been following other projects and their processes are even leaner
>> than ours.
>>
>> On Tue, Sep 12, 2017 at 4:41 PM Clebert Suconic 
>> wrote:
>>
>>> -1 to change the process.
>>>
>>> +1 to add scripts to the reviewer.
>>>
>>> That is we improve the process of reviews. But I don't think we need to
>>> change how this is released.
>>>
>>>
>>>
>>>
 On Tue, Sep 12, 2017 at 12:36 PM Daniel Kulp  wrote:

 Just to be clear…

 This proposal creates more work for the release manager prior to starting
 the vote but in hopes of reducing the work for the reviewers.   It’s a bit
 more than a “mvn release:prepare ; man release:perform”.  Some of the extra
 work can obviously be scripted, but it is still a bit more to do.

 That said,  script provided to the reviewer could accomplish the same
 things using the current staging location/setup.

 Anyway, I’m -0 to the idea.Getting folks to actually be a release
 manager is hard enough, why make it even more work.Since I haven’t been
 a release manager for an ActiveMQ release in a while, I certainly wouldn’t
 hold up the idea though.

 Dan



> On Sep 12, 2017, at 9:49 AM, Robbie Gemmell 
 wrote:
>
> Hi folks,
>
> I mentioned on the recent Artemis 2.3.0 vote that I had some suggested
> changes for the release process improvements, not just for Artemis but
> for other components too, and would send a mail later.
>
> The short version is there are three main things I'd like to suggest
> as improvements, both for folks testing+voting, and end users
> downloading the release later:
> - Using the dist dev repo for publishing bits for folks to test and
 vote on.
> - Providing checksum files in the dist repo which verify more easily
> with the related tools.
> - Use SHA512 rather than SHA1 for checksums in the dist repo.
>
> # Dist dev repo for votes
>
> Currently the ActiveMQ votes for the Java components tend to link to
> the artifacts in the nexus staging repo. I think using the dist dev
> repo (https://dist.apache.org/repos/dist/dev/activemq/) to publish the
> bits under vote would be an improvement. Its easy for folks to grab
> all the files at once, helps ensure that what people test is actually
> what will end up in the dist release repo later, and it simplifies the
> eventual release 

How to implement a custom policy?

[linlinxiao]

1.I wrote a java file extends SimpleDispatchPolicy in the
org.apache.activemq.broker.region.policy package
2.Make the Maven compilation packaged under the parent project

But this custom policy is not generated in the activemq-spring/activemq.xsd
file 
 

Java code:

public class SimpleMqttDispatchPolicy extends SimpleDispatchPolicy {

private static final Log LOG =
LogFactory.getLog(SimpleMqttDispatchPolicy.class);


private static final String P2P_TAG = "P2P";


@Override
public boolean dispatch(MessageReference node, MessageEvaluationContext
msgContext, List consumers)
throws Exception {
boolean flag = false;

ActiveMQDestination desination = 
node.getMessage().getDestination();

// String topicUrl = desination.getQualifiedName();

String topic = desination.getPhysicalName();
String clientId = parseClientId4Topic(topic);
if (null != clientId && !"".equals(clientId)) {
LOG.info("=Client id:" + clientId);
// p2p
flag = p2pProccess(consumers, node, clientId, 
desination, msgContext);
} else {
flag = super.dispatch(node, msgContext, consumers);
}
return flag;
}
}

How the simpleDispatchPolicy in the activemq.xsd file is generated, how do I
customize a policy？
Someone helped me?



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html