[VOTE] Apache ActiveMQ 5.15.12 release

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
Hi everyone,

I'm submitting ActiveMQ 5.15.12 release to your vote.

This release includes dependency updates (especially for CVE/Security
fixes), important improvements on JDBC persistence adapter, other improvements and fixes.

Please take a look on the Release Notes for details:

https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>

The Maven staging repository is:
https://repository.apache.org/content/repositories/orgapacheactivemq-1204

The dist staging repository is:
https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>

Git tag:
activemq-5.15.12

Website PR:
https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>

Please vote to approve this release:

[ ] +1 Approve the release
[ ] -1 Don't approve the release (please provide specific comments)

This vote will be open for at least 72 hours.

Thanks !
Regards
JB
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jgoodyear
+1 (Non-binding)

Cheers,
Jamie

On Fri, Mar 6, 2020 at 3:45 AM Jean-Baptiste Onofre <[hidden email]> wrote:

>
> Hi everyone,
>
> I'm submitting ActiveMQ 5.15.12 release to your vote.
>
> This release includes dependency updates (especially for CVE/Security
> fixes), important improvements on JDBC persistence adapter, other improvements and fixes.
>
> Please take a look on the Release Notes for details:
>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>
>
> The Maven staging repository is:
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>
> The dist staging repository is:
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>
> Git tag:
> activemq-5.15.12
>
> Website PR:
> https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>
>
> Please vote to approve this release:
>
> [ ] +1 Approve the release
> [ ] -1 Don't approve the release (please provide specific comments)
>
> This vote will be open for at least 72 hours.
>
> Thanks !
> Regards
> JB
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

Francois Papon
In reply to this post by jbonofre
+1 (non-binding)

Thanks!

regards,

François
[hidden email]

Le 06/03/2020 à 08:15, Jean-Baptiste Onofre a écrit :

> Hi everyone,
>
> I'm submitting ActiveMQ 5.15.12 release to your vote.
>
> This release includes dependency updates (especially for CVE/Security
> fixes), important improvements on JDBC persistence adapter, other improvements and fixes.
>
> Please take a look on the Release Notes for details:
>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>
>
> The Maven staging repository is:
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>
> The dist staging repository is:
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>
> Git tag:
> activemq-5.15.12
>
> Website PR:
> https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>
>
> Please vote to approve this release:
>
> [ ] +1 Approve the release
> [ ] -1 Don't approve the release (please provide specific comments)
>
> This vote will be open for at least 72 hours.
>
> Thanks !
> Regards
> JB
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jgenender
In reply to this post by jbonofre
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
In reply to this post by jbonofre
Here"s my own +1 (binding)

Regards
JB

> Le 6 mars 2020 à 08:15, Jean-Baptiste Onofre <[hidden email]> a écrit :
>
> Hi everyone,
>
> I'm submitting ActiveMQ 5.15.12 release to your vote.
>
> This release includes dependency updates (especially for CVE/Security
> fixes), important improvements on JDBC persistence adapter, other improvements and fixes.
>
> Please take a look on the Release Notes for details:
>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>
>
> The Maven staging repository is:
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>
> The dist staging repository is:
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>
> Git tag:
> activemq-5.15.12
>
> Website PR:
> https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>
>
> Please vote to approve this release:
>
> [ ] +1 Approve the release
> [ ] -1 Don't approve the release (please provide specific comments)
>
> This vote will be open for at least 72 hours.
>
> Thanks !
> Regards
> JB

Reply | Threaded
Open this post in threaded view
|

Re:[VOTE] Apache ActiveMQ 5.15.12 release

KimmKing-2
In reply to this post by jbonofre
+1



At 2020-03-06 15:15:10, "Jean-Baptiste Onofre" <[hidden email]> wrote:

>Hi everyone,
>
>I'm submitting ActiveMQ 5.15.12 release to your vote.
>
>This release includes dependency updates (especially for CVE/Security
>fixes), important improvements on JDBC persistence adapter, other improvements and fixes.
>
>Please take a look on the Release Notes for details:
>
>https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>
>
>The Maven staging repository is:
>https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>
>The dist staging repository is:
>https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>
>Git tag:
>activemq-5.15.12
>
>Website PR:
>https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>
>
>Please vote to approve this release:
>
>[ ] +1 Approve the release
>[ ] -1 Don't approve the release (please provide specific comments)
>
>This vote will be open for at least 72 hours.
>
>Thanks !
>Regards
>JB
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

