Re: [Discuss] Create new issues to SDKs in different languages
+1. Great idea. I wonder if we can create issue in client repo automatically with bots for PRs labelled"component/client" in pulsar repo. This would save the extra effort for the reviewer. Thanks, Haiting Jiang On 2022/01/12 03:45:18 "r...@apache.org" wrote: > Hello everyone: > > At present, all our PIP and related function changes are mainly in the Java > language, and all new functions will be merged into the Java SDK first, but > for SDKs in other languages, this is completely a black box, they don't > know what changes or optimizations have been made on the Java SDK side. > > The most typical problem is that when users of other languages encounter > various problems during use, when the maintainers of other languages want > to fix these problems, we do not know that the Java SDK side has made these > changes. Therefore, every current solution is to constantly check where the > gap of the current Java SDK is, which brings great challenges to the > maintainers themselves. > > So here is an idea, when the committters/PMC responsible for reviewing the > Java SDK can do more to help evaluate whether these PIPs or new changes > need to support this function in other languages, and then the > corresponding issue is created in the corresponding SDK, so that it is > convenient for the maintainers of other language SDKs to further evaluate > the priority of this function, and it can also attract more contributors > who are good at certain languages to claim the corresponding issue and > contribute the corresponding function. > > -- > Thanks > Xiaolong Ran >
[GitHub] [pulsar-helm-chart] lhotari closed issue #197: -Dlog4j2.formatMsgNoLookups=true not set for pulsar manager
lhotari closed issue #197: URL: https://github.com/apache/pulsar-helm-chart/issues/197 -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-helm-chart] lhotari merged pull request #198: Added -Dlog4j2.formatMsgNoLookups=true to PULSAR_MANAGER_OPTS
lhotari merged pull request #198: URL: https://github.com/apache/pulsar-helm-chart/pull/198 -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-helm-chart] lhotari opened a new pull request #199: Fix chart releaser command which uses docker image
lhotari opened a new pull request #199: URL: https://github.com/apache/pulsar-helm-chart/pull/199 ### Motivation - chart releaser command broker after upgrading to 1.3.0 version by #192 - [example failure](https://github.com/apache/pulsar-helm-chart/runs/4785990468?check_suite_focus=true#step:6:28): ``` Error: unknown command "cr" for "cr" Run 'cr --help' for usage. ``` ### Modifications - remove `cr` from command since it's part of the docker entrypoint for the `quay.io/helmpack/chart-releaser:v1.3.0` image. - `quay.io/helmpack/chart-releaser:v1.0.0-beta.1` image didn't have `cr` as the entrypoint -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [Discuss] Create new issues to SDKs in different languages
+1. I agree with Haiting Jiang. The automatic process is a better way to solve this problem. Thanks, Mattison chao
[GitHub] [pulsar-helm-chart] lhotari merged pull request #199: Fix chart releaser command which uses docker image
lhotari merged pull request #199: URL: https://github.com/apache/pulsar-helm-chart/pull/199 -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-helm-chart] lhotari opened a new pull request #200: Use appVersion as default tag for Pulsar images
lhotari opened a new pull request #200: URL: https://github.com/apache/pulsar-helm-chart/pull/200 ### Motivation There was a suggestion [in a dev mailing list discussion](https://lists.apache.org/thread/bgkvcyt1qq6h67p2k8xwp89xlncbqn3d) that the Helm chart's appVersion should be used as the default image tag. ### Additional context There are some limitations in Helm. It is not possible to set "appVersion" from the command line. There's in an open feature request https://github.com/helm/helm/issues/8194 to add such a feature to Helm. ### Modifications - change default values.yaml and set the tags for the images that use the Pulsar image to an empty value - add "defaultPulsarImageTag" to values.yaml - add a helper template "pulsar.imageFullName" that contains the logic to fall back to .Values.defaultPulsarImageTag and if it's not set, falling back to .Chart.AppVersion - use the helper template in all other templates that require the logic -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: upcoming change: Apache Pulsar Helm Chart switching from Pulsar 2.7.4 version to 2.8.2; known issue with ZK when TLS is enabled
Hi Sijie, Thanks for the suggestions. > That means: > > > 1. We should have a separate `version` from `appVersion`. > > 2. We should use the Pulsar image version as the `appVersion`. > > 3. It is okay to only update broker and proxy images version and leave > zookeeper and bookkeeper version unchanged. > I believe 1.) is already how we handle apache/pulsar-helm-chart. version of the chart is independent of appVersion. For 2.) I have created https://github.com/apache/pulsar-helm-chart/pull/200 . @si...@apache.org is that what you meant? Please review the PR. 3.) I guess this is more about addition documentation? Are there any changes that need to be made in the Apache Pulsar Helm Chart to support this? BR, Lari On Fri, Jan 7, 2022 at 2:58 AM Sijie Guo wrote: > The fundamental problem I see is that we don't have a proper helm chart > release and we don't have good versioning guidance. > > Chart should have its own version, which is independent of the Pulsar > version. > > Also, we need to provide a guide to the community - most of the time, you > don't need to upgrade zookeeper and bookkeeper. Because these two > components are rarely changed. > > That means: > > 1. We should have a separate `version` from `appVersion`. > 2. We should use the Pulsar image version as the `appVersion`. > 3. It is okay to only update broker and proxy images version and leave > zookeeper and bookkeeper version unchanged. > > That's probably not the final guide. But at least, we can have something to > get started to formalize the process. This guide will then help us handle > such situations better. > > - Sijie > > On Wed, Jan 5, 2022 at 6:00 AM 陳智弘 wrote: > > > Hi everyone, > > > > From my side, I think currently merging the request without regarding > the > > known issue and announcing this information on the website is a good > > option. > > > > < > > > http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail > > > > > 不含病毒。www.avg.com > > < > > > http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail > > > > > <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2> > > > > Lari Hotari 於 2022年1月5日 週三 下午9:04寫道： > > > > > Reminder: we need to decide about the developement of Apache Pulsar > Helm > > > chart. > > > Please reply to the email or review > > > https://github.com/apache/pulsar-helm-chart/pull/190 . > > > That PR is blocked since a decision must be made whether it's fine to > > make > > > the change, although there's a known issue in Zookeeper when TLS is > > > enabled. > > > The issue is ZOOKEEPER-3988 / > > > https://github.com/apache/pulsar/issues/11070 . > > > The bug currently only impacts TLS since the change > > > https://github.com/apache/pulsar-helm-chart/pull/180 switched > > > to use NIOServerCnxnFactory for Zookeeper. NIOServerCnxnFactory doesn't > > > support TLS and the impacted NettyServerCnxnFactory must be used for > TLS. > > > > > > How do we handle the decision? Can we proceed in merging > > > https://github.com/apache/pulsar-helm-chart/pull/190 regardless of the > > > known issue? > > > > > > BR, > > > Lari > > > > > > On Mon, Jan 3, 2022 at 4:39 PM Lari Hotari wrote: > > > > > > > Hi all, > > > > > > > > There's an upcoming change in the Apache Pulsar Helm chart to finally > > > > switch to Pulsar 2.8.x, more specifically to Apache Pulsar version > > 2.8.2 > > > . > > > > The latest Apache Pulsar Helm Chart release uses the Apache Pulsar > > 2.7.4 > > > > image. > > > > > > > > The pull request to switch to Apache Pulsar image version 2.8.2 is > > > current > > > > in review: > > > > https://github.com/apache/pulsar-helm-chart/pull/190 > > > > > > > > There's a known issue that Zookeeper TLS isn't stable because of > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-3988 (also reported > in > > > > apache/pulsar as https://github.com/apache/pulsar/issues/11070) . > > > > The fix https://github.com/apache/zookeeper/pull/1770 is planned for > > > > Zookeeper 3.7.1 version. > > > > There's a workaround in the Apache Pulsar Helm chart when TLS isn't > > > > enabled for Zookeeper. That was added by > > > > https://github.com/apache/pulsar-helm-chart/pull/180 . > > > > However, the workaround cannot be applied when TLS is enabled for > > > > Zookeeper. > > > > > > > > Should we postpone switching to Apache Pulsar 2.8.2 in the Helm chart > > > > until there's a fix for ZOOKEEPER-3988 / > > > > https://github.com/apache/pulsar/issues/11070 ? > > > > > > > > BR, > > > > Lari > > > > > > > > > >
Re: [Discuss] Create new issues to SDKs in different languages
+1 > I wonder if we can create issue in client repo automatically with bots for > PRs labelled"component/client" in pulsar repo. > This would save the extra effort for the reviewer. But there are many PRs with "component/client" label that are specific to java client changes. I think these should not be added to other clients' repos. On Wed, Jan 12, 2022 at 4:18 PM Haiting Jiang wrote: > > +1. Great idea. > > I wonder if we can create issue in client repo automatically with bots for > PRs labelled"component/client" in pulsar repo. > This would save the extra effort for the reviewer. > > Thanks, > Haiting Jiang > > On 2022/01/12 03:45:18 "r...@apache.org" wrote: > > Hello everyone: > > > > At present, all our PIP and related function changes are mainly in the Java > > language, and all new functions will be merged into the Java SDK first, but > > for SDKs in other languages, this is completely a black box, they don't > > know what changes or optimizations have been made on the Java SDK side. > > > > The most typical problem is that when users of other languages encounter > > various problems during use, when the maintainers of other languages want > > to fix these problems, we do not know that the Java SDK side has made these > > changes. Therefore, every current solution is to constantly check where the > > gap of the current Java SDK is, which brings great challenges to the > > maintainers themselves. > > > > So here is an idea, when the committters/PMC responsible for reviewing the > > Java SDK can do more to help evaluate whether these PIPs or new changes > > need to support this function in other languages, and then the > > corresponding issue is created in the corresponding SDK, so that it is > > convenient for the maintainers of other language SDKs to further evaluate > > the priority of this function, and it can also attract more contributors > > who are good at certain languages to claim the corresponding issue and > > contribute the corresponding function. > > > > -- > > Thanks > > Xiaolong Ran > > -- Zike Yang
[GitHub] [pulsar-helm-chart] dragonls commented on a change in pull request #155: Add grafana data persistence support.
dragonls commented on a change in pull request #155:
URL: https://github.com/apache/pulsar-helm-chart/pull/155#discussion_r783057866
##
File path: charts/pulsar/templates/grafana-pvc.yaml
##
@@ -0,0 +1,40 @@
+#
+# 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.
+#
+
+{{- if or .Values.monitoring.grafana .Values.extra.monitoring }}
+{{- if and (and .Values.persistence .Values.volumes.persistence)
.Values.grafana.volumes.persistence }}
+apiVersion: v1
+kind: PersistentVolumeClaim
+metadata:
+ name: "{{ template "pulsar.fullname" . }}-{{ .Values.grafana.component }}-{{
.Values.grafana.volumes.data.name }}"
+ namespace: {{ template "pulsar.namespace" . }}
+spec:
+ resources:
+requests:
+ storage: {{ .Values.grafana.volumes.data.size }}
+ accessModes: [ "ReadWriteOnce" ]
+{{- if .Values.grafana.volumes.data.storageClassName }}
+ storageClassName: "{{ .Values.grafana.volumes.data.storageClassName }}"
+{{- else if and (not (and .Values.volumes.local_storage
.Values.grafana.volumes.data.local_storage))
.Values.grafana.volumes.data.storageClass }}
+ storageClassName: "{{ template "pulsar.fullname" . }}-{{
.Values.grafana.component }}-{{ .Values.grafana.volumes.data.name }}"
Review comment:
Add creation for grafana storageclass.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar-helm-chart] dragonls commented on a change in pull request #155: Add grafana data persistence support.
dragonls commented on a change in pull request #155:
URL: https://github.com/apache/pulsar-helm-chart/pull/155#discussion_r783058040
##
File path: charts/pulsar/templates/grafana-deployment.yaml
##
@@ -88,4 +91,20 @@ spec:
name: "{{ template "pulsar.fullname" . }}-{{
.Values.grafana.component }}-secret"
key: GRAFANA_ADMIN_PASSWORD
{{- include "pulsar.imagePullSecrets" . | nindent 6}}
+ volumes:
+{{- if not (and (and .Values.persistence .Values.volumes.persistence)
.Values.grafana.volumes.persistence) }}
Review comment:
Simplified.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar-helm-chart] dragonls commented on pull request #155: Add grafana data persistence support.
dragonls commented on pull request #155: URL: https://github.com/apache/pulsar-helm-chart/pull/155#issuecomment-1011028456 Sorry for the late modification. @MarvinCai PTAL. -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[DISCUSS] Add icebox label for issues and PRs that have been inactive for more than 4 weeks
Hi Pulsar Community, I want to start a discussion about introducing an icebox label that can be added to the issue or PR by pulsar bot automatically to help us can focus on the active PRs and issue. To avoid missing merge PRs, review PRs, triage issues. It looks like the following: 1. If the issue or PR is inactive for more than 4 weeks, the pulsar bot add the icebox label 2. If the issue or PR is re-active again, the pulsar bot remove the icebox label How to determine the PR or issue is inactive? 1. No comments for 4 weeks. 2. No code review(approve, comment, or change request) for 4 weeks. 3. No commits for 4 weeks. 4. No description update for 4 weeks. How to determine the PR or issue is re-inactive? With the icebox label first and: 1. New comment added 2. New commits pushed 3. Description updated 4. New code review updates Note: all the approved PRs we should not add the icebox label This will help us to focus on the active issues and PRs so that we can track the active issues and PRs better first. After we get this part done (maybe keep active opened PR under 20 and active opened issue under 50?), we can move forward to continue to handle the stale PRs (already discussed in https://lists.apache.org/thread/k7lyw0q0fyc729w0fqlj5vqng5ny63f2). Thanks, Penghui
Re: [VOTE] PIP-132: Include message header size when check maxMessageSize of non-batch message on the client side.
+1 (non-binding)
r...@apache.org 于2022年1月12日周三 11:46写道：
> +1 (non-binding)
>
> --
> Thanks
> Xiaolong Ran
>
> mattison chao 于2022年1月12日周三 10:15写道：
>
> > +1 (non-binding)
> >
> > On Wed, 12 Jan 2022 at 09:59, Zike Yang
> > wrote:
> >
> > > +1 (non-binding)
> > >
> > > On Wed, Jan 12, 2022 at 9:58 AM Haiting Jiang >
> > > wrote:
> > > >
> > > > This is the voting thread for PIP-132. It will stay open for at least
> > 48
> > > hours.
> > > >
> > > > https://github.com/apache/pulsar/issues/13591
> > > >
> > > > Pasted below for quoting convenience.
> > > >
> > > >
> > > >
> > > > ## Motivation
> > > >
> > > > Currently, Pulsar client (Java) only checks payload size for max
> > message
> > > size validation.
> > > >
> > > > Client throws TimeoutException if we produce a message with too many
> > > properties, see [1].
> > > > But the root cause is that is trigged TooLongFrameException in broker
> > > server.
> > > >
> > > > In this PIP, I propose to include message header size when check
> > > maxMessageSize of non-batch
> > > > messages, this brings the following benefits.
> > > > 1. Clients can throw InvalidMessageException immediately if
> properties
> > > takes too much storage space.
> > > > 2. This will make the behaviour consistent with topic level max
> message
> > > size check in broker.
> > > > 3. Strictly limit the entry size less than maxMessageSize, avoid
> > sending
> > > message to bookkeeper failed.
> > > >
> > > > ## Goal
> > > >
> > > > Include message header size when check maxMessageSize for non-batch
> > > message on the client side.
> > > >
> > > > ## Implementation
> > > >
> > > > ```
> > > > // Add a size check in
> > > org.apache.pulsar.client.impl.ProducerImpl#processOpSendMsg
> > > > if (op.msg != null // for non-batch messages only
> > > > && op.getMessageHeaderAndPayloadSize() >
> > ClientCnx.getMaxMessageSize()) {
> > > > // finish send op with InvalidMessageException
> > > > releaseSemaphoreForSendOp(op);
> > > > op.sendComplete(new PulsarClientException(new
> InvalidMessageException,
> > > op.sequenceId));
> > > > }
> > > >
> > > >
> > > > //
> > >
> >
> org.apache.pulsar.client.impl.ProducerImpl.OpSendMsg#getMessageHeaderAndPayloadSize
> > > >
> > > > public int getMessageHeaderAndPayloadSize() {
> > > > ByteBuf cmdHeader = cmd.getFirst();
> > > > cmdHeader.markReaderIndex();
> > > > int totalSize = cmdHeader.readInt();
> > > > int cmdSize = cmdHeader.readInt();
> > > > int msgHeadersAndPayloadSize = totalSize - cmdSize - 4;
> > > > cmdHeader.resetReaderIndex();
> > > > return msgHeadersAndPayloadSize;
> > > > }
> > > > ```
> > > >
> > > > ## Reject Alternatives
> > > > Add a new property like "maxPropertiesSize" or "maxHeaderSize" in
> > > broker.conf and pass it to
> > > > client like maxMessageSize. But the implementation is much more
> > complex,
> > > and don't have the
> > > > benefit 2 and 3 mentioned in Motivation.
> > > >
> > > > ## Compatibility Issue
> > > > As a matter of fact, this PIP narrows down the sendable range.
> > > Previously, when maxMessageSize
> > > > is 1KB, it's ok to send message with 1KB properties and 1KB payload.
> > But
> > > with this PIP, the
> > > > sending will fail with InvalidMessageException.
> > > >
> > > > One conservative way is to add a boolean config
> > > "includeHeaderInSizeCheck" to enable this
> > > > feature. But I think it's OK to enable this directly as it's more
> > > reasonable, and I don't see good
> > > > migration plan if we add a config for this.
> > > >
> > > > [1] https://github.com/apache/pulsar/issues/13560
> > > >
> > > > Thanks,
> > > > Haiting Jiang
> > >
> > >
> > >
> > > --
> > > Zike Yang
> > >
> >
>
Re: [VOTE] PIP-132: Include message header size when check maxMessageSize of non-batch message on the client side.
+1 (non)
On Tue, Jan 11, 2022 at 9:46 PM r...@apache.org
wrote:
> +1 (non-binding)
>
> --
> Thanks
> Xiaolong Ran
>
> mattison chao 于2022年1月12日周三 10:15写道：
>
> > +1 (non-binding)
> >
> > On Wed, 12 Jan 2022 at 09:59, Zike Yang
> > wrote:
> >
> > > +1 (non-binding)
> > >
> > > On Wed, Jan 12, 2022 at 9:58 AM Haiting Jiang >
> > > wrote:
> > > >
> > > > This is the voting thread for PIP-132. It will stay open for at least
> > 48
> > > hours.
> > > >
> > > > https://github.com/apache/pulsar/issues/13591
> > > >
> > > > Pasted below for quoting convenience.
> > > >
> > > >
> > > >
> > > > ## Motivation
> > > >
> > > > Currently, Pulsar client (Java) only checks payload size for max
> > message
> > > size validation.
> > > >
> > > > Client throws TimeoutException if we produce a message with too many
> > > properties, see [1].
> > > > But the root cause is that is trigged TooLongFrameException in broker
> > > server.
> > > >
> > > > In this PIP, I propose to include message header size when check
> > > maxMessageSize of non-batch
> > > > messages, this brings the following benefits.
> > > > 1. Clients can throw InvalidMessageException immediately if
> properties
> > > takes too much storage space.
> > > > 2. This will make the behaviour consistent with topic level max
> message
> > > size check in broker.
> > > > 3. Strictly limit the entry size less than maxMessageSize, avoid
> > sending
> > > message to bookkeeper failed.
> > > >
> > > > ## Goal
> > > >
> > > > Include message header size when check maxMessageSize for non-batch
> > > message on the client side.
> > > >
> > > > ## Implementation
> > > >
> > > > ```
> > > > // Add a size check in
> > > org.apache.pulsar.client.impl.ProducerImpl#processOpSendMsg
> > > > if (op.msg != null // for non-batch messages only
> > > > && op.getMessageHeaderAndPayloadSize() >
> > ClientCnx.getMaxMessageSize()) {
> > > > // finish send op with InvalidMessageException
> > > > releaseSemaphoreForSendOp(op);
> > > > op.sendComplete(new PulsarClientException(new
> InvalidMessageException,
> > > op.sequenceId));
> > > > }
> > > >
> > > >
> > > > //
> > >
> >
> org.apache.pulsar.client.impl.ProducerImpl.OpSendMsg#getMessageHeaderAndPayloadSize
> > > >
> > > > public int getMessageHeaderAndPayloadSize() {
> > > > ByteBuf cmdHeader = cmd.getFirst();
> > > > cmdHeader.markReaderIndex();
> > > > int totalSize = cmdHeader.readInt();
> > > > int cmdSize = cmdHeader.readInt();
> > > > int msgHeadersAndPayloadSize = totalSize - cmdSize - 4;
> > > > cmdHeader.resetReaderIndex();
> > > > return msgHeadersAndPayloadSize;
> > > > }
> > > > ```
> > > >
> > > > ## Reject Alternatives
> > > > Add a new property like "maxPropertiesSize" or "maxHeaderSize" in
> > > broker.conf and pass it to
> > > > client like maxMessageSize. But the implementation is much more
> > complex,
> > > and don't have the
> > > > benefit 2 and 3 mentioned in Motivation.
> > > >
> > > > ## Compatibility Issue
> > > > As a matter of fact, this PIP narrows down the sendable range.
> > > Previously, when maxMessageSize
> > > > is 1KB, it's ok to send message with 1KB properties and 1KB payload.
> > But
> > > with this PIP, the
> > > > sending will fail with InvalidMessageException.
> > > >
> > > > One conservative way is to add a boolean config
> > > "includeHeaderInSizeCheck" to enable this
> > > > feature. But I think it's OK to enable this directly as it's more
> > > reasonable, and I don't see good
> > > > migration plan if we add a config for this.
> > > >
> > > > [1] https://github.com/apache/pulsar/issues/13560
> > > >
> > > > Thanks,
> > > > Haiting Jiang
> > >
> > >
> > >
> > > --
> > > Zike Yang
> > >
> >
>
--
Chris Herzog Messaging Team | O 630 300 7718 | M 815 263 3764 |
www.tibco.com
Re: [DISCUSS] Add icebox label for issues and PRs that have been inactive for more than 4 weeks
> On Jan 12, 2022, at 8:15 AM, PengHui Li wrote: > > Hi Pulsar Community, > > I want to start a discussion about introducing an icebox label that can be > added to > the issue or PR by pulsar bot automatically to help us can focus on the > active PRs > and issue. To avoid missing merge PRs, review PRs, triage issues. I used the "status/stale" label for some old PRs that I closed. I think that "status/inactive” would be a more descriptive label than “icebox”. > > It looks like the following: > > 1. If the issue or PR is inactive for more than 4 weeks, the pulsar bot add > the icebox label > 2. If the issue or PR is re-active again, the pulsar bot remove the icebox > label > > How to determine the PR or issue is inactive? > > 1. No comments for 4 weeks. > 2. No code review(approve, comment, or change request) for 4 weeks. > 3. No commits for 4 weeks. > 4. No description update for 4 weeks. Can the time period be made a configuration parameter to make it easy to adjust? > > How to determine the PR or issue is re-inactive? > > With the icebox label first and: > > 1. New comment added > 2. New commits pushed > 3. Description updated > 4. New code review updates > > Note: all the approved PRs we should not add the icebox label > > This will help us to focus on the active issues and PRs so that we can > track the active issues and PRs better first. After we get this part done > (maybe keep active opened PR under 20 and active opened issue under 50?), > we can move forward to continue to handle the stale PRs (already discussed > in https://lists.apache.org/thread/k7lyw0q0fyc729w0fqlj5vqng5ny63f2). Great initiative! +1 All the best, Dave > > Thanks, > Penghui
[GitHub] [pulsar-helm-chart] cogito-kyle opened a new pull request #201: Update Helpers Template, Values, and Chart version
cogito-kyle opened a new pull request #201: URL: https://github.com/apache/pulsar-helm-chart/pull/201 Fixes # ### Motivation *Explain here the context, and why you're making that change. What is the problem you're trying to solve.* ### Modifications - Added labels to all K8s Objects in chart by extending helper labels ### Verifying this change - [ ] Make sure that the change passes the CI checks. -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-helm-chart] michaeljmarshall commented on pull request #199: Fix chart releaser command which uses docker image
michaeljmarshall commented on pull request #199: URL: https://github.com/apache/pulsar-helm-chart/pull/199#issuecomment-1011251281 +1 -- 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. To unsubscribe, e-mail: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [DISCUSS] Add icebox label for issues and PRs that have been inactive for more than 4 weeks
> I used the "status/stale" label for some old PRs that I closed. I think that "status/inactive” would be a more descriptive label than “icebox”. Ok, both "status/stale" and "status/inactive” looks good. Let's use "status/inactive” > Can the time period be made a configuration parameter to make it easy to adjust? Yes, we can easy to change the CI params. Thank you Dave for the quick response. Penghui On Thu, Jan 13, 2022 at 12:48 AM Dave Fisher wrote: > > > On Jan 12, 2022, at 8:15 AM, PengHui Li wrote: > > > > Hi Pulsar Community, > > > > I want to start a discussion about introducing an icebox label that can > be > > added to > > the issue or PR by pulsar bot automatically to help us can focus on the > > active PRs > > and issue. To avoid missing merge PRs, review PRs, triage issues. > > I used the "status/stale" label for some old PRs that I closed. > > I think that "status/inactive” would be a more descriptive label than > “icebox”. > > > > > It looks like the following: > > > > 1. If the issue or PR is inactive for more than 4 weeks, the pulsar bot > add > > the icebox label > > 2. If the issue or PR is re-active again, the pulsar bot remove the > icebox > > label > > > > How to determine the PR or issue is inactive? > > > > 1. No comments for 4 weeks. > > 2. No code review(approve, comment, or change request) for 4 weeks. > > 3. No commits for 4 weeks. > > 4. No description update for 4 weeks. > > Can the time period be made a configuration parameter to make it easy to > adjust? > > > > > How to determine the PR or issue is re-inactive? > > > > With the icebox label first and: > > > > 1. New comment added > > 2. New commits pushed > > 3. Description updated > > 4. New code review updates > > > > Note: all the approved PRs we should not add the icebox label > > > > This will help us to focus on the active issues and PRs so that we can > > track the active issues and PRs better first. After we get this part done > > (maybe keep active opened PR under 20 and active opened issue under 50?), > > we can move forward to continue to handle the stale PRs (already > discussed > > in https://lists.apache.org/thread/k7lyw0q0fyc729w0fqlj5vqng5ny63f2). > > Great initiative! > > +1 > > All the best, > Dave > > > > > > Thanks, > > Penghui > >
[DISCUSS] Recent Checkstyle PRs
Hi Pulsar Community, I notice that we have had several recent PRs adding checkstyle to more of our modules: https://github.com/apache/pulsar/pull/13409 https://github.com/apache/pulsar/pull/13413 https://github.com/apache/pulsar/pull/13343 https://github.com/apache/pulsar/pull/13284 https://github.com/apache/pulsar/pull/13676 The above is an incomplete list. In order to minimize divergence for release branches and master, I think we should cherry-pick all of these PRs to our active release branches, and I propose that future CheckStyle PRs be cherry-picked to active branches when they're merged to master. At the time of writing this email, none of the above PRs have been cherry-picked yet. Let me know what you think. I am not able to do any cherry picking this week, but I might be able to help out on this task next week. Thanks, Michael
Re: [DISCUSS] Add icebox label for issues and PRs that have been inactive for more than 4 weeks
> Ok, both "status/stale" and "status/inactive” looks good. Let's use > "status/inactive” +1 - I agree with using "status/inactive" for these issues/PRs. >> Can the time period be made a configuration parameter to make it easy to adjust? >Yes, we can easy to change the CI params. I agree with setting it to 4 weeks as an initial value, and it's good that it'll be easily tunable. Overall, +1 for adding automated labeling--I think it will help reviewers prioritize which issues/PRs they review. Thanks for moving this discussion forward, Penghui. Thanks, Michael On Wed, Jan 12, 2022 at 11:04 AM PengHui Li wrote: > > > I used the "status/stale" label for some old PRs that I closed. > > I think that "status/inactive” would be a more descriptive label than > “icebox”. > > Ok, both "status/stale" and "status/inactive” looks good. Let's use > "status/inactive” > > > Can the time period be made a configuration parameter to make it easy to > adjust? > > Yes, we can easy to change the CI params. > > Thank you Dave for the quick response. > > Penghui > > On Thu, Jan 13, 2022 at 12:48 AM Dave Fisher wrote: > > > > > > On Jan 12, 2022, at 8:15 AM, PengHui Li wrote: > > > > > > Hi Pulsar Community, > > > > > > I want to start a discussion about introducing an icebox label that can > > be > > > added to > > > the issue or PR by pulsar bot automatically to help us can focus on the > > > active PRs > > > and issue. To avoid missing merge PRs, review PRs, triage issues. > > > > I used the "status/stale" label for some old PRs that I closed. > > > > I think that "status/inactive” would be a more descriptive label than > > “icebox”. > > > > > > > > It looks like the following: > > > > > > 1. If the issue or PR is inactive for more than 4 weeks, the pulsar bot > > add > > > the icebox label > > > 2. If the issue or PR is re-active again, the pulsar bot remove the > > icebox > > > label > > > > > > How to determine the PR or issue is inactive? > > > > > > 1. No comments for 4 weeks. > > > 2. No code review(approve, comment, or change request) for 4 weeks. > > > 3. No commits for 4 weeks. > > > 4. No description update for 4 weeks. > > > > Can the time period be made a configuration parameter to make it easy to > > adjust? > > > > > > > > How to determine the PR or issue is re-inactive? > > > > > > With the icebox label first and: > > > > > > 1. New comment added > > > 2. New commits pushed > > > 3. Description updated > > > 4. New code review updates > > > > > > Note: all the approved PRs we should not add the icebox label > > > > > > This will help us to focus on the active issues and PRs so that we can > > > track the active issues and PRs better first. After we get this part done > > > (maybe keep active opened PR under 20 and active opened issue under 50?), > > > we can move forward to continue to handle the stale PRs (already > > discussed > > > in https://lists.apache.org/thread/k7lyw0q0fyc729w0fqlj5vqng5ny63f2). > > > > Great initiative! > > > > +1 > > > > All the best, > > Dave > > > > > > > > > > Thanks, > > > Penghui > > > >
Re: [DISCUSS] Add icebox label for issues and PRs that have been inactive for more than 4 weeks
I am +1 with the initiative. I would like to add a suggestion, maybe I am exaggerating... Why not "closing" those PRs ? Closing a PR does not mean to delete it btw I am fine with the process you suggested Enrico Il giorno mer 12 gen 2022 alle ore 18:54 Michael Marshall ha scritto: > > > Ok, both "status/stale" and "status/inactive” looks good. Let's use > > "status/inactive” > > +1 - I agree with using "status/inactive" for these issues/PRs. > > >> Can the time period be made a configuration parameter to make it easy to > adjust? > >Yes, we can easy to change the CI params. > > I agree with setting it to 4 weeks as an initial value, and it's good > that it'll be easily tunable. > > Overall, +1 for adding automated labeling--I think it will help > reviewers prioritize which issues/PRs they review. > > Thanks for moving this discussion forward, Penghui. > > Thanks, > Michael > > On Wed, Jan 12, 2022 at 11:04 AM PengHui Li wrote: > > > > > I used the "status/stale" label for some old PRs that I closed. > > > > I think that "status/inactive” would be a more descriptive label than > > “icebox”. > > > > Ok, both "status/stale" and "status/inactive” looks good. Let's use > > "status/inactive” > > > > > Can the time period be made a configuration parameter to make it easy to > > adjust? > > > > Yes, we can easy to change the CI params. > > > > Thank you Dave for the quick response. > > > > Penghui > > > > On Thu, Jan 13, 2022 at 12:48 AM Dave Fisher wrote: > > > > > > > > > On Jan 12, 2022, at 8:15 AM, PengHui Li wrote: > > > > > > > > Hi Pulsar Community, > > > > > > > > I want to start a discussion about introducing an icebox label that can > > > be > > > > added to > > > > the issue or PR by pulsar bot automatically to help us can focus on the > > > > active PRs > > > > and issue. To avoid missing merge PRs, review PRs, triage issues. > > > > > > I used the "status/stale" label for some old PRs that I closed. > > > > > > I think that "status/inactive” would be a more descriptive label than > > > “icebox”. > > > > > > > > > > > It looks like the following: > > > > > > > > 1. If the issue or PR is inactive for more than 4 weeks, the pulsar bot > > > add > > > > the icebox label > > > > 2. If the issue or PR is re-active again, the pulsar bot remove the > > > icebox > > > > label > > > > > > > > How to determine the PR or issue is inactive? > > > > > > > > 1. No comments for 4 weeks. > > > > 2. No code review(approve, comment, or change request) for 4 weeks. > > > > 3. No commits for 4 weeks. > > > > 4. No description update for 4 weeks. > > > > > > Can the time period be made a configuration parameter to make it easy to > > > adjust? > > > > > > > > > > > How to determine the PR or issue is re-inactive? > > > > > > > > With the icebox label first and: > > > > > > > > 1. New comment added > > > > 2. New commits pushed > > > > 3. Description updated > > > > 4. New code review updates > > > > > > > > Note: all the approved PRs we should not add the icebox label > > > > > > > > This will help us to focus on the active issues and PRs so that we can > > > > track the active issues and PRs better first. After we get this part > > > > done > > > > (maybe keep active opened PR under 20 and active opened issue under > > > > 50?), > > > > we can move forward to continue to handle the stale PRs (already > > > discussed > > > > in https://lists.apache.org/thread/k7lyw0q0fyc729w0fqlj5vqng5ny63f2). > > > > > > Great initiative! > > > > > > +1 > > > > > > All the best, > > > Dave > > > > > > > > > > > > > > Thanks, > > > > Penghui > > > > > >
Re: [DISCUSSION] PIP-135: Include MetadataStore backend for Etcd
+1 陳智弘 于2022年1月12日周三 10:19写道： > +1 > > Haiting Jiang 於 2022年1月12日 週三 09:50 寫道： > > > +1 > > > > On 2022/01/12 01:44:21 PengHui Li wrote: > > > +1 > > > > > > Penghui > > > > > > On Wed, Jan 12, 2022 at 8:39 AM mattison chao > > > wrote: > > > > > > > +1 > > > > > > > > On Wed, 12 Jan 2022 at 08:09, Matteo Merli > wrote: > > > > > > > > > https://github.com/apache/pulsar/issues/13717 > > > > > > > > > > - > > > > > > > > > > ## Motivation > > > > > > > > > > Since all the pieces that composed the proposal in PIP-45 were > > finally > > > > > merged > > > > > and are currently ready for 2.10 release, it is now possible to add > > other > > > > > metadata backends that can be used to support a BookKeeper + Pulsar > > > > > cluster. > > > > > > > > > > One of the popular systems that is most commonly used as an > > alternative > > > > of > > > > > ZooKeeper is Etcd, thus it makes sense to have this as the first > > > > > non-zookeeper > > > > > implementation. > > > > > > > > > > ## Goal > > > > > > > > > > Provide an Etcd implementation for the `MetadataStore` API. This > will > > > > allow > > > > > users to deploy Pulsar clusters using Etcd service for the metadata > > and > > > > it > > > > > will > > > > > not require the presence of ZooKeeper. > > > > > > > > > > > > > > > ## Implementation > > > > > > > > > > * Use the existing JEtcd Java client library for Etcd > > > > > * Extends the `AbstractBatchedMetadataStore` class, in order to > > reuse > > > > the > > > > >transparent batching logic that will be shared with the > ZooKeeper > > > > >implementation. > > > > > > > > > > Work in progress: https://github.com/apache/pulsar/pull/13225 > > > > > > > > > > > > > > > > > > > > -- > > > > > Matteo Merli > > > > > > > > > > > > > > > > > > > >
Re: [VOTE] PIP-122: Change loadBalancer default loadSheddingStrategy to ThresholdShedder
+1 Michael Marshall 于2022年1月12日周三 13:37写道： > +1 - assuming we ensure that the `ThresholdShedder` has unit test coverage. > > Thanks, > Michael > > > On Tue, Jan 11, 2022 at 9:53 PM r...@apache.org > wrote: > > > > +1 (non-binding) > > > > -- > > > > Thanks > > Xiaolong Ran > > > > Haiting Jiang 于2022年1月12日周三 09:52写道： > > > > > +1 > > > > > > On 2022/01/10 06:47:44 Hang Chen wrote: > > > > This is the voting thread for PIP-122. It will stay open for at > least 48 > > > > hours. > > > > > > > > https://github.com/apache/pulsar/issues/13340 > > > > > > > > Pasted below for quoting convenience. > > > > > > > > > > > > > > > > ### Motivation > > > > The ThresholdShedder load balance policy since Pulsar 2.6.0 by > > > > https://github.com/apache/pulsar/pull/6772. It can resolve many load > > > > balance issues of `OverloadShedder` and works well in many Pulsar > > > > production clusters. > > > > > > > > In Pulsar 2.6.0, 2.7.0, 2.8.0 and 2.9.0, Pulsar's default load > balance > > > > policy is `OverloadShedder`. > > > > > > > > I think it's a good time for 2.10 to change default load balance > > > > policy to `ThresholdShedder`, it will make throughput more balance > > > > between brokers. > > > > > > > > ### Proposed Changes > > > > In 2.10 release，for `broker.conf`, change > > > > `loadBalancerLoadSheddingStrategy` from > > > > `org.apache.pulsar.broker.loadbalance.impl.OverloadShedder` to > > > > `org.apache.pulsar.broker.loadbalance.impl.ThresholdShedder` > > > > > > > >
Re: [DISCUSS] Recent Checkstyle PRs
Hi all, Not all modules enable the checkstyle. I think we need to make sure the behavior is consistent across all modules. On Wed, Jan 12, 2022 at 9:42 AM Michael Marshall wrote: > Hi Pulsar Community, > > I notice that we have had several recent PRs adding checkstyle to more > of our modules: > > https://github.com/apache/pulsar/pull/13409 > https://github.com/apache/pulsar/pull/13413 > https://github.com/apache/pulsar/pull/13343 > https://github.com/apache/pulsar/pull/13284 > https://github.com/apache/pulsar/pull/13676 > > The above is an incomplete list. > > In order to minimize divergence for release branches and master, I > think we should cherry-pick all of these PRs to our active release > branches, and I propose that future CheckStyle PRs be cherry-picked to > active branches when they're merged to master. > > At the time of writing this email, none of the above PRs have been > cherry-picked yet. > > Let me know what you think. I am not able to do any cherry picking > this week, but I might be able to help out on this task next week. > > Thanks, > Michael > -- Best Regards, Neng
Re: [DISCUSS] Recent Checkstyle PRs
I have no objection to the motivation. Just one thing is the PR changed many files, I guess we will get many conflicts there. With a few conflicts, we can handle them confidently and submit them directly to branches. If there are many conflicts, I would suggest creating PR direct to the branch so that we can have more eyes on the change. Regards, Penghui On Thu, Jan 13, 2022 at 9:56 AM Neng Lu wrote: > Hi all, > > Not all modules enable the checkstyle. > I think we need to make sure the behavior is consistent across all modules. > > On Wed, Jan 12, 2022 at 9:42 AM Michael Marshall > wrote: > > > Hi Pulsar Community, > > > > I notice that we have had several recent PRs adding checkstyle to more > > of our modules: > > > > https://github.com/apache/pulsar/pull/13409 > > https://github.com/apache/pulsar/pull/13413 > > https://github.com/apache/pulsar/pull/13343 > > https://github.com/apache/pulsar/pull/13284 > > https://github.com/apache/pulsar/pull/13676 > > > > The above is an incomplete list. > > > > In order to minimize divergence for release branches and master, I > > think we should cherry-pick all of these PRs to our active release > > branches, and I propose that future CheckStyle PRs be cherry-picked to > > active branches when they're merged to master. > > > > At the time of writing this email, none of the above PRs have been > > cherry-picked yet. > > > > Let me know what you think. I am not able to do any cherry picking > > this week, but I might be able to help out on this task next week. > > > > Thanks, > > Michael > > > > > -- > Best Regards, > Neng >
Re: [VOTE] PIP-122: Change loadBalancer default loadSheddingStrategy to ThresholdShedder
+1 Penghui On Thu, Jan 13, 2022 at 7:54 AM Aloys Zhang wrote: > +1 > > Michael Marshall 于2022年1月12日周三 13:37写道： > > > +1 - assuming we ensure that the `ThresholdShedder` has unit test > coverage. > > > > Thanks, > > Michael > > > > > > On Tue, Jan 11, 2022 at 9:53 PM r...@apache.org > > > wrote: > > > > > > +1 (non-binding) > > > > > > -- > > > > > > Thanks > > > Xiaolong Ran > > > > > > Haiting Jiang 于2022年1月12日周三 09:52写道： > > > > > > > +1 > > > > > > > > On 2022/01/10 06:47:44 Hang Chen wrote: > > > > > This is the voting thread for PIP-122. It will stay open for at > > least 48 > > > > > hours. > > > > > > > > > > https://github.com/apache/pulsar/issues/13340 > > > > > > > > > > Pasted below for quoting convenience. > > > > > > > > > > > > > > > > > > > > ### Motivation > > > > > The ThresholdShedder load balance policy since Pulsar 2.6.0 by > > > > > https://github.com/apache/pulsar/pull/6772. It can resolve many > load > > > > > balance issues of `OverloadShedder` and works well in many Pulsar > > > > > production clusters. > > > > > > > > > > In Pulsar 2.6.0, 2.7.0, 2.8.0 and 2.9.0, Pulsar's default load > > balance > > > > > policy is `OverloadShedder`. > > > > > > > > > > I think it's a good time for 2.10 to change default load balance > > > > > policy to `ThresholdShedder`, it will make throughput more balance > > > > > between brokers. > > > > > > > > > > ### Proposed Changes > > > > > In 2.10 release，for `broker.conf`, change > > > > > `loadBalancerLoadSheddingStrategy` from > > > > > `org.apache.pulsar.broker.loadbalance.impl.OverloadShedder` to > > > > > `org.apache.pulsar.broker.loadbalance.impl.ThresholdShedder` > > > > > > > > > > > >
Re: [VOTE] PIP-132: Include message header size when check maxMessageSize of non-batch message on the client side.
+1 (binding)
This is a behavior change, which we should highlight in the release note.
Penghui
On Thu, Jan 13, 2022 at 12:44 AM Chris Herzog
wrote:
> +1 (non)
>
>
> On Tue, Jan 11, 2022 at 9:46 PM r...@apache.org
> wrote:
>
> > +1 (non-binding)
> >
> > --
> > Thanks
> > Xiaolong Ran
> >
> > mattison chao 于2022年1月12日周三 10:15写道：
> >
> > > +1 (non-binding)
> > >
> > > On Wed, 12 Jan 2022 at 09:59, Zike Yang .invalid>
> > > wrote:
> > >
> > > > +1 (non-binding)
> > > >
> > > > On Wed, Jan 12, 2022 at 9:58 AM Haiting Jiang <
> jianghait...@apache.org
> > >
> > > > wrote:
> > > > >
> > > > > This is the voting thread for PIP-132. It will stay open for at
> least
> > > 48
> > > > hours.
> > > > >
> > > > > https://github.com/apache/pulsar/issues/13591
> > > > >
> > > > > Pasted below for quoting convenience.
> > > > >
> > > > >
> > > > >
> > > > > ## Motivation
> > > > >
> > > > > Currently, Pulsar client (Java) only checks payload size for max
> > > message
> > > > size validation.
> > > > >
> > > > > Client throws TimeoutException if we produce a message with too
> many
> > > > properties, see [1].
> > > > > But the root cause is that is trigged TooLongFrameException in
> broker
> > > > server.
> > > > >
> > > > > In this PIP, I propose to include message header size when check
> > > > maxMessageSize of non-batch
> > > > > messages, this brings the following benefits.
> > > > > 1. Clients can throw InvalidMessageException immediately if
> > properties
> > > > takes too much storage space.
> > > > > 2. This will make the behaviour consistent with topic level max
> > message
> > > > size check in broker.
> > > > > 3. Strictly limit the entry size less than maxMessageSize, avoid
> > > sending
> > > > message to bookkeeper failed.
> > > > >
> > > > > ## Goal
> > > > >
> > > > > Include message header size when check maxMessageSize for non-batch
> > > > message on the client side.
> > > > >
> > > > > ## Implementation
> > > > >
> > > > > ```
> > > > > // Add a size check in
> > > > org.apache.pulsar.client.impl.ProducerImpl#processOpSendMsg
> > > > > if (op.msg != null // for non-batch messages only
> > > > > && op.getMessageHeaderAndPayloadSize() >
> > > ClientCnx.getMaxMessageSize()) {
> > > > > // finish send op with InvalidMessageException
> > > > > releaseSemaphoreForSendOp(op);
> > > > > op.sendComplete(new PulsarClientException(new
> > InvalidMessageException,
> > > > op.sequenceId));
> > > > > }
> > > > >
> > > > >
> > > > > //
> > > >
> > >
> >
> org.apache.pulsar.client.impl.ProducerImpl.OpSendMsg#getMessageHeaderAndPayloadSize
> > > > >
> > > > > public int getMessageHeaderAndPayloadSize() {
> > > > > ByteBuf cmdHeader = cmd.getFirst();
> > > > > cmdHeader.markReaderIndex();
> > > > > int totalSize = cmdHeader.readInt();
> > > > > int cmdSize = cmdHeader.readInt();
> > > > > int msgHeadersAndPayloadSize = totalSize - cmdSize - 4;
> > > > > cmdHeader.resetReaderIndex();
> > > > > return msgHeadersAndPayloadSize;
> > > > > }
> > > > > ```
> > > > >
> > > > > ## Reject Alternatives
> > > > > Add a new property like "maxPropertiesSize" or "maxHeaderSize" in
> > > > broker.conf and pass it to
> > > > > client like maxMessageSize. But the implementation is much more
> > > complex,
> > > > and don't have the
> > > > > benefit 2 and 3 mentioned in Motivation.
> > > > >
> > > > > ## Compatibility Issue
> > > > > As a matter of fact, this PIP narrows down the sendable range.
> > > > Previously, when maxMessageSize
> > > > > is 1KB, it's ok to send message with 1KB properties and 1KB
> payload.
> > > But
> > > > with this PIP, the
> > > > > sending will fail with InvalidMessageException.
> > > > >
> > > > > One conservative way is to add a boolean config
> > > > "includeHeaderInSizeCheck" to enable this
> > > > > feature. But I think it's OK to enable this directly as it's more
> > > > reasonable, and I don't see good
> > > > > migration plan if we add a config for this.
> > > > >
> > > > > [1] https://github.com/apache/pulsar/issues/13560
> > > > >
> > > > > Thanks,
> > > > > Haiting Jiang
> > > >
> > > >
> > > >
> > > > --
> > > > Zike Yang
> > > >
> > >
> >
>
>
> --
>
>
> Chris Herzog Messaging Team | O 630 300 7718 | M 815 263 3764 |
> www.tibco.com
>
>
Re: [Discuss] Create new issues to SDKs in different languages
I'm not sure if the bots can detect if the change is a Java client change, maybe based on the changes introduced in which directory. The main headache here is missing it. If there are some mechanisms that can remind us. It will be great. Looks like "hey, new changes introduced in Java client, you might need to create an issue to other clients repos" Use a label to allow the bots to sync to other repos LGTM here, we can run it manually first. So that we can know more clearly if we need automatic synchronization. Thanks, Penghui On Wed, Jan 12, 2022 at 9:06 PM Zike Yang wrote: > +1 > > > I wonder if we can create issue in client repo automatically with bots > for PRs labelled"component/client" in pulsar repo. > > This would save the extra effort for the reviewer. > > But there are many PRs with "component/client" label that are specific > to java client changes. I think these should not be added to other > clients' repos. > > > > On Wed, Jan 12, 2022 at 4:18 PM Haiting Jiang > wrote: > > > > +1. Great idea. > > > > I wonder if we can create issue in client repo automatically with bots > for PRs labelled"component/client" in pulsar repo. > > This would save the extra effort for the reviewer. > > > > Thanks, > > Haiting Jiang > > > > On 2022/01/12 03:45:18 "r...@apache.org" wrote: > > > Hello everyone: > > > > > > At present, all our PIP and related function changes are mainly in the > Java > > > language, and all new functions will be merged into the Java SDK > first, but > > > for SDKs in other languages, this is completely a black box, they don't > > > know what changes or optimizations have been made on the Java SDK side. > > > > > > The most typical problem is that when users of other languages > encounter > > > various problems during use, when the maintainers of other languages > want > > > to fix these problems, we do not know that the Java SDK side has made > these > > > changes. Therefore, every current solution is to constantly check > where the > > > gap of the current Java SDK is, which brings great challenges to the > > > maintainers themselves. > > > > > > So here is an idea, when the committters/PMC responsible for reviewing > the > > > Java SDK can do more to help evaluate whether these PIPs or new changes > > > need to support this function in other languages, and then the > > > corresponding issue is created in the corresponding SDK, so that it is > > > convenient for the maintainers of other language SDKs to further > evaluate > > > the priority of this function, and it can also attract more > contributors > > > who are good at certain languages to claim the corresponding issue and > > > contribute the corresponding function. > > > > > > -- > > > Thanks > > > Xiaolong Ran > > > > > > > -- > Zike Yang >
Re: [PR] Pulsar non root docker image
I am not sure if the fix in https://github.com/apache/pulsar/commit/04b5da0f95794259694cc781e8960b7e52fac06b is sufficient. I would like to see there is at least one integration test that covers running functions on k8s to ensure we don't break the basic stuff. - Sijie On Fri, Jan 7, 2022 at 9:58 PM Michael Marshall wrote: > > Is Functions being verified? > > I discussed this a bit in PR 13376's description and comments, please > let me know if you have additional questions. I haven't done any extra > verification. > > Note that since [0] is in both 2.8.x and 2.9.x, the upgrade scenario > that led us to revert the first non-root work is no longer an issue. > > I plan to follow up with a PR to the Pulsar Helm Chart to make the > fsGroup configurable. > > - Michael > > [0] > https://github.com/apache/pulsar/commit/04b5da0f95794259694cc781e8960b7e52fac06b > > On Thu, Jan 6, 2022 at 7:00 PM Sijie Guo wrote: > > > > Is Functions being verified? > > > > - Sijie > > > > On Wed, Jan 5, 2022 at 11:26 AM Michael Marshall > > wrote: > > > > > PR 13376 is ready for review, PTAL. > > > > > > What approach should we take regarding docker image size? > > > I propose providing a minimal image along with documentation > > > on how to add your own debugging tools. Is that sufficient? > > > > > > I'd like to include this feature in 2.10.0. > > > > > > Note that you can test the new docker image here: > > > michaelmarshall/pulsar:2.10.0-SNAPSHOT-1dec8b9 > > > > > > Thanks, > > > Michael > > > > > > > > > > > > On Wed, Dec 22, 2021 at 1:51 PM Michael Marshall > > > > wrote: > > > > > > > > Thanks for raising this important topic, Enrico. > > > > > > > > > Basically if you are running as non root, you cannot add tools on > > > demand, > > > > > so we need to add basic tools, like netstat/vim/unzip otherwise > > > when > > > > > you have a problem you are trapped. > > > > > > > > I agree that running as a non root user presents challenges for > > > > debugging. However, I don't think we should optimize our production > > > > image for debugging. We should instead optimize for a minimal docker > > > > image with as few dependencies as possible to decrease the image's > > > > attack surface. > > > > > > > > Also, I think it would be valuable to audit the current > > > > dependencies we already ship in our docker image. For example, this > > > > single `RUN` command [0] adds 1 GB of dependencies to our > > > > docker image. > > > > > > > > If there is a need for an image with debug tools, we could publish a > > > > "debug" image that extends our production image with extra tooling. > > > > Users could swap out the prod image with the debug image, as needed. > > > > However, it is pretty easy and quick to extend a public docker image > > > > to add your own tooling. By documenting how to extend our docker > > > > image, we could avoid decisions about which tools should be in our > > > > image. > > > > > > > > > there are ways to inject tools, but they are very hard to execute > for > > > > > people who is not very experienced > > > > > > > > We can solve this through additional documentation. Freeznet asked a > > > > similar question about debugging on the PR. I provided some methods > for > > > > debugging here [1]. > > > > > > > > Thanks, > > > > Michael > > > > > > > > [0] > > > > https://github.com/apache/pulsar/blob/5dd60dbd748e446f8da396b448a5bb16a2ae6902/docker/pulsar/Dockerfile#L46-L55 > > > > [1] > https://github.com/apache/pulsar/pull/13376#discussion_r773580954 > > > > > > > > > > > > > > > > > > > > On Wed, Dec 22, 2021 at 1:29 AM Enrico Olivelli > > > > wrote: > > > > > > > > > > Michael, > > > > > > > > > > I would like to add this item to the list > > > > > https://github.com/apache/pulsar/pull/10815 > > > > > > > > > > Basically if you are running as non root, you cannot add tools on > > > demand, > > > > > so we need to add basic tools, like netstat/vim/unzip otherwise > > > when > > > > > you have a problem you are trapped. > > > > > > > > > > there are ways to inject tools, but they are very hard to execute > for > > > > > people who is not very experienced > > > > > > > > > > Enrico > > > > > > > > > > > > > > > Il giorno mer 22 dic 2021 alle ore 04:40 Haiting Jiang < > > > > > jianghait...@apache.org> ha scritto: > > > > > > > > > > > > 1. Pulsar configuration is read in only from configuration > files in > > > > > > > `/pulsar/conf`. A non root user must be able to update these > files > > > to > > > > > > > have run with custom configuration. > > > > > > > > > > > > About the configurations, I also see similar require like this > > > lately [0]. > > > > > > IMHO, update any configs without redeploy service is a useful > > > feature. > > > > > > I would like post a PIP for this later. > > > > > > Basic idea is like make all configs dynamic by default except > > > > > > `metadataStoreUrl` and all configs are stored under path > > > > > > "/admin/configuration" in metadata store. > > > > > > > >
Re: [DISCUSSION] PIP-135: Include MetadataStore backend for Etcd
+1 On Wed, Jan 12, 2022 at 3:52 PM Aloys Zhang wrote: > +1 > > 陳智弘 于2022年1月12日周三 10:19写道： > > > +1 > > > > Haiting Jiang 於 2022年1月12日 週三 09:50 寫道： > > > > > +1 > > > > > > On 2022/01/12 01:44:21 PengHui Li wrote: > > > > +1 > > > > > > > > Penghui > > > > > > > > On Wed, Jan 12, 2022 at 8:39 AM mattison chao < > mattisonc...@gmail.com> > > > > wrote: > > > > > > > > > +1 > > > > > > > > > > On Wed, 12 Jan 2022 at 08:09, Matteo Merli > > wrote: > > > > > > > > > > > https://github.com/apache/pulsar/issues/13717 > > > > > > > > > > > > - > > > > > > > > > > > > ## Motivation > > > > > > > > > > > > Since all the pieces that composed the proposal in PIP-45 were > > > finally > > > > > > merged > > > > > > and are currently ready for 2.10 release, it is now possible to > add > > > other > > > > > > metadata backends that can be used to support a BookKeeper + > Pulsar > > > > > > cluster. > > > > > > > > > > > > One of the popular systems that is most commonly used as an > > > alternative > > > > > of > > > > > > ZooKeeper is Etcd, thus it makes sense to have this as the first > > > > > > non-zookeeper > > > > > > implementation. > > > > > > > > > > > > ## Goal > > > > > > > > > > > > Provide an Etcd implementation for the `MetadataStore` API. This > > will > > > > > allow > > > > > > users to deploy Pulsar clusters using Etcd service for the > metadata > > > and > > > > > it > > > > > > will > > > > > > not require the presence of ZooKeeper. > > > > > > > > > > > > > > > > > > ## Implementation > > > > > > > > > > > > * Use the existing JEtcd Java client library for Etcd > > > > > > * Extends the `AbstractBatchedMetadataStore` class, in order to > > > reuse > > > > > the > > > > > >transparent batching logic that will be shared with the > > ZooKeeper > > > > > >implementation. > > > > > > > > > > > > Work in progress: https://github.com/apache/pulsar/pull/13225 > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Matteo Merli > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: About the implementation of a Mail Queue using Pulsar in Apache James
Hello Enrico, On 06/01/2022 17:07, Enrico Olivelli wrote: > Benoit, > > Il Gio 6 Gen 2022, 10:54 Benoit TELLIER ha scritto: > >> Hello Pulsar folks! >> >> I am a member of the Apache James PMC, whose maintain the Java Apache >> Mail Extensible Server (for the intimates hence James). The Distributed >> James server aims at scaling emails on top of modern databases and >> messaging technologies. >> >> So far we relies on ActiveMQ/RabbitMQ as a messaging technology (please >> don't judge us...). Features includes: >> - A mail queue (~work queue) supporting browsing, removes, delays >> - A messaging system for notifications (think IMAP IDLE spec) >> - Some smaller stuff: work-queues, ... >> >> However this matter of fact will hopefully change as we receive a >> contribution for a Mail Queue implemented with Pulsar [1]. >> > > This is great. > > I have been working on mail queue systems for years and I will be happy to > review the PR and help with this opportunity to engage with the James > community Thanks for your enthusiasm! We did put together a ticket summarizing the work remaining to be done regarding Pulsar integration with Apache James. This can be found here: https://issues.apache.org/jira/projects/JAMES/issues/JAMES-3695 . This could be a good candidate for a GSoc 2022 project in my opinion. Cheers, Benoit > > Cheers > Enrico > > >> As an Apache enthusiast, I think this could be an opportunity to >> strengthen links between our communities. Who knows, some devs within >> the Pulsar community might feel interested by this, and could share some >> interesting insights on this. The Apache James PMC is mostly >> Pulsar-naive so we might also benefit from an external eyes. In the >> future we might also propose some work (eg GSOC 2022) on further >> integrationg Pulsar into James, which could be an opportunity for >> cooperation. >> >> I apologize if this mail is of no interest to the Apache Pulsar >> community or if it is the wrong place to post this kind of stuff. >> >> Best regards, >> >> Benoit TELLIER >> >> [1] The aforementioned Pulsar PR : >> https://github.com/apache/james-project/pull/808 >> >>
Re: [DISCUSSION] PIP-135: Include MetadataStore backend for Etcd
+1 Enrico Il Gio 13 Gen 2022, 06:01 Joe F ha scritto: > +1 > > On Wed, Jan 12, 2022 at 3:52 PM Aloys Zhang wrote: > > > +1 > > > > 陳智弘 于2022年1月12日周三 10:19写道： > > > > > +1 > > > > > > Haiting Jiang 於 2022年1月12日 週三 09:50 寫道： > > > > > > > +1 > > > > > > > > On 2022/01/12 01:44:21 PengHui Li wrote: > > > > > +1 > > > > > > > > > > Penghui > > > > > > > > > > On Wed, Jan 12, 2022 at 8:39 AM mattison chao < > > mattisonc...@gmail.com> > > > > > wrote: > > > > > > > > > > > +1 > > > > > > > > > > > > On Wed, 12 Jan 2022 at 08:09, Matteo Merli > > > wrote: > > > > > > > > > > > > > https://github.com/apache/pulsar/issues/13717 > > > > > > > > > > > > > > - > > > > > > > > > > > > > > ## Motivation > > > > > > > > > > > > > > Since all the pieces that composed the proposal in PIP-45 were > > > > finally > > > > > > > merged > > > > > > > and are currently ready for 2.10 release, it is now possible to > > add > > > > other > > > > > > > metadata backends that can be used to support a BookKeeper + > > Pulsar > > > > > > > cluster. > > > > > > > > > > > > > > One of the popular systems that is most commonly used as an > > > > alternative > > > > > > of > > > > > > > ZooKeeper is Etcd, thus it makes sense to have this as the > first > > > > > > > non-zookeeper > > > > > > > implementation. > > > > > > > > > > > > > > ## Goal > > > > > > > > > > > > > > Provide an Etcd implementation for the `MetadataStore` API. > This > > > will > > > > > > allow > > > > > > > users to deploy Pulsar clusters using Etcd service for the > > metadata > > > > and > > > > > > it > > > > > > > will > > > > > > > not require the presence of ZooKeeper. > > > > > > > > > > > > > > > > > > > > > ## Implementation > > > > > > > > > > > > > > * Use the existing JEtcd Java client library for Etcd > > > > > > > * Extends the `AbstractBatchedMetadataStore` class, in order > to > > > > reuse > > > > > > the > > > > > > >transparent batching logic that will be shared with the > > > ZooKeeper > > > > > > >implementation. > > > > > > > > > > > > > > Work in progress: https://github.com/apache/pulsar/pull/13225 > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Matteo Merli > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSSION] PIP-135: Include MetadataStore backend for Etcd
+1 Best, Hang Enrico Olivelli 于2022年1月13日周四 14:20写道： > > +1 > > Enrico > > Il Gio 13 Gen 2022, 06:01 Joe F ha scritto: > > > +1 > > > > On Wed, Jan 12, 2022 at 3:52 PM Aloys Zhang wrote: > > > > > +1 > > > > > > 陳智弘 于2022年1月12日周三 10:19写道： > > > > > > > +1 > > > > > > > > Haiting Jiang 於 2022年1月12日 週三 09:50 寫道： > > > > > > > > > +1 > > > > > > > > > > On 2022/01/12 01:44:21 PengHui Li wrote: > > > > > > +1 > > > > > > > > > > > > Penghui > > > > > > > > > > > > On Wed, Jan 12, 2022 at 8:39 AM mattison chao < > > > mattisonc...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > +1 > > > > > > > > > > > > > > On Wed, 12 Jan 2022 at 08:09, Matteo Merli > > > > wrote: > > > > > > > > > > > > > > > https://github.com/apache/pulsar/issues/13717 > > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > > ## Motivation > > > > > > > > > > > > > > > > Since all the pieces that composed the proposal in PIP-45 were > > > > > finally > > > > > > > > merged > > > > > > > > and are currently ready for 2.10 release, it is now possible to > > > add > > > > > other > > > > > > > > metadata backends that can be used to support a BookKeeper + > > > Pulsar > > > > > > > > cluster. > > > > > > > > > > > > > > > > One of the popular systems that is most commonly used as an > > > > > alternative > > > > > > > of > > > > > > > > ZooKeeper is Etcd, thus it makes sense to have this as the > > first > > > > > > > > non-zookeeper > > > > > > > > implementation. > > > > > > > > > > > > > > > > ## Goal > > > > > > > > > > > > > > > > Provide an Etcd implementation for the `MetadataStore` API. > > This > > > > will > > > > > > > allow > > > > > > > > users to deploy Pulsar clusters using Etcd service for the > > > metadata > > > > > and > > > > > > > it > > > > > > > > will > > > > > > > > not require the presence of ZooKeeper. > > > > > > > > > > > > > > > > > > > > > > > > ## Implementation > > > > > > > > > > > > > > > > * Use the existing JEtcd Java client library for Etcd > > > > > > > > * Extends the `AbstractBatchedMetadataStore` class, in order > > to > > > > > reuse > > > > > > > the > > > > > > > >transparent batching logic that will be shared with the > > > > ZooKeeper > > > > > > > >implementation. > > > > > > > > > > > > > > > > Work in progress: https://github.com/apache/pulsar/pull/13225 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Matteo Merli > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
