Re: [DISCUSS] PIP-186: Introduce two phase deletion protocol based on system topic
Good idea. > Good idea. I suggest the naming endings with `Seconds` like > sendDelayOfTwoPhaseDeletionInSeconds,` > reconsumeLaterOfTwoPhaseDeletionInSeconds`. > >private int sendDelaySecondsOfTwoPhaseDeletion = 60; > >private int reconsumeLaterSecondsOfTwoPhaseDeletion = 600;
Re: [DISCUSS] Switch to the Temurin JDK in the Docker image
Thank Qiang for your explanation! Qiang Huang 于2022年8月14日周日 12:15写道： > LGTM. They are very similar. > The OpenJDK provides source-code, and the Temurin JDK provides builds of > the source code. > I would like to point out that they have different licenses. > - The OpenJDK implementation is licensed under the GPL-2.0-only with a > linking exception. [0] > - The Eclipse Temurin™ project provides code and processes that support the > building of runtime binaries and associated technologies that are high > performance, enterprise-caliber, cross-platform, open-source licensed, and > Java SE TCK-tested for general use across the Java ecosystem.[1] > > - [0] https://en.wikipedia.org/wiki/OpenJDK https://openjdk.org/legal/ > - [1] https://projects.eclipse.org/projects/adoptium.temurin > > Zixuan Liu 于2022年8月12日周五 15:46写道： > > > Hi tison, > > > > Great catch! > > > > The Temurin JDK is OpenJDK distribution from Adoptium, the old JDK from > > Ubuntu, they should all be built on the OpenJDK open source project, so I > > think should be fully compatible. > > > > Each Temurin release has passed the relevant Oracle Java Compatibility > Kit > > (JCK) to demonstrate that it is a compatible implementation of the Java > > specification. In addition, Temurin releases must pass the AQAvit quality > > verification suite[1] to ensure they are ready for production usage. > > > > [1] - https://adoptium.net/docs/qvs-policy > > > > Thanks, > > Zixuan > > > > > > tison 于2022年8月12日周五 15:23写道： > > > > > +1 > > > > > > Thanks for bringing this topic :) > > > > > > In Pulsar usages, these two distributions should not be quite > different. > > > Did you investigate the compatibility more? > > > > > > Best, > > > tison. > > > > > > > > > Zixuan Liu 于2022年8月12日周五 15:17写道： > > > > > > > Hi all, > > > > > > > > I noticed we are using OpenJDK in our Docker image, I suggest that we > > > > switch to the Temurin JDK, because our CI runs on the Temurin JDK, we > > > need > > > > to keep the same JDK everywhere to avoid unexpected problems. Thanks, > > > > Zixuan > > > > > > > > > > > > -- > BR, > Qiang Huang >
Re: [DISCUSS] Allow the producer to connect to the topic with producer_request_hold backlog policy
Great idea, Michael > 1. A new producer connecting to a topic that has exceeded its quota. > This case is trivial because the broker tells the producer it is > connected but it cannot send any messages (i.e. producer_ready=false), > and the client holds the producer's future until the broker sends > producer_ready=true. Agree with this method of dealing with new producers connecting to a topic that exceeds the quota, but maybe we need to add some metrics to these producers so that users can observe these producers that are connected successfully but not ready. Another concern is that if the topic is unloaded to another broker, we have to notify the client to reconnect, I'm not sure if `waitingExclusiveProducers` does that or if I missing some logic. > but it could be worth > extending the client so that an application could reactively discover > that the topic's quota is exceeded using a listener. Additionally, we > could disable the send timeouts when the producer enters this "hold" > state so that messages. It makes sense to me but looks like we will change the protocol to let the producer know why the producer does not ready. maybe need to add the type field in the `CommandProducerSuccess` command. I think based on the current logic, we can support the producer_hold policy by `producer_ready=false` first. and then draft a PIP to change the protocol to improve the logic. Best, Mattison On 2022/08/14 05:29:35 Michael Marshall wrote: > Great points, Penghui. > > > To optimize, I think we can only let the producer connect to the broker > > and the broker should tell the producer the backlog is exceeded. > > In looking at the `CommandProducerSuccess` protobuf message, we > already have the `producer_ready` boolean field. It was added for the > exclusive producer case, and it seems to match this logic exactly, > though the client wouldn't know "why" the producer was not ready. I > think this field meets our needs because the producer just needs to > know that it is connected and should not send messages yet. > > > If we have too many producers, try to reconnect to the broker again and > > again. It is also a non-negligible cost. > > This is a really important point. The current protocol implementation > leads to unnecessary network traffic, and it will be worse for > topics with many producers. Note that the lookup part of the protocol > introduces additional load on all of the brokers serving these > requests. > > > Looks like we need to fix the client-side to make sure users will not get > > ProducerBlockedQuotaExceededError when creating the producer with > > producer_hold_request backlog policy. I have tested it locally, the behavior > > can be confirmed > > Thanks for confirming this case. I think it would make sense to update > the behavior on the producer_requests_hold feature so that the future > is incomplete until the producer is ready to produce, just like the > exclusive producer implementation. > > Ultimately, there are two cases this feature needs to handle. > > 1. A new producer connecting to a topic that has exceeded its quota. > This case is trivial because the broker tells the producer it is > connected but it cannot send any messages (i.e. producer_ready=false), > and the client holds the producer's future until the broker sends > producer_ready=true. > > 2. An existing producer gets disconnected due to an exceeded quota. In > this case, it'd be easy enough for the producer to stop sending > messages, but because the client application already has a reference > to this producer, the application will be able to submit messages > until the client's buffer is full, at which point the send is blocked > or gets an exception. I think that would work, but it could be worth > extending the client so that an application could reactively discover > that the topic's quota is exceeded using a listener. Additionally, we > could disable the send timeouts when the producer enters this "hold" > state so that messages. > > In case number 2, it probably makes sense to extend the protocol so > that the broker sends a protocol message indicating the producer > should stop sending messages. This would be more elegant than > disconnecting the producer and making it look up the topic again. > > Thanks, > Michael > > > On Fri, Aug 12, 2022 at 5:50 AM PengHui Li wrote: > > > > > The producer fails > > the pending messages when the policy is producer_exception and the > > producer does nothing when the policy is producer_request_hold > > > > Eventually, it will fail [0] the user's create producer request because of > > the operation timeout [1]. > > > > > The primary advantage for this solution is that the broker does not > > need to hold a producer's messages in memory for some undefined time. > > > > Yes, I agree. And changing the protocol will also affect other clients. > > > > To optimize, I think we can only let the producer connect to the broker > > and the broker should
Re: [DISCUSS] Message redeliveryCount semantics and DLQ
I support this motivation. We should avoid any cases which might break the at-least-one delivery semantic from the user's perspective. Thanks, Penghui On Thu, Aug 11, 2022 at 1:36 PM Michael Marshall wrote: > Hi Pulsar Community, > > In Pulsar 2.5.0, the semantics for a message's redeliveryCount changed > at the request of this issue [0]. Please see the issue for relevant > context. > > In summary, the issue is whether a message that is neither positively > nor negatively acknowledged should get counted as "redelivered" when a > consumer disconnects from the broker. > > The issue asserts that because a message could lead to a fatal error > in the consumer application that could prevent negative > acknowledgement getting sent to the broker, every delivery of a > message should count towards the redeliveryCount. > > In my view, this edge case should not justify changing the > redeliveryCount semantics. It is the responsibility of the application > to handle all messages, even those that are malformed. > > My primary motivation for revisiting this design is to discuss one of > its consequences. After the change, messages that are only ever sent > to an application's receiverQueue are counted against the delivery > count used to determine whether a message ought to go to the DLQ > topic. Because the DLQ implementation uses the redeliveryCount to > determine when a message has been attempted "enough" times, there is a > chance that messages can get sent to the DLQ without ever having been > "received" by the consuming application. While this scenario is > unlikely, I think it introduces doubt into this feature's design > because it is possible for messages to get classified as dead letters > without delivery at least the maxRedeliverCount. > > Here is my PR to adopt the original behavior [1]. Please take a look. > > Thanks, > Michael > > [0] https://github.com/apache/pulsar/issues/5881 > [1] https://github.com/apache/pulsar/pull/17060 >
Re: [VOTE] Pulsar Release 2.8.4 Candidate 1
Got it. Thx. Yunze Xu 于2022年8月14日周日 23:22写道： > You can see > https://lists.apache.org/thread/rg1g083c06ozm5go6zo1jophg9y9zl2f > for more details about the LTS release. > > Thanks, > Yunze > > > > > > 2022年8月14日 11:00，Qiang Huang 写道： > > > > +1 (non-binding) > > Is 2.8.4 a long term support release? > > > > Yunze Xu 于2022年8月12日周五 16:20写道： > > > >> This is the first release candidate for Apache Pulsar, version 2.8.4. > >> > >> It fixes the following issues: > >> > >> > https://github.com/apache/pulsar/pulls?q=is%3Amerged+is%3Apr+label%3Arelease%2F2.8.4 > >> > >> *** Please download, test and vote on this release. This vote will stay > >> open > >> for at least 72 hours *** > >> > >> Note that we are voting upon the source (tag), binaries are provided for > >> convenience. > >> > >> Source and binary files: > >> https://dist.apache.org/repos/dist/dev/pulsar/pulsar-2.8.4-candidate-1/ > >> > >> SHA-512 checksums: > >> > >> > c3d26704f2cfb3365c29d4110612ca7351084f8bee3c306d5e906b3f9b22c7557cc5baf12f74f8c222baccae3310691419eda5b47fdf9cd6c5281b70134ab5eb > >> apache-pulsar-2.8.4-bin.tar.gz > >> > 28160ee94dccfb74dfb56e0e5d0e08870c6612659507333a52b5660ecbf060a75d1eed667cffd8596f9468de95055b78916b932db0e0d4c2745868d55429ee98 > >> apache-pulsar-2.8.4-src.tar.gz > >> > >> Maven staging repo: > >> > https://repository.apache.org/content/repositories/orgapachepulsar-1170/ > >> > >> The tag to be voted upon: > >> v2.8.4-candidate-1 (02ee5616866d4eda8dd94f85d9d9b71c459f248d) > >> https://github.com/apache/pulsar/releases/tag/v2.8.4-candidate-1 > >> > >> Pulsar's KEYS file containing PGP keys we use to sign the release: > >> https://dist.apache.org/repos/dist/dev/pulsar/KEYS > >> > >> Docker images: > >> > >> > >> > https://hub.docker.com/layers/pulsar/bewaremypower/pulsar/2.8.4/images/sha256-fba51a75c0f2ca79fbff7b254f80f641fcda661fd702f8149bbfdd5994078e3a > >> > >> > >> > https://hub.docker.com/layers/pulsar-all/bewaremypower/pulsar-all/2.8.4/images/sha256-42d4b41e5869edc6242bb49d6a1687bd6d191a6385637122edc5c7b2c44ee46f > >> > >> Please download the source package, and follow the Release Candidate > >> Validation[1] to validate the release > >> > >> [1] https://github.com/apache/pulsar/wiki/Release-Candidate-Validation > >> > >> Thanks, > >> Yunze > >> > >> > >> > >> > >> > > > > -- > > BR, > > Qiang Huang > > -- BR, Qiang Huang
[GitHub] [pulsar-site] SignorMercurio opened a new pull request, #161: Add execute permission on pulsar-config-doc-gen.sh
SignorMercurio opened a new pull request, #161: URL: https://github.com/apache/pulsar-site/pull/161 Add execute permission on pulsar-config-doc-gen.sh. @urfreespace -- 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-site] urfreespace merged pull request #160: Automate config docs generation
urfreespace merged PR #160: URL: https://github.com/apache/pulsar-site/pull/160 -- 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-site] urfreespace commented on pull request #160: Automate config docs generation
urfreespace commented on PR #160: URL: https://github.com/apache/pulsar-site/pull/160#issuecomment-1214503274 thanks @SignorMercurio, good job, LGTM -- 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] PIP 198: Standardize PR Naming Convention using GitHub Actions
To clarify, I don't have a strong feeling about either convention. According to the reason above, I'd prefer the Angular convention, while +0 for the customized convention. Best, tison. tison 于2022年8月14日周日 23:40写道： > Technically, the regexp of both conventions are: > > * Angular convention - /^(\w*)(?:\((.*)\))?!?: (.*)$/ > * Customized conventions - /^\[(\w*)\](?:\[(.*)\])?: (.*)$/ > > So, there're technically equal from the customized convention perspective, > or the Angular convention contains all expressiveness of the customized one. > > > It makes PR titles more clear and self-explanatory. > It's subjective. As described above, the Angular convention contains all > expressiveness of the customized one - it also has type and scope, and > delimiter length is almost the same. > > Let's think of the adoption of each convention: > > 1. Customized conventions: better to follow for developers who already use > it. > 2. Angular convention is a popular standard so that: > (1) It's well-known by _new_ developers. Just tell them we are using > Conventional Commits. > (2) Better toolchain support. This time we're lucky > that action-semantic-pull-request allows you to customize headerPattern. > But following the Conventional Commits will ensure we can use the toolchain > built around it, such as semantic release[1]. Also, Conventional Commits > have a standard to name a BREAKING CHANGE and a REVERT. We may or may not > want it later, shall we customize it further then? > > +1 for Angular convention. > > Best, > tison. > > [1] https://github.com/semantic-release/semantic-release > > > Qiang Huang 于2022年8月14日周日 12:15写道： > >> I agree that the customized one is better. +1 on the customized one. >> >> Jun M 于2022年8月12日周五 10:51写道： >> >> > +1 on the customized one. >> > >> > >> > Cheers >> > momo-jun >> > >> > >> >> -- >> BR, >> Qiang Huang >> >
Re: [Discuss] PIP 198: Standardize PR Naming Convention using GitHub Actions
Technically, the regexp of both conventions are: * Angular convention - /^(\w*)(?:\((.*)\))?!?: (.*)$/ * Customized conventions - /^\[(\w*)\](?:\[(.*)\])?: (.*)$/ So, there're technically equal from the customized convention perspective, or the Angular convention contains all expressiveness of the customized one. > It makes PR titles more clear and self-explanatory. It's subjective. As described above, the Angular convention contains all expressiveness of the customized one - it also has type and scope, and delimiter length is almost the same. Let's think of the adoption of each convention: 1. Customized conventions: better to follow for developers who already use it. 2. Angular convention is a popular standard so that: (1) It's well-known by _new_ developers. Just tell them we are using Conventional Commits. (2) Better toolchain support. This time we're lucky that action-semantic-pull-request allows you to customize headerPattern. But following the Conventional Commits will ensure we can use the toolchain built around it, such as semantic release[1]. Also, Conventional Commits have a standard to name a BREAKING CHANGE and a REVERT. We may or may not want it later, shall we customize it further then? +1 for Angular convention. Best, tison. [1] https://github.com/semantic-release/semantic-release Qiang Huang 于2022年8月14日周日 12:15写道： > I agree that the customized one is better. +1 on the customized one. > > Jun M 于2022年8月12日周五 10:51写道： > > > +1 on the customized one. > > > > > > Cheers > > momo-jun > > > > > > -- > BR, > Qiang Huang >
Re: [VOTE] Pulsar Release 2.8.4 Candidate 1
You can see https://lists.apache.org/thread/rg1g083c06ozm5go6zo1jophg9y9zl2f for more details about the LTS release. Thanks, Yunze > 2022年8月14日 11:00，Qiang Huang 写道： > > +1 (non-binding) > Is 2.8.4 a long term support release? > > Yunze Xu 于2022年8月12日周五 16:20写道： > >> This is the first release candidate for Apache Pulsar, version 2.8.4. >> >> It fixes the following issues: >> >> https://github.com/apache/pulsar/pulls?q=is%3Amerged+is%3Apr+label%3Arelease%2F2.8.4 >> >> *** Please download, test and vote on this release. This vote will stay >> open >> for at least 72 hours *** >> >> Note that we are voting upon the source (tag), binaries are provided for >> convenience. >> >> Source and binary files: >> https://dist.apache.org/repos/dist/dev/pulsar/pulsar-2.8.4-candidate-1/ >> >> SHA-512 checksums: >> >> c3d26704f2cfb3365c29d4110612ca7351084f8bee3c306d5e906b3f9b22c7557cc5baf12f74f8c222baccae3310691419eda5b47fdf9cd6c5281b70134ab5eb >> apache-pulsar-2.8.4-bin.tar.gz >> 28160ee94dccfb74dfb56e0e5d0e08870c6612659507333a52b5660ecbf060a75d1eed667cffd8596f9468de95055b78916b932db0e0d4c2745868d55429ee98 >> apache-pulsar-2.8.4-src.tar.gz >> >> Maven staging repo: >> https://repository.apache.org/content/repositories/orgapachepulsar-1170/ >> >> The tag to be voted upon: >> v2.8.4-candidate-1 (02ee5616866d4eda8dd94f85d9d9b71c459f248d) >> https://github.com/apache/pulsar/releases/tag/v2.8.4-candidate-1 >> >> Pulsar's KEYS file containing PGP keys we use to sign the release: >> https://dist.apache.org/repos/dist/dev/pulsar/KEYS >> >> Docker images: >> >> >> https://hub.docker.com/layers/pulsar/bewaremypower/pulsar/2.8.4/images/sha256-fba51a75c0f2ca79fbff7b254f80f641fcda661fd702f8149bbfdd5994078e3a >> >> >> https://hub.docker.com/layers/pulsar-all/bewaremypower/pulsar-all/2.8.4/images/sha256-42d4b41e5869edc6242bb49d6a1687bd6d191a6385637122edc5c7b2c44ee46f >> >> Please download the source package, and follow the Release Candidate >> Validation[1] to validate the release >> >> [1] https://github.com/apache/pulsar/wiki/Release-Candidate-Validation >> >> Thanks, >> Yunze >> >> >> >> >> > > -- > BR, > Qiang Huang
[GitHub] [pulsar-site] SignorMercurio commented on pull request #160: Automate config docs generation
SignorMercurio commented on PR #160: URL: https://github.com/apache/pulsar-site/pull/160#issuecomment-1214340328 In addition, a minor fix of an old problem (#134, #142) is committed here. -- 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-test-infra] tisonkun commented on a diff in pull request #56: avoid duplicate docbot comment
tisonkun commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945234643
##
docbot/main.go:
##
@@ -311,6 +311,11 @@ func (a *Action) onPullRequestOpenedOrEdited() error {
// Add missing label
if a.config.GetEnableLabelMissing() && checkedCount == 0 {
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()];
exist {
+ logger.Infoln("Already added missing label.")
+ return fmt.Errorf("%s", MessageLabelMissing)
Review Comment:
We should return the error here. Still the pr needs a valid doc label. We
just skip the create comment step.
##
docbot/main.go:
##
@@ -420,6 +425,11 @@ func (a *Action) onPullRequestLabeledOrUnlabeled() error {
// Add missing label
if a.config.GetEnableLabelMissing() && checkedCount == 0 {
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()];
exist {
+ logger.Infoln("Already added missing label.")
+ return fmt.Errorf("%s", MessageLabelMissing)
Review Comment:
See https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945234643
--
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