dkulp
In reply to this post by jbonofre
+1

Dan


> On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]> wrote:
>
> Hi everyone,
>
> I'm submitting ActiveMQ 5.15.12 release to your vote.
>
> This release includes dependency updates (especially for CVE/Security
> fixes), important improvements on JDBC persistence adapter, other improvements and fixes.
>
> Please take a look on the Release Notes for details:
>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>
>
> The Maven staging repository is:
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>
> The dist staging repository is:
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>
> Git tag:
> activemq-5.15.12
>
> Website PR:
> https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>
>
> Please vote to approve this release:
>
> [ ] +1 Approve the release
> [ ] -1 Don't approve the release (please provide specific comments)
>
> This vote will be open for at least 72 hours.
>
> Thanks !
> Regards
> JB

--
Daniel Kulp
[hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <http://dankulp.com/blog>
Talend Community Coder - http://talend.com <http://coders.talend.com/>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

christopher.l.shannon
-1, there are multiple STOMP regressions here

1) In some of my tests that verify error handling works properly, I'm
getting a NPE here that causes my build to fail:
https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261

The root cause is there's no guarantee there will be a linked exception and
in this case the linked exception is null leading to an NPE

2) It makes no sense to swallow the exception here and not propagate it
up:
https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67

Every other protocol throws the IOException and this no longer does.
Furthermore, it makes no sense to treat STOMP different than other
protocols.  I don't see why we need to log at all when the IOException
should propagate up and then be logged elsewhere.  If it's not being logged
properly then something else up the chain (TcpTransport or whatever) should
probably be catching the exception and logging.

On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[hidden email]> wrote:

