[GitHub] lucperkins opened a new pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
lucperkins opened a new pull request #920: Provide an Ansible playbook for AWS 
with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920
 
 
   ### Motivation
   
   The 
[streamlio/messaging-benchmark](https://github.com/streamlio/messaging-benchmark)
 repo provides an Ansible playbook for deploying a Pulsar cluster on AWS. This 
PR seeks to provide the same but in an official capacity in the main repo.
   
   ### Modifications
   
   I've modified the Ansible playbook in the original repo by removing the 
client node from the installation and renaming some resources to make their 
purpose more clear.
   
   ### Result
   
   Users will be able to stand up a Pulsar cluster on AWS using resources in 
the main Pulsar repo.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] afalko commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
afalko commented on a change in pull request #920: Provide an Ansible playbook 
for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153579421
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] afalko commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
afalko commented on a change in pull request #920: Provide an Ansible playbook 
for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153579568
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] afalko commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
afalko commented on a change in pull request #920: Provide an Ansible playbook 
for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153579930
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] afalko commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
afalko commented on a change in pull request #920: Provide an Ansible playbook 
for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153580924
 
 

 ##
 File path: ansible/deploy-pulsar.yaml
 ##
 @@ -0,0 +1,166 @@
+#
+# 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.
+#
+
+- name: Format and mount disks
+  hosts: pulsar
+  connection: ssh
+  become: true
+  tasks:
+- shell: >
+sudo tuned-adm profile latency-performance &&
+mkfs.xfs /dev/nvme0n1 &&
+mkfs.xfs /dev/nvme1n1 &&
+mkdir -p /mnt/journal &&
+mkdir -p /mnt/storage &&
+mount -o defaults,noatime,nodiscard,nobarrier /dev/nvme0n1 
/mnt/journal &&
+mount -o defaults,noatime,nodiscard,nobarrier /dev/nvme1n1 /mnt/storage
+  args:
+creates: /mnt/journal
+
+- name: Pulsar setup
+  hosts: all
+  connection: ssh
+  become: true
+  tasks:
+- name: Install RPM packages
+  yum: pkg={{ item }} state=latest
+  with_items:
+  - wget
+  - java
+  - sysstat
+  - vim
+
+- set_fact:
+zookeeperServers: "{{ groups['zookeeper']|map('extract', hostvars, 
['ansible_default_ipv4', 'address'])|map('regex_replace', '(.*)', '\\1:2181') | 
join(',') }}"
+serviceUrl: "pulsar://{{ hostvars[groups['pulsar'][0]].private_ip 
}}:6650/"
 
 Review comment:
   I don't know if terraform supports route53, but it might be more elegant to 
set this up via an internal domain name. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat closed pull request #919: Fix: broker-zookeeper latency-metrics's dimension-name

2017-11-28 Thread GitBox
merlimat closed pull request #919: Fix: broker-zookeeper latency-metrics's 
dimension-name
URL: https://github.com/apache/incubator-pulsar/pull/919
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java
 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java
