[GitHub] [activemq] jlmuir opened a new pull request #394: Improve activemq init JAVACMD auto detection

2019-09-27 Thread GitBox
jlmuir opened a new pull request #394: Improve activemq init JAVACMD auto 
detection
URL: https://github.com/apache/activemq/pull/394
 
 
   If `JAVACMD` is set to `auto` (the default from the `env` file) and the 
current working directory contains a directory named `auto` (and the `auto` 
directory is executable, which directories normally are), the `activemq` init 
script will incorrectly detect the `auto` directory as the `java` binary thus 
leaving `JAVACMD` set to `auto` which is incorrect and will fail to execute the 
Java VM.
   
   To fix this, in the second attempt to detect the `java` binary, repeat the 
tests for a zero-length `JAVACMD` or a `JAVACMD` equal to `auto` before testing 
whether `JAVACMD` does not exist or is not executable.  This is necessary 
because when the length of `JAVA_HOME` is zero, the first attempt to detect the 
location of the `java` binary will not set `JAVACMD` (so it will still be 
`auto` when it reaches the second auto-detection attempt, and since the `auto` 
directory is executable, the `[ ! -x "$JAVACMD" ]` test will evaluate to false).
   
   A real-world example is running the `activemq` init script as a `systemd` 
service where the current working directory for the `activemq` init script ends 
up being the root of the file system (i.e., `/`) and an NFS automounter is 
being used such that `/auto` exists and is a directory (and is executable).
   
   To reproduce the problem from a Bourne-style shell:
   
   ```
   $ unset JAVA_HOME
   $ which java
   /usr/bin/java
   $ cd /tmp
   $ mkdir activemq-javacmd-auto-test
   $ cd activemq-javacmd-auto-test
   $ mkdir auto
   $ export ACTIVEMQ_HOME=/tmp/activemq-javacmd-auto-test
   $ /usr/local/bin/activemq start | grep 'INFO: Using java'
   INFO: Using java 'auto'
   ```
   
   The expected result is that the `INFO` line says:
   
   ```
   INFO: Using java '/usr/bin/java'
   ```


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] clebertsuconic commented on a change in pull request #2848: ARTEMIS-2497: [AMQP] Allow handling of the reject disposition to be configured

2019-09-27 Thread GitBox
clebertsuconic commented on a change in pull request #2848: ARTEMIS-2497: 
[AMQP] Allow handling of the reject disposition to be configured
URL: https://github.com/apache/activemq-artemis/pull/2848#discussion_r329220926
 
 

 ##
 File path: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java
 ##
 @@ -306,6 +306,8 @@
 
private Long globalMaxSize;
 
+   private boolean amqpTreatRejectAsUnmodifiedDeliveryFailed = 
ActiveMQDefaultConfiguration.getDefaultTreatRejectAsUnmodifiedDeliveryFailed();
 
 Review comment:
   Are you changing this similar to your other featrure? add it to 
ProtonProtocolManager instead?


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] clebertsuconic commented on issue #2847: ARTEMIS-2494: [AMQP] Allow Modified disposition to be used signal address full to a sending peer

2019-09-27 Thread GitBox
clebertsuconic commented on issue #2847: ARTEMIS-2494: [AMQP] Allow Modified 
disposition to be used signal address full to a sending peer
URL: https://github.com/apache/activemq-artemis/pull/2847#issuecomment-536069236
 
 
   Can you squash commits? I will then run tests and merge 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [activemq-artemis] asfgit closed pull request #2852: ARTEMIS-2505: Fix wiring of the max-size-bytes-reject-threshold address-setting

2019-09-27 Thread GitBox
asfgit closed pull request #2852: ARTEMIS-2505: Fix wiring of the 
max-size-bytes-reject-threshold address-setting
URL: https://github.com/apache/activemq-artemis/pull/2852
 
 
   


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] asfgit merged pull request #2853: ARTEMIS-2506 MQTT doesn't cleanup underlying connection for bad clients

2019-09-27 Thread GitBox
asfgit merged pull request #2853: ARTEMIS-2506 MQTT doesn't cleanup underlying 
connection for bad clients
URL: https://github.com/apache/activemq-artemis/pull/2853
 
 
   


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] asfgit merged pull request #2853: ARTEMIS-2506 MQTT doesn't cleanup underlying connection for bad clients

