[GitHub] lucperkins opened a new pull request #920: Provide an Ansible playbook for AWS with documentation (WIP)
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)
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)
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)
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)
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
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)
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
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
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
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
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
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
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
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
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
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
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)
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)
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)
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)
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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