index de988b7d8..8588d9f4c 100644
--- 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java
+++ 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java
@@ -62,11 +62,11 @@ Metrics getTopicLoadMetrics() {
 }
 
 Metrics getZkWriteLatencyMetrics() {
-return getDimensionMetrics("zk_write_latency", "zk_write_latency", 
zkWriteLatencyStats);
+return getDimensionMetrics("zk_write_latency", "zk_write", 
zkWriteLatencyStats);
 }
 
 Metrics getZkReadLatencyMetrics() {
-return getDimensionMetrics("zk_read_latency", "zk_read_latency", 
zkReadLatencyStats);
+return getDimensionMetrics("zk_read_latency", "zk_read", 
zkReadLatencyStats);
 }
 
 Metrics getDimensionMetrics(String metricsName, String dimensionName, 
DimensionStats stats) {
@@ -90,6 +90,9 @@ Metrics getDimensionMetrics(String metricsName, String 
dimensionName, DimensionS
 
 public void reset() {
 metricsList.clear();
+topicLoadStats.reset();
+zkWriteLatencyStats.reset();
+zkReadLatencyStats.reset();
 }
 
 public void recordTopicLoadTimeValue(long topicLoadLatencyMs) {
diff --git 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/DimensionStats.java
 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/DimensionStats.java
index 08db011f6..0844e2914 100644
--- 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/DimensionStats.java
+++ 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/DimensionStats.java
@@ -102,6 +102,10 @@ public double getDimensionCount() {
 return 
defaultRegistry.getSampleValue(dimensionCountLabel).doubleValue();
 }
 
+public void reset() {
+summary.clear();
+}
+
 private double getQuantile(double q) {
 return defaultRegistry.getSampleValue(name, QUANTILE_LABEL, new 
String[] { Collector.doubleToGoString(q) })
 .doubleValue();
diff --git 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/zookeeper/ZooKeeperClientAspectJTest.java
 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/zookeeper/ZooKeeperClientAspectJTest.java
index 93a3b08d6..e950fbd62 100644
--- 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/zookeeper/ZooKeeperClientAspectJTest.java
+++ 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/zookeeper/ZooKeeperClientAspectJTest.java
@@ -185,23 +185,23 @@ public void testZkOpStatsMetrics() throws Exception {
 
pulsarClient.createProducer("persistent://my-property/use/my-ns/my-topic1");
 Metrics zkOpMetric = getMetric(pulsar, "zk_write_latency");
 Assert.assertNotNull(zkOpMetric);
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_rate_s"));
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_time_95percentile_ms"));
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_time_99_99_percentile_ms"));
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_time_99_9_percentile_ms"));
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_time_99_percentile_ms"));
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_time_mean_ms"));
-
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_latency_time_median_ms"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_rate_s"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_time_95percentile_ms"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_time_99_99_percentile_ms"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_time_99_9_percentile_ms"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_time_99_percentile_ms"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_time_mean_ms"));
+
Assert.assertTrue(zkOpMetric.getMetrics().containsKey("brk_zk_write_time_median_ms"));
 
 zkOpMetric = getMetric(pulsar, "zk_read_latency");
 Assert.assertNotNull(zkOpMetric);
-
Assert.a

[GitHub] lucperkins opened a new pull request #921: Restructure bare metal deployment documentation (WIP)

2017-11-28 Thread GitBox
lucperkins opened a new pull request #921: Restructure bare metal deployment 
documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/921
 
 
   ### Motivation
   
   At the moment, the instructions on installing a bare metal cluster or 
instance are overly complex and in need of trimming and streamlining.
   
   ### Modifications
   
   This PR splits the documentation into separate docs (cluster vs. instance) 
and streamlines wherever possible.
   
   ### Result
   
   Users will have a much easier time doing bare metal deployments.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on issue #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on issue #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#issuecomment-347655624
 
 
   @rdhabalia Fixed the issues after upgrade, can you give a final check? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on issue #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on issue #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#issuecomment-347335108
 
 
   retest this please


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rdhabalia commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
rdhabalia commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153636992
 
 

 ##
 File path: pom.xml
 ##
 @@ -125,7 +125,7 @@ flexible messaging model and an intuitive client 
API.
   
 org.asynchttpclient
 async-http-client
-2.0.31
+2.1.0-alpha26
 
 Review comment:
   we may want to add following properties in 
[ahc.properties](https://github.com/apache/incubator-pulsar/blob/master/pulsar-client/src/main/resources/ahc.properties)
 or just copy `ahc-default.properties` file from `2.1.0-alpha26` to 
`pulsar-client`.
   ```
   
org.apache.pulsar.shade.org.asynchttpclient.aggregateWebSocketFrameFragments=true
   org.apache.pulsar.shade.org.asynchttpclient.useLaxCookieEncoder=false
   org.apache.pulsar.shade.org.asynchttpclient.useInsecureTrustManager=false
   org.apache.pulsar.shade.org.asynchttpclient.acceptAnyCertificate=false
   
org.apache.pulsar.shade.org.asynchttpclient.disableHttpsEndpointIdentificationAlgorithm=false
   
org.apache.pulsar.shade.org.asynchttpclient.httpClientCodecInitialBufferSize=128
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rdhabalia commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
rdhabalia commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153638970
 
 

 ##
 File path: 
pulsar-common/src/main/java/org/apache/pulsar/common/util/protobuf/ByteBufCodedOutputStream.java
 ##
 @@ -261,7 +260,6 @@ static int makeTag(final int fieldNumber, final int 
wireType) {
 /** Write an double field, including tag, to the stream. */
 public void writeDouble(final int fieldNumber, double value) throws 
IOException {
 writeTag(fieldNumber, WireFormat.WIRETYPE_FIXED64);
-buf.order(ByteOrder.LITTLE_ENDIAN).writeDouble(value);
+buf.writeLongLE(Double.doubleToLongBits(value));
 
 Review comment:
   don't we have a way to preserve double value instead casting to long?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rdhabalia commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
rdhabalia commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153633906
 
 

 ##
 File path: build/docker/protobuf.patch
 ##
 @@ -161,30 +167,30 @@ index 4c087db..cd18e28 100644
  "classname", ClassName(descriptor_));
  }
}
-@@ -760,9 +792,21 @@ void 
MessageGenerator::GenerateDescriptorMethods(io::Printer* printer) {
+@@ -760,9 +794,21 @@ void 
MessageGenerator::GenerateDescriptorMethods(io::Printer* printer) {
  void MessageGenerator::GenerateCommonBuilderMethods(io::Printer* printer) {
printer->Print(
  "// Construct using $classname$.newBuilder()\n"
 -"private Builder() {\n"
-+"private final io.netty.util.Recycler.Handle handle;\n"
-+"private Builder(io.netty.util.Recycler.Handle handle) {\n"
++"private final io.netty.util.Recycler.Handle handle;\n"
++"private Builder(io.netty.util.Recycler.Handle handle) {\n"
 +"  this.handle = handle;\n"
  "  maybeForceBuilderInitialization();\n"
  "}\n"
 +"private final static io.netty.util.Recycler RECYCLER = new 
io.netty.util.Recycler() {\n"
-+"   protected Builder newObject(io.netty.util.Recycler.Handle handle) {\n"
++"   protected Builder newObject(io.netty.util.Recycler.Handle 
handle) {\n"
 +" return new Builder(handle);\n"
 +"   }\n"
 +"  };\n"
 +"\n"
 +" public void recycle() {\n"
 +"  clear();\n"
-+"  if (handle != null) {RECYCLER.recycle(this, handle);}\n"
-+"  }\n"
++"  handle.recycle(this);\n"
 
 Review comment:
   I am not exactly sure, but if we remove null-check then can we have any 
scenario where we recycle objects whose handler is null.?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rdhabalia commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
rdhabalia commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153639881
 
 

 ##
 File path: 
pulsar-common/src/test/java/org/apache/pulsar/common/api/DoubleByteBufTest.java
 ##
 @@ -50,5 +50,22 @@ public void testReadableBytes() throws Exception {
 buf.skipBytes(64);
 assertEquals(buf.readableBytes(), 256 - 64 * (i + 1));
 }
+
+buf.release();
+}
+
+@Test
+public void testCapacity() throws Exception {
+
+ByteBuf b1 = PooledByteBufAllocator.DEFAULT.heapBuffer(128, 128);
+b1.writerIndex(b1.capacity());
+ByteBuf b2 = PooledByteBufAllocator.DEFAULT.heapBuffer(128, 128);
+b2.writerIndex(b2.capacity());
+ByteBuf buf = DoubleByteBuf.get(b1, b2);
+
+assertEquals(buf.capacity(), 256);
+assertEquals(buf.maxCapacity(), 256);
+
+buf.release();
 
 Review comment:
   not necessary but as we don't have `release()` test in this class, should we 
also assert if b1 and b2 are also released and set as null by `DoubleByteBuf`


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153646222
 
 

 ##
 File path: pom.xml
 ##
 @@ -125,7 +125,7 @@ flexible messaging model and an intuitive client 
API.
   
 org.asynchttpclient
 async-http-client
-2.0.31
+2.1.0-alpha26
 
 Review comment:
   Oh, sure, should I then copy the properties file and modify the package 
name, right?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153646724
 
 

 ##
 File path: build/docker/protobuf.patch
 ##
 @@ -161,30 +167,30 @@ index 4c087db..cd18e28 100644
  "classname", ClassName(descriptor_));
  }
}
-@@ -760,9 +792,21 @@ void 
MessageGenerator::GenerateDescriptorMethods(io::Printer* printer) {
+@@ -760,9 +794,21 @@ void 
MessageGenerator::GenerateDescriptorMethods(io::Printer* printer) {
  void MessageGenerator::GenerateCommonBuilderMethods(io::Printer* printer) {
printer->Print(
  "// Construct using $classname$.newBuilder()\n"
 -"private Builder() {\n"
-+"private final io.netty.util.Recycler.Handle handle;\n"
-+"private Builder(io.netty.util.Recycler.Handle handle) {\n"
++"private final io.netty.util.Recycler.Handle handle;\n"
++"private Builder(io.netty.util.Recycler.Handle handle) {\n"
 +"  this.handle = handle;\n"
  "  maybeForceBuilderInitialization();\n"
  "}\n"
 +"private final static io.netty.util.Recycler RECYCLER = new 
io.netty.util.Recycler() {\n"
-+"   protected Builder newObject(io.netty.util.Recycler.Handle handle) {\n"
++"   protected Builder newObject(io.netty.util.Recycler.Handle 
handle) {\n"
 +" return new Builder(handle);\n"
 +"   }\n"
 +"  };\n"
 +"\n"
 +" public void recycle() {\n"
 +"  clear();\n"
-+"  if (handle != null) {RECYCLER.recycle(this, handle);}\n"
-+"  }\n"
++"  handle.recycle(this);\n"
 
 Review comment:
   No, I've made the handle final and I've checked that is only set to null on 
the "default" instances for these object. The default instance is static and 
it's not used outside the class, is just there to be cloned when creating new 
builders. In any case it would be an error to recycle a static object.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153646828
 
 

 ##
 File path: 
pulsar-common/src/test/java/org/apache/pulsar/common/api/DoubleByteBufTest.java
 ##
 @@ -50,5 +50,22 @@ public void testReadableBytes() throws Exception {
 buf.skipBytes(64);
 assertEquals(buf.readableBytes(), 256 - 64 * (i + 1));
 }
+
+buf.release();
+}
+
+@Test
+public void testCapacity() throws Exception {
+
+ByteBuf b1 = PooledByteBufAllocator.DEFAULT.heapBuffer(128, 128);
+b1.writerIndex(b1.capacity());
+ByteBuf b2 = PooledByteBufAllocator.DEFAULT.heapBuffer(128, 128);
+b2.writerIndex(b2.capacity());
+ByteBuf buf = DoubleByteBuf.get(b1, b2);
+
+assertEquals(buf.capacity(), 256);
+assertEquals(buf.maxCapacity(), 256);
+
+buf.release();
 
 Review comment:
   Yes, I can add that check


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rdhabalia commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
rdhabalia commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153646864
 
 

 ##
 File path: pom.xml
 ##
 @@ -125,7 +125,7 @@ flexible messaging model and an intuitive client 
API.
   
 org.asynchttpclient
 async-http-client
-2.0.31
+2.1.0-alpha26
 
 Review comment:
   yes, that would be better.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] lucperkins commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
lucperkins commented on a change in pull request #920: Provide an Ansible 
playbook for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153655770
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] lucperkins commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
lucperkins commented on a change in pull request #920: Provide an Ansible 
playbook for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153656217
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] lucperkins commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
lucperkins commented on a change in pull request #920: Provide an Ansible 
playbook for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153655770
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] afalko commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
afalko commented on a change in pull request #920: Provide an Ansible playbook 
for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153661028
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] lucperkins commented on a change in pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)

2017-11-28 Thread GitBox
lucperkins commented on a change in pull request #920: Provide an Ansible 
playbook for AWS with documentation (WIP)
URL: https://github.com/apache/incubator-pulsar/pull/920#discussion_r153662966
 
 

 ##
 File path: ansible/provision-pulsar-aws.tf
 ##
 @@ -0,0 +1,117 @@
+variable "public_key_path" {
+  description = <

[GitHub] merlimat commented on a change in pull request #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on a change in pull request #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#discussion_r153672292
 
 

 ##
 File path: 
pulsar-common/src/main/java/org/apache/pulsar/common/util/protobuf/ByteBufCodedOutputStream.java
 ##
 @@ -261,7 +260,6 @@ static int makeTag(final int fieldNumber, final int 
wireType) {
 /** Write an double field, including tag, to the stream. */
 public void writeDouble(final int fieldNumber, double value) throws 
IOException {
 writeTag(fieldNumber, WireFormat.WIRETYPE_FIXED64);
-buf.order(ByteOrder.LITTLE_ENDIAN).writeDouble(value);
+buf.writeLongLE(Double.doubleToLongBits(value));
 
 Review comment:
   It's not a "casting" to long. The value is anyway encoded as a varint. I 
remember I've checked when I made the original change. The bit wise output is 
that same as now, otherwise the test would catch it anyway.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on issue #689: Netty 4.1

2017-11-28 Thread GitBox
merlimat commented on issue #689: Netty 4.1
URL: https://github.com/apache/incubator-pulsar/pull/689#issuecomment-347722576
 
 
   @rdhabalia Updated with comments


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] sijie opened a new issue #922: Allow running bookie together with broker

2017-11-28 Thread GitBox
sijie opened a new issue #922: Allow running bookie together with broker
URL: https://github.com/apache/incubator-pulsar/issues/922
 
 
   Description:
   
   To simplify deployment, it will be good to run bookie along with broker. so 
people don't have to run separate bookkeeper cluster. It is good for small-size 
or midsize deployments.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on issue #922: Allow running bookie together with broker

2017-11-28 Thread GitBox
merlimat commented on issue #922: Allow running bookie together with broker
URL: 
https://github.com/apache/incubator-pulsar/issues/922#issuecomment-347732687
 
 
   ?  We could add a config option on whether to start bookie component in same 
process as broker, also with bookie auto-recovery as well. 
   
   In any case, we should just point the bookie to `conf/bookkeeper.conf` 
without having to import the same config keys into `broker.conf`


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat opened a new pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose journalSyncData option

2017-11-28 Thread GitBox
merlimat opened a new pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose 
journalSyncData option
URL: https://github.com/apache/incubator-pulsar/pull/923
 
 
   ### Motivation
   
   Upgrade to bk-4.3.1.83-yahoo to expose journalSyncData option
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat closed issue #686: DuplicatedByteBuf is deprecated

2017-11-28 Thread GitBox
merlimat closed issue #686: DuplicatedByteBuf is deprecated
URL: https://github.com/apache/incubator-pulsar/issues/686
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] jiazhai commented on issue #917: Provide an DCOS Universe package for pulsar

2017-11-28 Thread GitBox
jiazhai commented on issue #917: Provide an DCOS Universe package for pulsar
URL: 
https://github.com/apache/incubator-pulsar/issues/917#issuecomment-347736962
 
 
   Pulsar is composed of several apps (such as bookie, broker, monitor) 
together. But currently it only support to have one universe package for one 
app. Sadly, We may not able to do it currently.
   
   Universe package could be treat as a wrap on Marathon. Marathon supports 
starting multiple apps from one json, which called a "[Marathon Application 
Groups](https://mesosphere.github.io/marathon/docs/application-groups.html)", 
this json is submitted to [/v2/groups 
endpoint](https://mesosphere.github.io/marathon/docs/rest-api.html#post-v2-groups).
   
   However, the [Cosmos (the DC/OS package 
manager)](https://github.com/dcos/cosmos/) doesn?t accept the same request, 
because it only accepts request for /v2/apps endpoint from [this 
code](https://github.com/dcos/cosmos/blob/master/cosmos-server/src/main/scala/com/mesosphere/cosmos/MarathonClient.scala#L26),
 which is starting a single app.
   
   There was already an 
[issue](https://github.com/mesosphere/universe/issues/569) opened to request a 
supporting for this group work in DCOS.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] jiazhai commented on issue #917: Provide an DCOS Universe package for pulsar

2017-11-28 Thread GitBox
jiazhai commented on issue #917: Provide an DCOS Universe package for pulsar
URL: 
https://github.com/apache/incubator-pulsar/issues/917#issuecomment-347736962
 
 
   Pulsar is composed of several apps (such as bookie, broker, monitor) 
together. But currently it only support to have one universe package for one 
app. Sadly, We may not able to do it currently.
   
   Universe package could be treat as a wrap on Marathon. Marathon supports 
starting multiple apps from one json, which called a "[Marathon Application 
Groups](https://mesosphere.github.io/marathon/docs/application-groups.html)", 
this json is submitted to [/v2/groups 
endpoint](https://mesosphere.github.io/marathon/docs/rest-api.html#post-v2-groups).
   
   However, the [Cosmos (the DC/OS package 
manager)](https://github.com/dcos/cosmos/) doesn?t accept the same request, 
because it only accepts request for /v2/apps endpoint from [this 
code](https://github.com/dcos/cosmos/blob/master/cosmos-server/src/main/scala/com/mesosphere/cosmos/MarathonClient.scala#L26),
 which is starting a single app.
   
   There was already an 
[issue](https://github.com/mesosphere/universe/issues/569) opened to request a 
supporting for this group work in DCOS.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zhaijack commented on issue #917: Provide an DCOS Universe package for pulsar

2017-11-28 Thread GitBox
zhaijack commented on issue #917: Provide an DCOS Universe package for pulsar
URL: 
https://github.com/apache/incubator-pulsar/issues/917#issuecomment-347737386
 
 
   Pulsar is composed of several apps (such as bookie, broker, monitor) 
together. But currently it only support to have one universe package for one 
app. Sadly, We may not able to do it currently.
   
   Universe package could be treat as a wrap on Marathon. Marathon supports 
starting multiple apps from one json, which called a "[Marathon Application 
Groups](https://mesosphere.github.io/marathon/docs/application-groups.html)", 
this json is submitted to [/v2/groups 
endpoint](https://mesosphere.github.io/marathon/docs/rest-api.html#post-v2-groups).
   
   However, the [Cosmos (the DC/OS package 
manager)](https://github.com/dcos/cosmos/) doesn?t accept the same request, 
because it only accepts request for /v2/apps endpoint from [this 
code](https://github.com/dcos/cosmos/blob/master/cosmos-server/src/main/scala/com/mesosphere/cosmos/MarathonClient.scala#L26),
 which is starting a single app.
   
   There was already an 
[issue](https://github.com/mesosphere/universe/issues/569) opened to request a 
supporting for this group work in DCOS.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zhaijack opened a new issue #924: Provide a json file for Marathon app groups to run Pulsar on DCOS

2017-11-28 Thread GitBox
zhaijack opened a new issue #924: Provide a json file for Marathon app groups 
to run Pulsar on DCOS
URL: https://github.com/apache/incubator-pulsar/issues/924
 
 
   There was a requirement to run Pulsar on DCOS in #917 , but Universe Package 
is not support well now. This ticket would like to create  a json file for 
Marathon app groups to run Pulsar on DCOS.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zhaijack commented on issue #917: Provide an DCOS Universe package for pulsar

2017-11-28 Thread GitBox
zhaijack commented on issue #917: Provide an DCOS Universe package for pulsar
URL: 
https://github.com/apache/incubator-pulsar/issues/917#issuecomment-347738299
 
 
   would like to left this issue tracking the DCOS supporting work, and open 
another issue #924 for the marathon app groups work.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] zhaijack commented on issue #917: Provide an DCOS Universe package for pulsar

2017-11-28 Thread GitBox
zhaijack commented on issue #917: Provide an DCOS Universe package for pulsar
URL: 
https://github.com/apache/incubator-pulsar/issues/917#issuecomment-347738299
 
 
   would like to left this issue tracking the DCOS package supporting work, and 
open another issue #924 for the marathon app groups work.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rdhabalia commented on a change in pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose journalSyncData option

2017-11-28 Thread GitBox
rdhabalia commented on a change in pull request #923: Upgrade to 
bk-4.3.1.83-yahoo to expose journalSyncData option
URL: https://github.com/apache/incubator-pulsar/pull/923#discussion_r153695650
 
 

 ##
 File path: pom.xml
 ##
 @@ -106,15 +106,15 @@ flexible messaging model and an intuitive client 
API.
 UTF-8
 UTF-8
 
-4.3.1.82-yahoo
+4.3.1.83-yahoo
 
 Review comment:
   I didn't realize that we incremented bk version from `4.3.1.72-yahoo` to 
`4.3.1.82-yahoo`  in #689. In past it was always useful to know BK-changes in 
the PR when we have incremented bk-version to confirm changes in case of any 
issue (as it's tricky in yahooBK to know changes for a specific version).
   is it fine to mention changes or just commit-range from 
[f87e0eda574](https://github.com/yahoo/bookkeeper/commit/f87e0eda574fb5f9d22dd7d92422eb221199a92c)
 to 
[165971209](https://github.com/yahoo/bookkeeper/commit/165971209f933e0bcefaac997db8e75c8776f942)
 in PR description.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on a change in pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose journalSyncData option

2017-11-28 Thread GitBox
merlimat commented on a change in pull request #923: Upgrade to 
bk-4.3.1.83-yahoo to expose journalSyncData option
URL: https://github.com/apache/incubator-pulsar/pull/923#discussion_r153696154
 
 

 ##
 File path: pom.xml
 ##
 @@ -106,15 +106,15 @@ flexible messaging model and an intuitive client 
API.
 UTF-8
 UTF-8
 
-4.3.1.82-yahoo
+4.3.1.83-yahoo
 
 Review comment:
   Yes, the increment there was needed because the berry version needs to be 
the same across BK and Pulsar at this point. 
   
   Since 4.3.1.83 was already with netty 4.1, I had to first get broker to work 
with 4.1. 
   
   The BK version change should be mentioned in one of the commits and should 
be there in the log of the squashed commit that was merged. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat commented on a change in pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose journalSyncData option

2017-11-28 Thread GitBox
merlimat commented on a change in pull request #923: Upgrade to 
bk-4.3.1.83-yahoo to expose journalSyncData option
URL: https://github.com/apache/incubator-pulsar/pull/923#discussion_r153696236
 
 

 ##
 File path: pom.xml
 ##
 @@ -106,15 +106,15 @@ flexible messaging model and an intuitive client 
API.
 UTF-8
 UTF-8
 
-4.3.1.82-yahoo
+4.3.1.83-yahoo
 
 Review comment:
   Oh, I see. Yeah we should probably mention the changes in the bk version


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] merlimat closed pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose journalSyncData option

2017-11-28 Thread GitBox
merlimat closed pull request #923: Upgrade to bk-4.3.1.83-yahoo to expose 
journalSyncData option
URL: https://github.com/apache/incubator-pulsar/pull/923
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/conf/bookkeeper.conf b/conf/bookkeeper.conf
index 60803af4b..78b2ae44f 100644
--- a/conf/bookkeeper.conf
+++ b/conf/bookkeeper.conf
@@ -142,6 +142,14 @@ journalWriteBufferSizeKB=64
 # Should we remove pages from page cache after force write
 journalRemoveFromPageCache=true
 
+# Should the data be fsynced on journal before acknowledgment.
+# By default, data sync is enabled to guarantee durability of writes.
+# Beware: while disabling data sync in the Bookie journal might improve the 
bookie write performance, it will also
+# introduce the possibility of data loss. With no sync, the journal entries 
are written in the OS page cache but
+# not flushed to disk. In case of power failure, the affected bookie might 
lose the unflushed data. If the ledger
+# is replicated to multiple bookies, the chances of data loss are reduced 
though still present.
+journalSyncData=true
+
 # Should we group journal force writes, which optimize group commit
 # for higher throughput
 journalAdaptiveGroupWrites=true
diff --git a/pom.xml b/pom.xml
index fea799690..6fbb017be 100644
--- a/pom.xml
+++ b/pom.xml
@@ -106,7 +106,7 @@ flexible messaging model and an intuitive client 
API.
 UTF-8
 UTF-8
 
-4.3.1.82-yahoo
+4.3.1.83-yahoo
 3.4.10
 4.1.12.Final
 1.0.5
@@ -114,7 +114,7 @@ flexible messaging model and an intuitive client 
API.
 1.7.17
 0.0.23
 1.8.9
-5.5.5
+5.8.6
 1.7.25
 1.55
   


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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