2019-09-27 Thread GitBox
asfgit merged pull request #2853: ARTEMIS-2506 MQTT doesn't cleanup underlying 
connection for bad clients
URL: https://github.com/apache/activemq-artemis/pull/2853
 
 
   


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 #2850: ARTEMIS-2504 implement retroactive addresses

2019-09-27 Thread GitBox
jbertram commented on issue #2850: ARTEMIS-2504 implement retroactive addresses
URL: https://github.com/apache/activemq-artemis/pull/2850#issuecomment-535998932
 
 
   > How does someone control if the retroactive address will behave when full, 
e.g. page, block, drop?
   
   For the most part you can control the configuration of the internal 
retroactive address/queues using address settings just like any other address. 
The resources are named according to a specific pattern which can be used in 
the `match` for the address-setting.
   
   > How or where can someone configure the naming of a retroactive address so 
they can control their names to avoid conflict or for security settings.
   
   The naming pattern is outlined in the documentation. Conflicts can be 
avoided through the `internal-naming-prefix` setting.
   
   > For queues created for retroactive, if there is a very high number of 
message groups and someone is using buckets so that the group map doesnt 
explode in size, how is this being avoided in the retroactive queue?
   
   Configure the group buckets via an address-setting. I changed this in my 
latest update.
   
   > Is routing type being preserved.
   
   Yes, it is being preserved now. I changed this in my latest update.
   
   > Why divert and separate address is needed. Why not have the retro queue 
under the original address?
   
   My original plan was to have the ring queues for the retroactive address 
under the main address itself, but I didn't want those queues to be subject to 
the address-settings of that address. Separating the ring queues onto their own 
address means better configuration flexibility.
   
   > Default address routing type in the validator for msg count?
   
   Fixed in the my latest update.
   
   > On the retroactive queue, would want a flag to tell us its a retro queue, 
thats then exposed to jmx so that metrics systems can get the tag, and so 
alerting rules on queue depths can ignore these queues automatically based on 
that flag.
   
   Added on my latest update.
   
   > In future or even in this pr it would be good to be able to explicitly 
set/control [the retroactive-message-count] at an address level. Thus this 
would just be a default thats then applied if none is set at that level
   
   As far as I can tell that is already how `retroactive-message-count` is 
functioning. If not defined then the default of `0` is used. Addresses 
themselves have no direct configuration attributes other than `name`. The 
`address-settings` block is designed to configure the settings for an address. 
I see `retroactive-message-count` the same as settings like `max-size-bytes` 
and a bunch of others.
   
   > This suggests it updates dynamically. It isnt entirely true as will only 
update the default for new addresses, existing addresses where the retro queue 
is created already it doesnt update.
   
   That's not true. There is a listener which will update the `ring-size` on 
the ring queues dynamically.
   
   > This feature would be more powerful if support for a timebased or bytes 
size retention, e.g. retro active for last hour etc.
   
   Fair enough, but that's not the subject of this PR.


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-nms-amqp] HavretGC opened a new pull request #39: AMQNET-617: Support Message Selectors

2019-09-27 Thread GitBox
HavretGC opened a new pull request #39: AMQNET-617: Support Message Selectors
URL: https://github.com/apache/activemq-nms-amqp/pull/39
 
 
   This PR adds support for message selectors.


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] gaohoward commented on a change in pull request #2853: ARTEMIS-2506 MQTT doesn't cleanup underlying connection for bad clients

2019-09-27 Thread GitBox
gaohoward commented on a change in pull request #2853: ARTEMIS-2506 MQTT 
doesn't cleanup underlying connection for bad clients
URL: https://github.com/apache/activemq-artemis/pull/2853#discussion_r329032089
 
 

 ##
 File path: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTConnnectionCleanupTest.java
 ##
 @@ -0,0 +1,84 @@
