[GitHub] [activemq] jbonofre merged pull request #389: AMQ-7275 - Update Commons BeanUtils
jbonofre merged pull request #389: AMQ-7275 - Update Commons BeanUtils URL: https://github.com/apache/activemq/pull/389 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on issue #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE
wy96f commented on issue #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE URL: https://github.com/apache/activemq-artemis/pull/2791#issuecomment-522204295 @jbertram will re-work this based on comments. After that will rebase and push :) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool
wy96f commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool URL: https://github.com/apache/activemq-artemis/pull/2726#discussion_r314930518 ## File path: artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplTest.java ## @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.impl; + +import java.lang.ref.WeakReference; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.junit.Assert; +import org.junit.Test; + +public class ActiveMQServerImplTest extends ActiveMQTestBase { + + @Test + public void testScheduledPoolGC() throws Exception { + ActiveMQServer server = createServer(false); + + server.start(); + + Runnable scheduledRunnable = new Runnable() { + @Override + public void run() { +Assert.fail(); + } + }; + WeakReference scheduledRunnableRef = new WeakReference<>(scheduledRunnable); + + ScheduledExecutorService scheduledPool = server.getScheduledPool(); + ScheduledFuture scheduledFuture = scheduledPool.schedule(scheduledRunnable, 5000, TimeUnit.MILLISECONDS); + + Assert.assertFalse(scheduledFuture.isCancelled()); + Assert.assertTrue(scheduledFuture.cancel(true)); + Assert.assertTrue(scheduledFuture.isCancelled()); + + Assert.assertNotEquals(null, scheduledRunnableRef.get()); Review comment: you are right, i missed that :) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE
michaelandrepearce commented on a change in pull request #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE URL: https://github.com/apache/activemq-artemis/pull/2791#discussion_r314877470 ## File path: artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java ## @@ -257,6 +257,26 @@ public void testParsingDefaultServerConfigWithENCMaskedPwd() throws Exception { assertEquals("helloworld", bconfig.getPassword()); } + @Test + public void testParsingOverflowPageSize() throws Exception { + FileConfigurationParser parser = new FileConfigurationParser(); Review comment: Point more so was i assume the same issue may lay in other persistence such as journal etc. As alot of it started from a common base. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE
michaelandrepearce commented on a change in pull request #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE URL: https://github.com/apache/activemq-artemis/pull/2791#discussion_r314877470 ## File path: artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java ## @@ -257,6 +257,26 @@ public void testParsingDefaultServerConfigWithENCMaskedPwd() throws Exception { assertEquals("helloworld", bconfig.getPassword()); } + @Test + public void testParsingOverflowPageSize() throws Exception { + FileConfigurationParser parser = new FileConfigurationParser(); Review comment: Point more so was i assume the same issue may lay in other persistence such as journal etc 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE
michaelandrepearce commented on a change in pull request #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE URL: https://github.com/apache/activemq-artemis/pull/2791#discussion_r314877174 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/AddressSettings.java ## @@ -505,6 +505,9 @@ public long getPageSizeBytes() { } public AddressSettings setPageSizeBytes(final long pageSize) { + if (pageSize > Integer.MAX_VALUE) { + throw new IllegalArgumentException("pageSize must be < " + Integer.MAX_VALUE); Review comment: Not really. If you now decode something thats larger than int it will blow up with the exception youve added. No different. Except we are actually making the method take the correct primitive number 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #2768: ARTEMIS-2433 add ExternalCertificateLoginModule to surface a SASL EXT…
jbertram commented on issue #2768: ARTEMIS-2433 add ExternalCertificateLoginModule to surface a SASL EXT… URL: https://github.com/apache/activemq-artemis/pull/2768#issuecomment-522019846 @gtully, can you `rebase` and `push -f`? A defect was recently fixed that was preventing the Travis PR build from finishing properly. Thanks! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #2783: Treat empty providerUrl string same way as null value.
jbertram commented on issue #2783: Treat empty providerUrl string same way as null value. URL: https://github.com/apache/activemq-artemis/pull/2783#issuecomment-522017246 @Premik, can you please squash your commits and rebase? A defect was recently fixed that prevented the PR test build from finishing properly. Thanks! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE
jbertram commented on issue #2791: ARTEMIS-2450 page-size-bytes should not be greater than Integer.MAX_VALUE URL: https://github.com/apache/activemq-artemis/pull/2791#issuecomment-522007729 @wy96f, can you rebase and push -f to trigger a rebuild of the PR by Travis? An issue was recently fixed that was preventing the PR build from completing properly. Thanks! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] brusdev commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool
brusdev commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool URL: https://github.com/apache/activemq-artemis/pull/2726#discussion_r314716751 ## File path: artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplTest.java ## @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.impl; + +import java.lang.ref.WeakReference; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.junit.Assert; +import org.junit.Test; + +public class ActiveMQServerImplTest extends ActiveMQTestBase { + + @Test + public void testScheduledPoolGC() throws Exception { + ActiveMQServer server = createServer(false); + + server.start(); + + Runnable scheduledRunnable = new Runnable() { + @Override + public void run() { +Assert.fail(); + } + }; + WeakReference scheduledRunnableRef = new WeakReference<>(scheduledRunnable); + + ScheduledExecutorService scheduledPool = server.getScheduledPool(); + ScheduledFuture scheduledFuture = scheduledPool.schedule(scheduledRunnable, 5000, TimeUnit.MILLISECONDS); + + Assert.assertFalse(scheduledFuture.isCancelled()); + Assert.assertTrue(scheduledFuture.cancel(true)); + Assert.assertTrue(scheduledFuture.isCancelled()); + + Assert.assertNotEquals(null, scheduledRunnableRef.get()); Review comment: The scheduledRunnableRef referent shouldn't be cleared until the local variable scheduledRunnable won't be set null. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #2796: ARTEMIS-2451 remove need for knownDestination Cache
jbertram commented on issue #2796: ARTEMIS-2451 remove need for knownDestination Cache URL: https://github.com/apache/activemq-artemis/pull/2796#issuecomment-522006385 I just sent #2800 which is based on your work here, @michaelandrepearce. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] jbertram opened a new pull request #2800: ARTEMIS-2451 eliminate knownDestinations cache
jbertram opened a new pull request #2800: ARTEMIS-2451 eliminate knownDestinations cache URL: https://github.com/apache/activemq-artemis/pull/2800 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool
wy96f commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool URL: https://github.com/apache/activemq-artemis/pull/2726#discussion_r314688744 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java ## @@ -2667,10 +2667,18 @@ public ThreadFactory run() { return new ActiveMQThreadFactory("ActiveMQ-scheduled-threads", false, ClientSessionFactoryImpl.class.getClassLoader()); } }); - scheduledPool = new ScheduledThreadPoolExecutor(configuration.getScheduledThreadPoolMaxSize(), tFactory); + + ScheduledThreadPoolExecutor scheduledPoolExecutor = new ScheduledThreadPoolExecutor(configuration.getScheduledThreadPoolMaxSize(), tFactory); + scheduledPoolExecutor.setRemoveOnCancelPolicy(true); + scheduledPool = scheduledPoolExecutor; } else { this.scheduledPoolSupplied = true; this.scheduledPool = serviceRegistry.getScheduledExecutorService(); + + if (!(scheduledPool instanceof ScheduledThreadPoolExecutor) || Review comment: Shouldn't this be `if (scheduledPool instanceof ScheduledThreadPoolExecutor && !((ScheduledThreadPoolExecutor) scheduledPool).getRemoveOnCancelPolicy()) {`? ScheduledExecutorService used by broker might remove task after cancel, E.g. kinds of executors in Netty. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool
wy96f commented on a change in pull request #2726: ARTEMIS-2392 Enable remove on cancel policy for scheduled pool URL: https://github.com/apache/activemq-artemis/pull/2726#discussion_r314691138 ## File path: artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplTest.java ## @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.impl; + +import java.lang.ref.WeakReference; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.junit.Assert; +import org.junit.Test; + +public class ActiveMQServerImplTest extends ActiveMQTestBase { + + @Test + public void testScheduledPoolGC() throws Exception { + ActiveMQServer server = createServer(false); + + server.start(); + + Runnable scheduledRunnable = new Runnable() { + @Override + public void run() { +Assert.fail(); + } + }; + WeakReference scheduledRunnableRef = new WeakReference<>(scheduledRunnable); + + ScheduledExecutorService scheduledPool = server.getScheduledPool(); + ScheduledFuture scheduledFuture = scheduledPool.schedule(scheduledRunnable, 5000, TimeUnit.MILLISECONDS); + + Assert.assertFalse(scheduledFuture.isCancelled()); + Assert.assertTrue(scheduledFuture.cancel(true)); + Assert.assertTrue(scheduledFuture.isCancelled()); + + Assert.assertNotEquals(null, scheduledRunnableRef.get()); Review comment: `Assert.assertNotEquals(null, scheduledRunnableRef.get());` should be put before `Assert.assertTrue(scheduledFuture.cancel(true));`. GC might occur right at this moment and cause reference object cleared, resulting in test failure. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521938303 @michaelandrepearce squashed commits and pushed. BTW, I added some code to avoid file leak in race conditions in PageSubscriptionImpl::internalGetNext() : ``` serverMessage = ((PageReader) cache).getMessage(retPos, false, true); PageCache previousPageCache = pageReaders.putIfAbsent(retPos.getPageNr(), (PageReader) cache); if (previousPageCache != null && previousPageCache != cache) { // Maybe other cursor iterators have added page reader, we have to close this one to avoid file leak cache.close(); } ``` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521938303 @michaelandrepearce squashed commits and pushed. BTW, I added some code to avoid file leak in race conditions in PageSubscriptionImpl::internalGetNext() : `serverMessage = ((PageReader) cache).getMessage(retPos, false, true); PageCache previousPageCache = pageReaders.putIfAbsent(retPos.getPageNr(), (PageReader) cache); if (previousPageCache != null && previousPageCache != cache) { // Maybe other cursor iterators have added page reader, we have to close this one to avoid file leak cache.close(); }` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521938303 @michaelandrepearce squashed commits and pushed. BTW, I added some code to avoid file leak in race conditions in PageSubscriptionImpl::internalGetNext() : ` serverMessage = ((PageReader) cache).getMessage(retPos, false, true); PageCache previousPageCache = pageReaders.putIfAbsent(retPos.getPageNr(), (PageReader) cache); if (previousPageCache != null && previousPageCache != cache) { // Maybe other cursor iterators have added page reader, we have to close this one to avoid file leak cache.close(); } ` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521938303 @michaelandrepearce squashed commits and pushed. BTW, I added some code to avoid file leak in race conditions in PageSubscriptionImpl::internalGetNext() : ` serverMessage = ((PageReader) cache).getMessage(retPos, false, true); PageCache previousPageCache = pageReaders.putIfAbsent(retPos.getPageNr(), (PageReader) cache); if (previousPageCache != null && previousPageCache != cache) { // Maybe other cursor iterators have added page reader, we have to close this one to avoid file leak cache.close(); }` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521938303 @michaelandrepearce squashed commits and pushed. BTW, I added some code to avoid file leak in race conditions in PageSubscriptionImpl::internalGetNext() : ` serverMessage = ((PageReader) cache).getMessage(retPos, false, true); PageCache previousPageCache = pageReaders.putIfAbsent(retPos.getPageNr(), (PageReader) cache); if (previousPageCache != null && previousPageCache != cache) { // Maybe other cursor iterators have added page reader, we have to close this one to avoid file leak cache.close(); }` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelandrepearce commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521928878 @wy96f can you squash commits pls 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-521921421 pushed commit to close page reader when cursor iterator is closed 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: us...@infra.apache.org With regards, Apache Git Services