> +1
>
> Dan
>
>
> > On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]>
> wrote:
> >
> > Hi everyone,
> >
> > I'm submitting ActiveMQ 5.15.12 release to your vote.
> >
> > This release includes dependency updates (especially for CVE/Security
> > fixes), important improvements on JDBC persistence adapter, other
> improvements and fixes.
> >
> > Please take a look on the Release Notes for details:
> >
> >
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
> <
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
> >
> >
> > The Maven staging repository is:
> >
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
> >
> > The dist staging repository is:
> > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
> >
> > Git tag:
> > activemq-5.15.12
> >
> > Website PR:
> > https://github.com/apache/activemq-website/pull/28 <
> https://github.com/apache/activemq-website/pull/28>
> >
> > Please vote to approve this release:
> >
> > [ ] +1 Approve the release
> > [ ] -1 Don't approve the release (please provide specific comments)
> >
> > This vote will be open for at least 72 hours.
> >
> > Thanks !
> > Regards
> > JB
>
> --
> Daniel Kulp
> [hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <
> http://dankulp.com/blog>
> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

christopher.l.shannon
Furthermore the regressions in STOMP can be seen here in Jenkins:
https://builds.apache.org/view/A/view/ActiveMQ/job/ActiveMQ-Java8/lastCompletedBuild/testReport/

Also looking at another fix that was just merged a couple days ago:  I'm
not convinced that this is correct:
https://issues.apache.org/jira/projects/AMQ/issues/AMQ-7314  There's no
test here to demonstrate the fix and it seems like it might be wrong to
mark a sequenceId less than the lastDeliveredSequenceId to be the
lastDeliveredRef.  A change like this needs to demonstrate the issue and
fix works

My recommendation when doing future releases is to freeze the release
earlier in the process and to not try and merge in a ton of stuff in the
last couple days before a release.  Last minute changes can be pushed to
the next release unless critical/blockers.  Merging at the last second
before a release doesn't give much time for others to review the changes
and verify they make sense or to even give CI a chance to run and be
verified as shown here.

On Mon, Mar 9, 2020 at 8:49 AM Christopher Shannon <
[hidden email]> wrote:

> -1, there are multiple STOMP regressions here
>
> 1) In some of my tests that verify error handling works properly, I'm
> getting a NPE here that causes my build to fail:
> https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261
>
> The root cause is there's no guarantee there will be a linked exception
> and in this case the linked exception is null leading to an NPE
>
> 2) It makes no sense to swallow the exception here and not propagate it
> up:
> https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67
>
> Every other protocol throws the IOException and this no longer does.
> Furthermore, it makes no sense to treat STOMP different than other
> protocols.  I don't see why we need to log at all when the IOException
> should propagate up and then be logged elsewhere.  If it's not being logged
> properly then something else up the chain (TcpTransport or whatever) should
> probably be catching the exception and logging.
>
> On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[hidden email]> wrote:
>
>> +1
>>
>> Dan
>>
>>
>> > On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]>
>> wrote:
>> >
>> > Hi everyone,
>> >
>> > I'm submitting ActiveMQ 5.15.12 release to your vote.
>> >
>> > This release includes dependency updates (especially for CVE/Security
>> > fixes), important improvements on JDBC persistence adapter, other
>> improvements and fixes.
>> >
>> > Please take a look on the Release Notes for details:
>> >
>> >
>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>> <
>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>> >
>> >
>> > The Maven staging repository is:
>> >
>> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>> >
>> > The dist staging repository is:
>> > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>> >
>> > Git tag:
>> > activemq-5.15.12
>> >
>> > Website PR:
>> > https://github.com/apache/activemq-website/pull/28 <
>> https://github.com/apache/activemq-website/pull/28>
>> >
>> > Please vote to approve this release:
>> >
>> > [ ] +1 Approve the release
>> > [ ] -1 Don't approve the release (please provide specific comments)
>> >
>> > This vote will be open for at least 72 hours.
>> >
>> > Thanks !
>> > Regards
>> > JB
>>
>> --
>> Daniel Kulp
>> [hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <
>> http://dankulp.com/blog>
>> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

christopher.l.shannon
Also, there seem to be JDBC regressions as well as shown by the link to
CI.  The LeaseLockerDatabaseTest is now failing.

On Mon, Mar 9, 2020 at 9:15 AM Christopher Shannon <
[hidden email]> wrote:

> Furthermore the regressions in STOMP can be seen here in Jenkins:
> https://builds.apache.org/view/A/view/ActiveMQ/job/ActiveMQ-Java8/lastCompletedBuild/testReport/
>
> Also looking at another fix that was just merged a couple days ago:  I'm
> not convinced that this is correct:
> https://issues.apache.org/jira/projects/AMQ/issues/AMQ-7314  There's no
> test here to demonstrate the fix and it seems like it might be wrong to
> mark a sequenceId less than the lastDeliveredSequenceId to be the
> lastDeliveredRef.  A change like this needs to demonstrate the issue and
> fix works
>
> My recommendation when doing future releases is to freeze the release
> earlier in the process and to not try and merge in a ton of stuff in the
> last couple days before a release.  Last minute changes can be pushed to
> the next release unless critical/blockers.  Merging at the last second
> before a release doesn't give much time for others to review the changes
> and verify they make sense or to even give CI a chance to run and be
> verified as shown here.
>
> On Mon, Mar 9, 2020 at 8:49 AM Christopher Shannon <
> [hidden email]> wrote:
>
>> -1, there are multiple STOMP regressions here
>>
>> 1) In some of my tests that verify error handling works properly, I'm
>> getting a NPE here that causes my build to fail:
>> https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261
>>
>> The root cause is there's no guarantee there will be a linked exception
>> and in this case the linked exception is null leading to an NPE
>>
>> 2) It makes no sense to swallow the exception here and not propagate it
>> up:
>> https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67
>>
>> Every other protocol throws the IOException and this no longer does.
>> Furthermore, it makes no sense to treat STOMP different than other
>> protocols.  I don't see why we need to log at all when the IOException
>> should propagate up and then be logged elsewhere.  If it's not being logged
>> properly then something else up the chain (TcpTransport or whatever) should
>> probably be catching the exception and logging.
>>
>> On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[hidden email]> wrote:
>>
>>> +1
>>>
>>> Dan
>>>
>>>
>>> > On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]>
>>> wrote:
>>> >
>>> > Hi everyone,
>>> >
>>> > I'm submitting ActiveMQ 5.15.12 release to your vote.
>>> >
>>> > This release includes dependency updates (especially for CVE/Security
>>> > fixes), important improvements on JDBC persistence adapter, other
>>> improvements and fixes.
>>> >
>>> > Please take a look on the Release Notes for details:
>>> >
>>> >
>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>> <
>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>> >
>>> >
>>> > The Maven staging repository is:
>>> >
>>> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>>> >
>>> > The dist staging repository is:
>>> > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>>> >
>>> > Git tag:
>>> > activemq-5.15.12
>>> >
>>> > Website PR:
>>> > https://github.com/apache/activemq-website/pull/28 <
>>> https://github.com/apache/activemq-website/pull/28>
>>> >
>>> > Please vote to approve this release:
>>> >
>>> > [ ] +1 Approve the release
>>> > [ ] -1 Don't approve the release (please provide specific comments)
>>> >
>>> > This vote will be open for at least 72 hours.
>>> >
>>> > Thanks !
>>> > Regards
>>> > JB
>>>
>>> --
>>> Daniel Kulp
>>> [hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <
>>> http://dankulp.com/blog>
>>> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
In reply to this post by christopher.l.shannon
Hi Chris,

Thanks for your vote and feedback.

About 1, you are right I gonna test to avoid NPE.

About 2, I fixed a jira from users. And actually the exception is not fully
swallowed: it?s logged single line. I didn?t want to change globally but I
can if you think it makes more sense. However I had a long discussion with
one user in particular who reported the jira (it?s a very large activemq
user in Europe) and he has good arguments. So I fixed accordingly.

I will cancel vote to fix 1 but 2 looks good to me.

Regards
JB



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

christopher.l.shannon
For number 2 you are logging and then swallowing the error and not
propagating it up.  If you want to log it there you at least need to throw
the exception back up like every other protocol so it's handled
consistently.  As I said it makes no sense to treat that differently than
any other protocol.  The initializedStreams() method is called be all
NIOSSLTransport protocols and the others throw the exception.

 So number 2 is still a -1 for me the way it currently it is without
throwing the exception.

On Mon, Mar 9, 2020 at 9:38 AM jbonofre <[hidden email]> wrote:

> Hi Chris,
>
> Thanks for your vote and feedback.
>
> About 1, you are right I gonna test to avoid NPE.
>
> About 2, I fixed a jira from users. And actually the exception is not fully
> swallowed: it?s logged single line. I didn?t want to change globally but I
> can if you think it makes more sense. However I had a long discussion with
> one user in particular who reported the jira (it?s a very large activemq
> user in Europe) and he has good arguments. So I fixed accordingly.
>
> I will cancel vote to fix 1 but 2 looks good to me.
>
> Regards
> JB
>
>
>
> --
> Sent from:
> http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
OK, I got your point.

Just as reminder, the original Jira is AMQ-7301. The users are complaining to have a full stack trace in the log (useless). That’s why I changed.

Let me reopen the Jira and move forward with a satisfying solution for you.

Thanks,
Regards
JB

> Le 9 mars 2020 à 15:20, Christopher Shannon <[hidden email]> a écrit :
>
> For number 2 you are logging and then swallowing the error and not
> propagating it up.  If you want to log it there you at least need to throw
> the exception back up like every other protocol so it's handled
> consistently.  As I said it makes no sense to treat that differently than
> any other protocol.  The initializedStreams() method is called be all
> NIOSSLTransport protocols and the others throw the exception.
>
> So number 2 is still a -1 for me the way it currently it is without
> throwing the exception.
>
> On Mon, Mar 9, 2020 at 9:38 AM jbonofre <[hidden email]> wrote:
>
>> Hi Chris,
>>
>> Thanks for your vote and feedback.
>>
>> About 1, you are right I gonna test to avoid NPE.
>>
>> About 2, I fixed a jira from users. And actually the exception is not fully
>> swallowed: it?s logged single line. I didn?t want to change globally but I
>> can if you think it makes more sense. However I had a long discussion with
>> one user in particular who reported the jira (it?s a very large activemq
>> user in Europe) and he has good arguments. So I fixed accordingly.
>>
>> I will cancel vote to fix 1 but 2 looks good to me.
>>
>> Regards
>> JB
>>
>>
>>
>> --
>> Sent from:
>> http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
>>

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
In reply to this post by christopher.l.shannon
That’s really weird as Jenkins was green on the PR and the build worked when I did the release.

Let me do a new full build.

Thanks,
Regards
JB

> Le 9 mars 2020 à 14:19, Christopher Shannon <[hidden email]> a écrit :
>
> Also, there seem to be JDBC regressions as well as shown by the link to
> CI.  The LeaseLockerDatabaseTest is now failing.
>
> On Mon, Mar 9, 2020 at 9:15 AM Christopher Shannon <
> [hidden email]> wrote:
>
>> Furthermore the regressions in STOMP can be seen here in Jenkins:
>> https://builds.apache.org/view/A/view/ActiveMQ/job/ActiveMQ-Java8/lastCompletedBuild/testReport/
>>
>> Also looking at another fix that was just merged a couple days ago:  I'm
>> not convinced that this is correct:
>> https://issues.apache.org/jira/projects/AMQ/issues/AMQ-7314  There's no
>> test here to demonstrate the fix and it seems like it might be wrong to
>> mark a sequenceId less than the lastDeliveredSequenceId to be the
>> lastDeliveredRef.  A change like this needs to demonstrate the issue and
>> fix works
>>
>> My recommendation when doing future releases is to freeze the release
>> earlier in the process and to not try and merge in a ton of stuff in the
>> last couple days before a release.  Last minute changes can be pushed to
>> the next release unless critical/blockers.  Merging at the last second
>> before a release doesn't give much time for others to review the changes
>> and verify they make sense or to even give CI a chance to run and be
>> verified as shown here.
>>
>> On Mon, Mar 9, 2020 at 8:49 AM Christopher Shannon <
>> [hidden email]> wrote:
>>
>>> -1, there are multiple STOMP regressions here
>>>
>>> 1) In some of my tests that verify error handling works properly, I'm
>>> getting a NPE here that causes my build to fail:
>>> https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261
>>>
>>> The root cause is there's no guarantee there will be a linked exception
>>> and in this case the linked exception is null leading to an NPE
>>>
>>> 2) It makes no sense to swallow the exception here and not propagate it
>>> up:
>>> https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67
>>>
>>> Every other protocol throws the IOException and this no longer does.
>>> Furthermore, it makes no sense to treat STOMP different than other
>>> protocols.  I don't see why we need to log at all when the IOException
>>> should propagate up and then be logged elsewhere.  If it's not being logged
>>> properly then something else up the chain (TcpTransport or whatever) should
>>> probably be catching the exception and logging.
>>>
>>> On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[hidden email]> wrote:
>>>
>>>> +1
>>>>
>>>> Dan
>>>>
>>>>
>>>>> On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]>
>>>> wrote:
>>>>>
>>>>> Hi everyone,
>>>>>
>>>>> I'm submitting ActiveMQ 5.15.12 release to your vote.
>>>>>
>>>>> This release includes dependency updates (especially for CVE/Security
>>>>> fixes), important improvements on JDBC persistence adapter, other
>>>> improvements and fixes.
>>>>>
>>>>> Please take a look on the Release Notes for details:
>>>>>
>>>>>
>>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>>> <
>>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>>>>
>>>>>
>>>>> The Maven staging repository is:
>>>>>
>>>> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>>>>>
>>>>> The dist staging repository is:
>>>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
>>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>>>>>
>>>>> Git tag:
>>>>> activemq-5.15.12
>>>>>
>>>>> Website PR:
>>>>> https://github.com/apache/activemq-website/pull/28 <
>>>> https://github.com/apache/activemq-website/pull/28>
>>>>>
>>>>> Please vote to approve this release:
>>>>>
>>>>> [ ] +1 Approve the release
>>>>> [ ] -1 Don't approve the release (please provide specific comments)
>>>>>
>>>>> This vote will be open for at least 72 hours.
>>>>>
>>>>> Thanks !
>>>>> Regards
>>>>> JB
>>>>
>>>> --
>>>> Daniel Kulp
>>>> [hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <
>>>> http://dankulp.com/blog>
>>>> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
In reply to this post by christopher.l.shannon
I agree. I created PRs to have build running on Jenkins (and be sure it’s green before merging).

If you want to be reviewer on some, please let me know.

Thanks
Regards
JB

> Le 9 mars 2020 à 14:15, Christopher Shannon <[hidden email]> a écrit :
>
> Furthermore the regressions in STOMP can be seen here in Jenkins:
> https://builds.apache.org/view/A/view/ActiveMQ/job/ActiveMQ-Java8/lastCompletedBuild/testReport/
>
> Also looking at another fix that was just merged a couple days ago:  I'm
> not convinced that this is correct:
> https://issues.apache.org/jira/projects/AMQ/issues/AMQ-7314  There's no
> test here to demonstrate the fix and it seems like it might be wrong to
> mark a sequenceId less than the lastDeliveredSequenceId to be the
> lastDeliveredRef.  A change like this needs to demonstrate the issue and
> fix works
>
> My recommendation when doing future releases is to freeze the release
> earlier in the process and to not try and merge in a ton of stuff in the
> last couple days before a release.  Last minute changes can be pushed to
> the next release unless critical/blockers.  Merging at the last second
> before a release doesn't give much time for others to review the changes
> and verify they make sense or to even give CI a chance to run and be
> verified as shown here.
>
> On Mon, Mar 9, 2020 at 8:49 AM Christopher Shannon <
> [hidden email]> wrote:
>
>> -1, there are multiple STOMP regressions here
>>
>> 1) In some of my tests that verify error handling works properly, I'm
>> getting a NPE here that causes my build to fail:
>> https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261
>>
>> The root cause is there's no guarantee there will be a linked exception
>> and in this case the linked exception is null leading to an NPE
>>
>> 2) It makes no sense to swallow the exception here and not propagate it
>> up:
>> https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67
>>
>> Every other protocol throws the IOException and this no longer does.
>> Furthermore, it makes no sense to treat STOMP different than other
>> protocols.  I don't see why we need to log at all when the IOException
>> should propagate up and then be logged elsewhere.  If it's not being logged
>> properly then something else up the chain (TcpTransport or whatever) should
>> probably be catching the exception and logging.
>>
>> On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[hidden email]> wrote:
>>
>>> +1
>>>
>>> Dan
>>>
>>>
>>>> On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]>
>>> wrote:
>>>>
>>>> Hi everyone,
>>>>
>>>> I'm submitting ActiveMQ 5.15.12 release to your vote.
>>>>
>>>> This release includes dependency updates (especially for CVE/Security
>>>> fixes), important improvements on JDBC persistence adapter, other
>>> improvements and fixes.
>>>>
>>>> Please take a look on the Release Notes for details:
>>>>
>>>>
>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>> <
>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>>>
>>>>
>>>> The Maven staging repository is:
>>>>
>>> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>>>>
>>>> The dist staging repository is:
>>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>>>>
>>>> Git tag:
>>>> activemq-5.15.12
>>>>
>>>> Website PR:
>>>> https://github.com/apache/activemq-website/pull/28 <
>>> https://github.com/apache/activemq-website/pull/28>
>>>>
>>>> Please vote to approve this release:
>>>>
>>>> [ ] +1 Approve the release
>>>> [ ] -1 Don't approve the release (please provide specific comments)
>>>>
>>>> This vote will be open for at least 72 hours.
>>>>
>>>> Thanks !
>>>> Regards
>>>> JB
>>>
>>> --
>>> Daniel Kulp
>>> [hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <
>>> http://dankulp.com/blog>
>>> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

[CANCEL][VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
In reply to this post by jbonofre
Hi everyone,

As you probably saw, Chris detected three issues with the release.

So, I cancel this release in order to have cleaner fixes.

Sorry for the inconvenience, I will submit a 5.15.12 take 2 release to vote asap.

Thanks all for your review, especially Chris for the tests and investigations.

Regards
JB

> Le 6 mars 2020 à 08:15, Jean-Baptiste Onofre <[hidden email]> a écrit :
>
> Hi everyone,
>
> I'm submitting ActiveMQ 5.15.12 release to your vote.
>
> This release includes dependency updates (especially for CVE/Security
> fixes), important improvements on JDBC persistence adapter, other improvements and fixes.
>
> Please take a look on the Release Notes for details:
>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500>
>
> The Maven staging repository is:
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>
> The dist staging repository is:
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>
> Git tag:
> activemq-5.15.12
>
> Website PR:
> https://github.com/apache/activemq-website/pull/28 <https://github.com/apache/activemq-website/pull/28>
>
> Please vote to approve this release:
>
> [ ] +1 Approve the release
> [ ] -1 Don't approve the release (please provide specific comments)
>
> This vote will be open for at least 72 hours.
>
> Thanks !
> Regards
> JB

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Apache ActiveMQ 5.15.12 release

jbonofre
In reply to this post by christopher.l.shannon
Hi,

FYI, the lease lock test issue and several other test failures have been introduced by this commit:

b999e2b7787f8b9bd556456b1e6ebf4cbfc0881e is the first bad commit
commit b999e2b7787f8b9bd556456b1e6ebf4cbfc0881e
Author: gtully <[hidden email]>
Date:   Wed Feb 19 13:01:25 2020 +0000

    AMQ-7291 - allow setting properties after clearProperties for BytesMessage, closes #420
   
    (cherry picked from commit 503416a00167e2910630512426df77ecc32492a2)

:040000 040000 e8a2513be5d12a757efd33013413188fbbec1386 0bb23cb72920e96b5340f9d1cd2e253adcc40155 M      activemq-client
:040000 040000 fa1096d00b235f3b4b0ba9ca8e4b03add02edb87 147385f213814ca295fe89f888dccd48a6f6bc06 M      activemq-unit-tests

I have reopen AMQ-7291 to revert the commit and move forward on the release. We will do a better fix for 5.15.13.

Regards
JB

> Le 9 mars 2020 à 14:19, Christopher Shannon <[hidden email]> a écrit :
>
> Also, there seem to be JDBC regressions as well as shown by the link to
> CI.  The LeaseLockerDatabaseTest is now failing.
>
> On Mon, Mar 9, 2020 at 9:15 AM Christopher Shannon <
> [hidden email]> wrote:
>
>> Furthermore the regressions in STOMP can be seen here in Jenkins:
>> https://builds.apache.org/view/A/view/ActiveMQ/job/ActiveMQ-Java8/lastCompletedBuild/testReport/
>>
>> Also looking at another fix that was just merged a couple days ago:  I'm
>> not convinced that this is correct:
>> https://issues.apache.org/jira/projects/AMQ/issues/AMQ-7314  There's no
>> test here to demonstrate the fix and it seems like it might be wrong to
>> mark a sequenceId less than the lastDeliveredSequenceId to be the
>> lastDeliveredRef.  A change like this needs to demonstrate the issue and
>> fix works
>>
>> My recommendation when doing future releases is to freeze the release
>> earlier in the process and to not try and merge in a ton of stuff in the
>> last couple days before a release.  Last minute changes can be pushed to
>> the next release unless critical/blockers.  Merging at the last second
>> before a release doesn't give much time for others to review the changes
>> and verify they make sense or to even give CI a chance to run and be
>> verified as shown here.
>>
>> On Mon, Mar 9, 2020 at 8:49 AM Christopher Shannon <
>> [hidden email]> wrote:
>>
>>> -1, there are multiple STOMP regressions here
>>>
>>> 1) In some of my tests that verify error handling works properly, I'm
>>> getting a NPE here that causes my build to fail:
>>> https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261
>>>
>>> The root cause is there's no guarantee there will be a linked exception
>>> and in this case the linked exception is null leading to an NPE
>>>
>>> 2) It makes no sense to swallow the exception here and not propagate it
>>> up:
>>> https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67
>>>
>>> Every other protocol throws the IOException and this no longer does.
>>> Furthermore, it makes no sense to treat STOMP different than other
>>> protocols.  I don't see why we need to log at all when the IOException
>>> should propagate up and then be logged elsewhere.  If it's not being logged
>>> properly then something else up the chain (TcpTransport or whatever) should
>>> probably be catching the exception and logging.
>>>
>>> On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[hidden email]> wrote:
>>>
>>>> +1
>>>>
>>>> Dan
>>>>
>>>>
>>>>> On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[hidden email]>
>>>> wrote:
>>>>>
>>>>> Hi everyone,
>>>>>
>>>>> I'm submitting ActiveMQ 5.15.12 release to your vote.
>>>>>
>>>>> This release includes dependency updates (especially for CVE/Security
>>>>> fixes), important improvements on JDBC persistence adapter, other
>>>> improvements and fixes.
>>>>>
>>>>> Please take a look on the Release Notes for details:
>>>>>
>>>>>
>>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>>> <
>>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
>>>>>
>>>>>
>>>>> The Maven staging repository is:
>>>>>
>>>> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
>>>>>
>>>>> The dist staging repository is:
>>>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
>>>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
>>>>>
>>>>> Git tag:
>>>>> activemq-5.15.12
>>>>>
>>>>> Website PR:
>>>>> https://github.com/apache/activemq-website/pull/28 <
>>>> https://github.com/apache/activemq-website/pull/28>
>>>>>
>>>>> Please vote to approve this release:
>>>>>
>>>>> [ ] +1 Approve the release
>>>>> [ ] -1 Don't approve the release (please provide specific comments)
>>>>>
>>>>> This vote will be open for at least 72 hours.
>>>>>
>>>>> Thanks !
>>>>> Regards
>>>>> JB
>>>>
>>>> --
>>>> Daniel Kulp
>>>> [hidden email] <mailto:[hidden email]> - http://dankulp.com/blog <
>>>> http://dankulp.com/blog>
>>>> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>>>>
>>>