+/**
+ * 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.tests.integration.mqtt;
+
+import org.apache.activemq.artemis.api.core.ActiveMQException;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import org.apache.activemq.artemis.core.protocol.mqtt.MQTTConnection;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyAcceptor;
+import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import 
org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTestSupport;
+import org.fusesource.mqtt.client.BlockingConnection;
+import org.fusesource.mqtt.client.MQTT;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class MQTTConnnectionCleanupTest extends MQTTTestSupport {
+
+   @Override
+   protected void addMQTTConnector() {
+
+  Map params = new HashMap<>();
+  params.put(TransportConstants.PORT_PROP_NAME, "" + port);
+  params.put(TransportConstants.PROTOCOLS_PROP_NAME, "MQTT");
+  params.put(TransportConstants.CONNECTIONS_ALLOWED, 1);
+  params.put(TransportConstants.HOST_PROP_NAME, "localhost");
+
+  TransportConfiguration mqtt = new 
TransportConfiguration(NETTY_ACCEPTOR_FACTORY, params, "MQTT");
+
+  server.getConfiguration().addAcceptorConfiguration(mqtt);
+   }
+
+   @Test(timeout = 30 * 1000)
+   public void testBadClient() throws Exception {
+  MQTT mqtt = createMQTTConnection();
+  mqtt.setClientId("");
+  mqtt.setCleanSession(true);
+  BlockingConnection connection = mqtt.blockingConnection();
+  connection.connect();
+
+  try {
+ connection = mqtt.blockingConnection();
+ connection.connect();
+ fail("second connection shouldn't be allowed");
+  } catch (Exception e) {
+ //ignore.
+  }
+
+  NettyAcceptor acceptor = (NettyAcceptor) 
server.getRemotingService().getAcceptor("MQTT");
+  assertEquals(1, acceptor.getConnections().size());
+
+  //now simulate a bad client by manually fail the server connection
+  RemotingConnection conn = 
server.getRemotingService().getConnections().iterator().next();
+
+  assertTrue(conn instanceof MQTTConnection);
+
+  conn.fail(new ActiveMQException("testBadClient"));
+
+  assertEquals("Server connection not cleaned up!", 0, 
acceptor.getConnections().size());
 
 Review comment:
   done.


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] gaohoward commented on a change in pull request #2853: ARTEMIS-2506 MQTT doesn't cleanup underlying connection for bad clients

2019-09-27 Thread GitBox
gaohoward commented on a change in pull request #2853: ARTEMIS-2506 MQTT 
doesn't cleanup underlying connection for bad clients
URL: https://github.com/apache/activemq-artemis/pull/2853#discussion_r329028964
 
 

 ##
 File path: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTConnnectionCleanupTest.java
 ##
 @@ -0,0 +1,84 @@
+/**
+ * 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.tests.integration.mqtt;
+
+import org.apache.activemq.artemis.api.core.ActiveMQException;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import org.apache.activemq.artemis.core.protocol.mqtt.MQTTConnection;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyAcceptor;
+import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import 
org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTestSupport;
+import org.fusesource.mqtt.client.BlockingConnection;
+import org.fusesource.mqtt.client.MQTT;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class MQTTConnnectionCleanupTest extends MQTTTestSupport {
+
+   @Override
+   protected void addMQTTConnector() {
+
+  Map params = new HashMap<>();
+  params.put(TransportConstants.PORT_PROP_NAME, "" + port);
+  params.put(TransportConstants.PROTOCOLS_PROP_NAME, "MQTT");
+  params.put(TransportConstants.CONNECTIONS_ALLOWED, 1);
+  params.put(TransportConstants.HOST_PROP_NAME, "localhost");
+
+  TransportConfiguration mqtt = new 
TransportConfiguration(NETTY_ACCEPTOR_FACTORY, params, "MQTT");
+
+  server.getConfiguration().addAcceptorConfiguration(mqtt);
+   }
+
+   @Test(timeout = 30 * 1000)
+   public void testBadClient() throws Exception {
+  MQTT mqtt = createMQTTConnection();
+  mqtt.setClientId("");
+  mqtt.setCleanSession(true);
+  BlockingConnection connection = mqtt.blockingConnection();
+  connection.connect();
+
+  try {
+ connection = mqtt.blockingConnection();
+ connection.connect();
+ fail("second connection shouldn't be allowed");
+  } catch (Exception e) {
+ //ignore.
+  }
+
+  NettyAcceptor acceptor = (NettyAcceptor) 
server.getRemotingService().getAcceptor("MQTT");
+  assertEquals(1, acceptor.getConnections().size());
+
+  //now simulate a bad client by manually fail the server connection
+  RemotingConnection conn = 
server.getRemotingService().getConnections().iterator().next();
+
+  assertTrue(conn instanceof MQTTConnection);
+
+  conn.fail(new ActiveMQException("testBadClient"));
+
+  assertEquals("Server connection not cleaned up!", 0, 
acceptor.getConnections().size());
 
 Review comment:
   ok will do
   


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 #2851: ARTEMIS-2503 Improve wildcards for the roles access match

2019-09-27 Thread GitBox
brusdev commented on a change in pull request #2851: ARTEMIS-2503 Improve 
wildcards for the roles access match
URL: https://github.com/apache/activemq-artemis/pull/2851#discussion_r329007446
 
 

 ##
 File path: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/JMXAccessControlList.java
 ##
 @@ -25,12 +25,22 @@
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+import org.apache.activemq.artemis.core.settings.HierarchicalRepository;
+import 
org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+
 public class JMXAccessControlList {
 
private Access defaultAccess = new Access("*");
-   private Map domainAccess = new HashMap<>();
+   private HierarchicalRepository domainAccess;
private ConcurrentHashMap> whitelist = new 
ConcurrentHashMap<>();
 
+   public JMXAccessControlList() {
+  WildcardConfiguration domainAccessWildcardConfiguration = new 
WildcardConfiguration();
 
 Review comment:
   I pushed a commit to use the same flexible wildcard support used for the 
addresses but I added a new node to customize the wildcard syntax for the 
authorisation key attributes because:
   - the addresses and the keys wildcard support could have different 
requirements
   - to avoid to introduce a dependence between management.xml and broker.xml
   
   


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] franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535862167
 
 
   @wy96f probably the size estimator of Netty or/and the pending writes count 
is not working as I was expecting with file regions...


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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
wy96f commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535853861
 
 
   > Re the timeout I need to re-read your previous answers: you said that both 
the approaches (file region/chunked files) max out network and take the same 
time, but only the latter will fail on replication timeout?
   
   Yes. In file region, sync done msg was sent and received by backup 7 mins 
later. In chunked file, sync done msg was sent 40 seconds later and received by 
backup 7 min later because of packets queued up on live server side.
   
   collecting samples with -t. see https://filebin.net/1lyef9lv6rkew9ks
   
   


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] franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535837012
 
 
   To make profiling traces comparable in number of samples you should need to 
limit the duration and make them equals eg -d 10 (seconds) 
   Ok to do it after 40 seconds depending which part of the catching up we are 
interested in...
   Re the timeout I need to re-read your previous answers: you said that both 
the approaches (file region/chunked files) max out network and take the same 
time, but only the latter will fail on replication timeout?


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] assens commented on issue #2837: ARTEMIS-2476: New MQTT subscriptions receive older (not last published) retained message.

2019-09-27 Thread GitBox
assens commented on issue #2837: ARTEMIS-2476: New MQTT subscriptions receive 
older (not last published) retained message.
URL: https://github.com/apache/activemq-artemis/pull/2837#issuecomment-535832779
 
 
   > I will merge this after I release 2.10.1, ok?
   
   Thanks. We already use a patched version in our project, but I'd like to 
update to a release version, as soon as it is out. 
   
   


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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
wy96f commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535831615
 
 
   
   > tbh I'm not sure it makes sense to have to have such small timeout in that 
case, given that depends by disk speed/network bandwidth and total transferred 
size
   
   Do you mean initial-replication-sync-timeout? If so, it's ok with 
default(3) in master and file region=true, but not in file region=false, so 
i set it to 300 for profiling.
   
   > Just one advice: try collecting samples with -t..I wasn't expecting 
blockUntiWritable to be that costy
   
   `./profiler.sh -d 60 -t -f xx.svg pid` like this, profiling for master,file 
region=false/true? For file region=false, should i collect samples from 
beginning or after 40s(after log showed last page sent)?


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] franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535824970
 
 
   Sorry I have missed that the file region=true version was failing on timeout 
as well: tbh I'm not sure it makes sense to have to have such small timeout in 
that case, given that depends by disk speed/network bandwidth and total 
transferred size
   Just one advice: try collecting samples with -t..I wasn't expecting 
blockUntiWritable to be that costy


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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
wy96f edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535813732
 
 
   @franz1981 
   profiling without file region:
   https://filebin.net/r9o4bupoym9zxwk9/netty_false.svg?t=ld5f197t
   Note I profiled after 40s(after log showed last page sent) so most of 
samples were about netty.
   
   profiling with file region:
   https://filebin.net/r9o4bupoym9zxwk9/netty_true.svg?t=mo3tho33
   
   profiling master:
   https://filebin.net/r9o4bupoym9zxwk9/profiler_master.svg?t=mo3tho33


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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
wy96f edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535813732
 
 
   @franz1981 
   profiling without file region:
   https://filebin.net/r9o4bupoym9zxwk9/netty_false.svg?t=ld5f197t
   Note I profiled after 40s(after log showed last page sent) so most of 
samples were about netty.
   
   profiling with file region:
   https://filebin.net/r9o4bupoym9zxwk9/netty_true.svg?t=mlott712
   
   profiling master:
   https://filebin.net/r9o4bupoym9zxwk9/profiler_master.svg?t=mlott712


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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
wy96f commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535813732
 
 
   @franz1981 
   profiling without file region:
   [https://filebin.net/r9o4bupoym9zxwk9/netty_false.svg?t=ld5f197t](url)
   Note I profiled after 40s(after log showed last page sent) so most of 
samples were about netty.
   
   profiling with file region:
   [https://filebin.net/r9o4bupoym9zxwk9/netty_true.svg?t=mlott712](url)
   
   profiling master:
   [https://filebin.net/r9o4bupoym9zxwk9/profiler_master.svg?t=mlott712](url)


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 #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
wy96f commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535807466
 
 
   @franz1981 
   > If both cases (with/without file region) are saturating the network, why 
the latter will take more time? The total amount of data sent should be the 
same...
   
   They're taking same time(~7 mins). As i said,
   ``
   In the case of -Dio.netty.file.region=true and master, log(something like 
this 2019-09-26 11:02:49,348 DEBUG 
[org.apache.activemq.artemis.core.replication.ReplicationManager] sending 
1048576 bytes on file ) showed it took about 7 minutes to transfer files, 
then synchronization done message sent. However in the case of 
-Dio.netty.file.region=false, log showed it took about about 40 seconds to 
transfer files, then sync done message sent. 
   ``
   ``
   Yes, i saw the network was saturated(initial-replication-sync-timeout was 
set to 30, otherwise replication failed due to timeout). Note that log 
showed 13:47:01,945 last page sent, and sar showed 01:47:37 PM queued up data 
still transferring and saturating network.
   ``
   Saturation lasted for ~7 mins.
   
   > Do you have tried master as well?
   
   The same. 
   
   


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] franz1981 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
franz1981 edited a comment on issue #2845: ARTEMIS-2336 Use zero copy to 
replicate journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535803060
 
 
   If both cases (with/without file region) are saturating the network, why the 
latter will take more time? The total amount of data sent should be the same...
   Do you have tried master as well? 
   I'm start to think that the pipelining happening while copying data in a 
non-netty thread and a separate Netty thread sending them across network is 
beneficial to improve the overall throughput, because we can do something (ie 
reading the file) while Netty is taking care to send data across network. But 
that means that if we don't use file regions I expect that network is not 
saturated 100% of the time and sometime wait ChunkInput to finish reading data 
and saturate it again: sar shows averages over sampling intervals so I believe 
that we can't spot those spikes...
   I don't know if your acceptor configuration of TCP buffer is working,need to 
inspect the logs...
   And profiling could be helpful as well...


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] franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file (AGAIN)

2019-09-27 Thread GitBox
franz1981 commented on issue #2845: ARTEMIS-2336 Use zero copy to replicate 
journal/page/large message file (AGAIN)
URL: https://github.com/apache/activemq-artemis/pull/2845#issuecomment-535803060
 
 
   If both cases (with/without file region) are saturating the network, why the 
latter will take more time? The total amount of data sent should be the same...
   Do you have tried master as well? 
   I'm start to think that the pipelining happening while copying data in a 
non-netty thread and a separate Netty thread sending them across network is 
beneficial to improve the overall throughput, because we can do something (ie 
reading the file) while Netty is taking care to send data across network. But 
that means that if we don't use file regions I expect that network is not 
saturated 100% of the time and sometime wait ChunkInput to finish reading data 
and saturate it again...
   I don't know if your acceptor configuration of TCP buffer is working,need to 
inspect the logs...
   And profiling could be helpful as well...


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