Re: [DISCUSS] Dropping the StreamingDispatcher
+1 for the reason that it was added when there is no PIP restriction for new APIs. The original author has left the community for some time and recently it seems that no one touched the code except for some necessary API changes. Thanks, Yunze On Thu, Apr 6, 2023 at 11:48 AM wrote: > > Totally agree with it. +1 > > Best > Mattison > On Apr 6, 2023, 10:53 +0800, Devin Bost , wrote: > > +1 since it can be pulled back up in git history if someone decides to do > > something with it to improve it at a later time. > > > > I also agree that it's a pain to maintain, and I don't know anyone using > > it. I've gone through some of those code paths, and I was concerned about > > divergence anyway. > > > > - Devin > > > > > > On Wed, Apr 5, 2023, 5:40 PM Michael Marshall wrote: > > > > > If the code isn't being used or maintained, I support removing it. The > > > code will be available in the git history in case someone decides to > > > resurrect it. > > > > > > Thanks, > > > Michael > > > > > > On Wed, Apr 5, 2023 at 7:14 AM Enrico Olivelli > > > wrote: > > > > > > > > > > Yunze, > > > > > > > > > > Il Mar 4 Apr 2023, 09:57 Yunze Xu ha > > > scritto: > > > > > > > > > > > > If the flaky tests were the only concern, I think we can just > > > > > > > disable > > > > > > > these tests. > > > > > > > > > > > > > > > My concern is not about the the flaky tests but a out maintenance of > > > > > dead > > > > > code. > > > > > > > > > > > > > > > > > > > > Whatever, this config in `ServiceConfiguration` has > > > > > > > existed for a long time, though when it was introduced, the PIP > > > > > > > rule > > > > > > > was not clear so there is no PIP for it. > > > > > > > > > > > > > > > > > I don't think it would work well in production, given the amount of > > > > > flakyness in the tests and the fact that nobody ever asked questions > > > about > > > > > it. > > > > > > > > > > > > > > > This is why I propose to drop the code now in Pulsar 3.0 > > > > > > > > > > > > > > > Enrico > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > Yunze > > > > > > > > > > > > > > On Tue, Apr 4, 2023 at 3:09 PM Gavin gao > > > wrote: > > > > > > > > > > > > > > > > > > +1, I totally agree with this idea. > > > > > > > > > > > > > > > > > > Enrico Olivelli 于2023年4月4日周二 14:47写道： > > > > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > It has been a long time that we have in the Pulsar code a > > > > > > > > > > > new > > > > > > > > > > > experimental Dispatcher implementation named > > > > > > > > > > > StreamingDispatcher. > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/pulsar/pull/9056 > > > > > > > > > > > > > > > > > > > > > > There are many flaky tests about that feature and I > > > > > > > > > > > believe that it > > > > > > > > > > > has never been used in Production by anyone, because it > > > > > > > > > > > happened a > > > few > > > > > > > > > > > times that we did some changes in the regular Dispatcher > > > > > > > > > > > and > > > > > > > > > > > introduced bugs on the StreamingDispacther (usually > > > > > > > > > > > manifested as > > > > > > > > > > > flaky tests) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I propose to drop the StreamingDispatcher code for Pulsar > > > > > > > > > > > 3.0. > > > > > > > > > > > I don't think we need a PIP for this, it is an > > > > > > > > > > > experimental code > > > that > > > > > > > > > > > was never delivered as a production ready feature. > > > > > > > > > > > > > > > > > > > > > > If anyone is aware of users please chime in. > > > > > > > > > > > > > > > > > > > > > > If anyone wants to sponsor that feature and objects in > > > > > > > > > > > removing > > > this > > > > > > > > > > > dead code (that we still have to maintain) please help us > > > > > > > > > > > in > > > > > > > > > > > completing the feature. > > > > > > > > > > > > > > > > > > > > > > On paper it is a very appealing feature, and I am > > > > > > > > > > > disappointed in > > > > > > > dropping > > > > > > > > > > > it. > > > > > > > > > > > On the other hand, this is dead code that we have to > > > > > > > > > > > maintain with > > > zero > > > > > > > > > > > benefit > > > > > > > > > > > > > > > > > > > > > > Thoughts ? > > > > > > > > > > > > > > > > > > > > > > Enrico > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] Dropping the StreamingDispatcher
+1, I support removing it if the code isn't being used or maintained. Thanks, Cong Zhao On 2023/04/04 06:47:24 Enrico Olivelli wrote: > Hello, > It has been a long time that we have in the Pulsar code a new > experimental Dispatcher implementation named StreamingDispatcher. > > https://github.com/apache/pulsar/pull/9056 > > There are many flaky tests about that feature and I believe that it > has never been used in Production by anyone, because it happened a few > times that we did some changes in the regular Dispatcher and > introduced bugs on the StreamingDispacther (usually manifested as > flaky tests) > > > I propose to drop the StreamingDispatcher code for Pulsar 3.0. > I don't think we need a PIP for this, it is an experimental code that > was never delivered as a production ready feature. > > If anyone is aware of users please chime in. > > If anyone wants to sponsor that feature and objects in removing this > dead code (that we still have to maintain) please help us in > completing the feature. > > On paper it is a very appealing feature, and I am disappointed in dropping it. > On the other hand, this is dead code that we have to maintain with zero > benefit > > Thoughts ? > > Enrico >
Re: [DISCUSS] Dropping the StreamingDispatcher
Hi all, +1 for removing the StreamingDispatcher in Pulsar 3.0. Balancing maintainability, scalability, and usability is critical for an open-source project. In this case, the StreamingDispatcher seems to be neither widely adopted nor actively maintained, and its code quality and unstable tests have been causing an extra maintenance burden. Best, Xiangying On Fri, Apr 7, 2023 at 6:30 PM Cong Zhao wrote: > +1, I support removing it if the code isn't being used or maintained. > > Thanks, > Cong Zhao > > On 2023/04/04 06:47:24 Enrico Olivelli wrote: > > Hello, > > It has been a long time that we have in the Pulsar code a new > > experimental Dispatcher implementation named StreamingDispatcher. > > > > https://github.com/apache/pulsar/pull/9056 > > > > There are many flaky tests about that feature and I believe that it > > has never been used in Production by anyone, because it happened a few > > times that we did some changes in the regular Dispatcher and > > introduced bugs on the StreamingDispacther (usually manifested as > > flaky tests) > > > > > > I propose to drop the StreamingDispatcher code for Pulsar 3.0. > > I don't think we need a PIP for this, it is an experimental code that > > was never delivered as a production ready feature. > > > > If anyone is aware of users please chime in. > > > > If anyone wants to sponsor that feature and objects in removing this > > dead code (that we still have to maintain) please help us in > > completing the feature. > > > > On paper it is a very appealing feature, and I am disappointed in > dropping it. > > On the other hand, this is dead code that we have to maintain with zero > benefit > > > > Thoughts ? > > > > Enrico > > >
Re: [DISCUSS] PIP-257: Add Open ID Connect Support to Server Components
Thanks for initiating the proposal, Michael. I strongly support this addition, which will greatly enhance Pulsar's security and reliability. > I am new to this security component, but how do we support token revoke? Regarding Heesung Sohn's question about token revocation, I believe that we can initially rely on short-lived tokens and periodic refreshes to ensure security. If the community has a clear demand for token revocation in the future, we can consider implementing this feature at that time. I'm really looking forward to the successful implementation of this proposal. Best regards, Xiangying On Fri, Apr 7, 2023 at 1:57 AM Heesung Sohn wrote: > Hi, > > I am new to this security component, but how do we support token revoke? > > Thanks, > Heesung > > On Mon, Mar 27, 2023 at 12:31 PM Michael Marshall > wrote: > > > Here is the K8s integration: https://github.com/apache/pulsar/pull/19888 > . > > > > That PR makes it possible to configure a function running in > > kubernetes to use the Service Account Token provided by kubernetes. > > > > Future work suggested by Eron Wright is to add support for the > > function worker to mount tokens for any requested audience. I think > > that feature will be very valuable, but in hopes of completing this > > PIP by 3.0.0, I would like to defer that feature. > > > > Thanks, > > Michael > > > > On Mon, Mar 20, 2023 at 5:59 PM Michael Marshall > > wrote: > > > > > > Update: the PR [0] to add the OIDC authentication provider module is > > > ready for review. > > > > > > I plan to start looking at the function worker integration with k8s > > tomorrow. > > > > > > I hope to start the vote later this week. > > > > > > Thanks, > > > Michael > > > > > > [0] https://github.com/apache/pulsar/pull/19849 > > > > > > On Mon, Mar 20, 2023 at 5:56 PM Michael Marshall > > > wrote: > > > > > > > > >> 2. Implement `KubernetesFunctionAuthProvider` with > > > > >`KubernetesSecretsAuthProvider`. > > > > > > > > >It looks like we add an authentication provider for the Kubernetes > > > > >environment. Is the OIDC authentication provider? > > > > > > > > The current KubernetesSecretsTokenAuthProvider [0] mounts the auth > > > > data used to create a function. Because OIDC often has short lived > > > > tokens, it won't work to copy the token from the call used to create > > > > the function. Instead, my initial proposal was to let a user specify > a > > > > pre-existing k8s secret that will have the correct authentication > > > > data. Because anything can be in the secret, there isn't a reason to > > > > require this secret to have the client id and client secret. > > > > > > > > Eron suggested in the PIP issue [1] that we make it possible to > easily > > > > integrate with the Kubernetes Service Account. I'll be looking into > > > > that integration this week. > > > > > > > > Thanks, > > > > Michael > > > > > > > > [0] > > > https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/auth/KubernetesSecretsTokenAuthProvider.java > > > > [1] > > https://github.com/apache/pulsar/issues/19771#issuecomment-1463029346 > > > > > > > > > > > > On Mon, Mar 13, 2023 at 3:03 AM Zixuan Liu > wrote: > > > > > > > > > > Hi Michael, > > > > > > > > > > +1, Thank you for your PIP! That's important for modern > > authentication! > > > > > > > > > > I have a question: > > > > > > > > > > > 2. Implement `KubernetesFunctionAuthProvider` with > > > > > `KubernetesSecretsAuthProvider`. > > > > > > > > > > It looks like we add an authentication provider for the Kubernetes > > > > > environment. Is the OIDC authentication provider? > > > > > > > > > > > > > > > Thanks, > > > > > Zixuan > > > > > > > > > > > > > > > > > > > > Lari Hotari 于2023年3月10日周五 14:56写道： > > > > > > > > > > > Thanks for starting this PIP, Michael. > > > > > > This is really important in improving Pulsar's security and > > reducing > > > > > > certain attack surfaces and eliminating certain attack vectors. > > I'm looking > > > > > > forward to having Open ID connect (OIDC) supported in Pulsar > server > > > > > > components so that Pulsar could be operated without the use of > > static JWT > > > > > > tokens such as the superuser token. > > > > > > > > > > > > -Lari > > > > > > > > > > > > On 2023/03/09 22:34:49 Michael Marshall wrote: > > > > > > > Hi Pulsar Community, > > > > > > > > > > > > > > I would like to contribute Open ID Connect support to the > server > > > > > > > components in Pulsar. Here is a link to the PIP: > > > > > > > https://github.com/apache/pulsar/issues/19771. I plan to start > > working > > > > > > > on the implementation next week. I look forward to your > feedback. > > > > > > > > > > > > > > Thanks, > > > > > > > Michael > > > > > > > > > > > > > > ### Motivation > > > > > > > > > > > > > > Apache Pulsar does not yet support a server side > > > > > > > `AuthenticationProvider` that implements the Open ID Connect > > spec
Re: [DISCUSS] PIP-263: Just auto-create no-partitioned DLQ And Prevent auto-create a DLQ for a DLQ
Hi Yubiao,
Appreciate your effort in initiating this PIP. I believe these changes will
address the existing issues and make DLQ and Retry Topic handling more
efficient and straightforward.
The goals you outlined are clear and, upon implementation, will improve the
overall functionality of Pulsar. The proposed API changes also seem
suitable for achieving the desired outcomes.
Looking forward to the progress on this PIP.
Best regards,
Xiangying
On Fri, Apr 7, 2023 at 1:56 AM Yubiao Feng
wrote:
> Hi community
>
> I started a PIP about "Just auto-create no-partitioned DLQ And Prevent
> auto-create a DLQ for a DLQ".
>
> PIP link: https://github.com/apache/pulsar/issues/20033
>
> ### Motivation
>
> Just auto-create no-partitioned DLQ/Retry Topic
> If enabled the config `allowAutoTopicCreation,` Pulsar will auto-create a
> topic when the client loads it; After setting config
> `allowAutoTopicCreationType=partitioned, defaultNumPartitions=2`, Pulsar
> will auto-create a partitioned topic(which have two partitions) when the
> client loads it.
>
> After the above, if using the feature [Retry Topic](
>
> https://pulsar.apache.org/docs/2.11.x/concepts-messaging/#retry-letter-topic
> )
> and [DLQ](
> https://pulsar.apache.org/docs/2.11.x/concepts-messaging/#dead-letter-topic
> )
> enable topic auto-creation, we will get a partitioned DLQ and a partitioned
> Retry Topic like this:
> - `{primary_topic_name}-{sub_name}-DLQ`
> -`{primary_topic_name}-{sub_name}-DLQ-partition-0`
> -`{primary_topic_name}-{sub_name}-DLQ-partition-1`
> - `{primary_topic_name}-{sub_name}-RETRY`
> -`{primary_topic_name}-{sub_name}-RETRY-partition-0`
> -`{primary_topic_name}-{sub_name}-RETRY-partition-1`
>
>
>
> I feel that almost all users will not use the multi-partitioned DLQ or
> multi-partitioned Retry topic because there is a bug that causes the above
> behavior to be incorrect, but we have yet to receive any issues about it.
> This bug causes the above behavior to look like this: When the partitioned
> DLQ is auto-created for the topic `tp1-partition-0`, Pulsar will create a
> partitioned topic meta which has two partitioned but only create a topic
> named `{primary_topic_name}-{sub_name}-DLQ,` there is no topic named
> `{primary_topic_name}-{sub_name}-DLQ-partition-x.` Please look at this
> [PR]( https://github.com/apache/pulsar/pull/19841) for a detailed bug
> description.
>
> So I want to change the behavior to Just auto-create no-partitioned
> DLQ/Retry Topic.
>
>
>
> Prevent auto-create the DLQ for a DLQ
> Please look at this [Discussion](
> https://lists.apache.org/thread/q1m23ckyy10wvtzy65v8bwqwnh7r0gc8) for the
> detail.
>
>
>
> ### Goal
>
> - Just auto-create no-partitioned DLQ/Retry Topic(with the other words:
> prevent auto-create partitioned DLQ)
> - DLQ/Retry topic should not create for a DLQ/Retry Topic
> - roles:
> - DLQ will not auto-create for a DLQ
> - Retry Topic will not auto-create for a Retry Topic
> - DLQ will not auto-create for a Retry Topic
> - Retry Topic will not auto-create for a DLQ
> - client changes: Clients will not create a DLQ for a DLQ
> - broker changes: rejected the request which wants to auto-create a DLQ
> for a DLQ
>
>
>
> ### API Changes
>
> CommandSubscribe.java
> ```java
> /**
> * This is an enumeration value with tree options: "standard", "dead
> letter", "retry letter".
> */
> private String topicPurpose;
> ```
>
> Properties of Topic
> ```properties
> "purposeOfAutoCreatedTopic": value with tree options: "standard", "dead
> letter", "retry letter"
> ```
>
> Why not use two properties: `isAutoCreated` and `topicPurpose`?
> Because there is a scenario like this: auto-create a topic, use it as a DLQ
> after a few days, and not use it as a DLQ after a few days, this Topic will
> be allowed to have DLQ/Retry Topic. We only mark the topics created for
> DLQ/Retry purposes.
>
>
> Thanks
> Yubiao Feng
>
