You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/07/06 19:13:16 UTC
[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
clebertsuconic opened a new pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213
Queue settings are reset to their default values upon broker.xml reload ONLY.
Regular calls to PostOfficeImpl#updateQueue are no longer affected.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jsmucr commented on a change in pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
jsmucr commented on a change in pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#discussion_r450606876
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -701,7 +701,7 @@ public QueueBinding updateQueue(QueueConfiguration queueConfiguration, boolean f
final SimpleString empty = new SimpleString("");
Filter oldFilter = FilterImpl.createFilter(queue.getFilter() == null ? empty : queue.getFilter().getFilterString());
Filter newFilter = FilterImpl.createFilter(queueConfiguration.getFilterString() == null ? empty : queueConfiguration.getFilterString());
- if ((forceUpdate || newFilter != null) && !Objects.equals(oldFilter, newFilter)) {
+ if ((forceUpdate || newFilter != oldFilter) && !Objects.equals(oldFilter, newFilter)) {
Review comment:
Yes, this is what I originally intended. Thank you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#issuecomment-654547763
@jsmucr Why such semantic change regarding updateQueue? that's a contract break on the API.. and a big semantic change.
As a matter of fact those testRemoteQueueFilter stopped working because of this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jsmucr commented on a change in pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
jsmucr commented on a change in pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#discussion_r450606562
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java
##########
@@ -514,8 +514,8 @@ public void testRemoveQueueFilter() throws Exception {
producer.send(m);
- assertNotNull(consumer1.receiveImmediate());
- assertNotNull(consumer2.receiveImmediate());
+ assertNotNull(consumer1.receive(1000));
Review comment:
I'll keep that in mind. Thank you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#issuecomment-654479243
4 tests failures on this PR:
org.apache.activemq.artemis.tests.integration.management.ActiveMQServerControlTest.testRemoveQueueFilter[legacyCreateQueue=true]
org.apache.activemq.artemis.tests.integration.management.ActiveMQServerControlTest.testRemoveQueueFilter[legacyCreateQueue=false]
org.apache.activemq.artemis.tests.integration.management.ActiveMQServerControlUsingCoreTest.testRemoveQueueFilter[legacyCreateQueue=true]
org.apache.activemq.artemis.tests.integration.management.ActiveMQServerControlUsingCoreTest.testRemoveQueueFilter[legacyCreateQueue=false]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#discussion_r450814008
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -701,7 +701,7 @@ public QueueBinding updateQueue(QueueConfiguration queueConfiguration, boolean f
final SimpleString empty = new SimpleString("");
Filter oldFilter = FilterImpl.createFilter(queue.getFilter() == null ? empty : queue.getFilter().getFilterString());
Filter newFilter = FilterImpl.createFilter(queueConfiguration.getFilterString() == null ? empty : queueConfiguration.getFilterString());
- if ((forceUpdate || newFilter != null) && !Objects.equals(oldFilter, newFilter)) {
+ if ((forceUpdate || newFilter != oldFilter) && !Objects.equals(oldFilter, newFilter)) {
Review comment:
@jsmucr merged, thank you
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#discussion_r450566421
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -701,7 +701,7 @@ public QueueBinding updateQueue(QueueConfiguration queueConfiguration, boolean f
final SimpleString empty = new SimpleString("");
Filter oldFilter = FilterImpl.createFilter(queue.getFilter() == null ? empty : queue.getFilter().getFilterString());
Filter newFilter = FilterImpl.createFilter(queueConfiguration.getFilterString() == null ? empty : queueConfiguration.getFilterString());
- if ((forceUpdate || newFilter != null) && !Objects.equals(oldFilter, newFilter)) {
+ if ((forceUpdate || newFilter != oldFilter) && !Objects.equals(oldFilter, newFilter)) {
Review comment:
@jsmucr here is the change that should fix the test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] asfgit merged pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#issuecomment-654416771
this supersedes #3212
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#discussion_r450566684
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java
##########
@@ -514,8 +514,8 @@ public void testRemoveQueueFilter() throws Exception {
producer.send(m);
- assertNotNull(consumer1.receiveImmediate());
- assertNotNull(consumer2.receiveImmediate());
+ assertNotNull(consumer1.receive(1000));
Review comment:
@jsmucr: just a note here. if you intended to assertNotNull, do a receive with a timeout.
if you assertNull, do a receiveImmediate (or receiveNoWait in JMS terms).. or a very short timeout.
assertNotNull(receiveImmediate) is a bit fragile and it may break a test intermittently.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#issuecomment-654549647
@jsmucr: never min, when I read this:
"Regular calls to PostOfficeImpl#updateQueue are no longer affected."
I thought it meant that updateQueue wouldn't no longer have any effect... I read it again.. and I see the real meaning.
I am adding a new commit to fix the offended tests.. if you could please review it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic edited a comment on pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic edited a comment on pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#issuecomment-654549647
@jsmucr: never mind, when I read this:
"Regular calls to PostOfficeImpl#updateQueue are no longer affected."
I thought it meant that updateQueue would no longer have any effect... I read it again.. and I see the real meaning.
I am adding a new commit to fix the offended tests.. if you could please review it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3213: ARTEMIS-2797 Fixing redeploy mechanism
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3213:
URL: https://github.com/apache/activemq-artemis/pull/3213#issuecomment-654417294
I'm running a full set of tests.. it will take me 2.5 hours to complete it. if all good this will be merged. (unless someone complains about it in the next 2 and 1/2 hours).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org