Re: [DISCUSS] PIP-247: Notifications for partitions update

2023-02-23 Thread houxiaoyu
Hi Joe,

When we use PartitionedProducerImpl or MultiTopicsConsumerImpl,  there is a
poll task to fetch the metadata of the partitioned-topic regularly for the
number of partitions updated.  This PIP wants to use a
notification mechanism to replace the metadata poll task.

Just the way to implements partitioned-topic metadata
notification mechanism is much like notifications on regex sub changes

Joe F  于2023年2月24日周五 13:37写道:

> Why is this needed when we have notifications on regex sub changes? Aren't
> the partition names a well-defined regex?
>
> Joe
>
> On Thu, Feb 23, 2023 at 8:52 PM houxiaoyu  wrote:
>
> > Hi Asaf,
> > thanks for your reminder.
> >
> > ## Changing
> > I have updated the following changes to make sure the notification
> arrived
> > successfully:
> > 1. The watch success response `CommandWatchPartitionUpdateSuccess` will
> > contain all the concerned topics of this watcher
> > 2. The notification `CommandPartitionUpdate` will always contain all the
> > concerned topics of this watcher.
> > 3. The notification `CommandPartitionUpdate`contains a monotonically
> > increased version.
> > 4. A map `PartitonUpdateWatcherService#inFlightUpdate > Pair>` will keep track of the updating
> > 5. A timer will check the updating timeout through `inFlightUpdate`
> > 6. The client acks `CommandPartitionUpdateResult` to broker when it
> > finishes updating.
> >
> > ## Details
> >
> > The following mechanism could make sure the newest notification arrived
> > successfully, copying the description from GH:
> >
> > A new class, `org.apache.pulsar.PartitonUpdateWatcherService` will keep
> > track of watchers and will listen to the changes in the metadata.
> Whenever
> > a topic partition updates it checks if any watchers should be notified
> and
> > sends an update for all topics the watcher concerns through the
> ServerCnx.
> > Then we will record this request into a map,
> > `PartitonUpdateWatcherService#inFlightUpdate Pair > long/*timestamp*/>>`.  A timer will check this update timeout through
> > inFlightUpdate .  We will query all the concerned topics's partition if
> > this watcher has sent an update timeout and will resend it.
> >
> > The client acks `CommandPartitionUpdateResult` to broker when it finishes
> > updating.  The broker handle `CommandPartitionUpdateResult` request:
> >  - If CommandPartitionUpdateResult#version <
> > PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version,
> broker
> > ignores this ack.
> >  -  If CommandPartitionUpdateResult#version ==
> > PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version
> > - If CommandPartitionUpdateResult#success is true,  broker just
> removes
> > the watcherID from inFlightUpdate.
> > - If CommandPartitionUpdateResult#success is false,  broker removes
> the
> > watcherId from inFlightUpdate, and queries all the concerned topics's
> > partition and resend.
> >  - If CommandPartitionUpdateResult#version >
> > PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version, this
> > should not happen.
> >
> >  ## Edge cases
> > - Broker restarts or crashes
> > Client will reconnect to another broker, broker responses
> > `CommandWatchPartitionUpdateSuccess` with watcher concerned topics's
> > partitions.  We will call `PartitionsUpdateListener` if the connection
> > opens.
> > - Client acks fail or timeout
> > Broker will resend the watcher concerned topics's partitions either
> client
> > acks fail or acks timeout.
> > - Partition updates before client acks.
> > `CommandPartitionUpdate#version` monotonically increases every time it is
> > updated. If Partition updates before client acks, a greater version will
> be
> > put into `PartitonUpdateWatcherService#inFlightUpdate`.  The previous
> acks
> > will be ignored because the version is less than the current version.
> >
> >
> > Asaf Mesika  于2023年2月22日周三 21:33写道:
> >
> > > How about edge cases?
> > > In Andra's PIP he took into account cases where updates were lost, so
> he
> > > created a secondary poll. Not saying it's the best situation for your
> > case
> > > of course.
> > > I'm saying that when a broker sends an update CommandPartitionUpdate,
> how
> > > do you know it arrived successfully? From my memory, there is no ACK in
> > the
> > > protocol, saying "I'm the client, I got the update successfully" and
> only
> > > then it removed the "dirty" flag for that topic, for this watcher ID.
> > >
> > > Are there any other edge cases we can have? Let's be exhaustive.
> > >
> > >
> > >
> > > On Wed, Feb 22, 2023 at 1:14 PM houxiaoyu  wrote:
> > >
> > > > Thanks for your great suggestion Enrico.
> > > >
> > > > I agreed with you. It's more reasonable to add a
> > > > `supports_partition_update_watchers`  in `FeatureFlags`  to detect
> that
> > > the
> > > > connected broker supporting this feature , and add a new broker
> > > > configuration property `enableNotificationForPartitionUpdate` with
> > > default
> > > > value true, which is much like PIP-145.
> 

Re: TLS regression verification for Python client 3.1.0 and Node.js client 1.8.1

2023-02-23 Thread Yunze Xu
Hi Zixuan,

This test is to verify if the specific client can avoid configuring
the CA file explicitly. From the test results, the Python client works
well without setting the file explicitly, while the Node.js client
does not.

Currently we have already figured out the reason why only the Python
client works [1] and Baodi will work on fixing the issue for Node.js
clients.

> The following are ca files from golang codebase

BTW, the code you gave only works for Linux. On some other systems [2]
[3], getting the CA file is complicated.

[1] https://github.com/apache/pulsar/pull/4216
[2] https://go.dev/src/crypto/x509/root_darwin.go
[3] https://go.dev/src/crypto/x509/root_windows.go

Thanks,
Yunze

On Fri, Feb 24, 2023 at 12:11 PM Zixuan Liu  wrote:
>
> This is not very friendly to explicitly set the ca file.
>
> Can we dynamically search the system ca file? and then go to set the ca
> file to the libcurl.
>
> The following are ca files from golang codebase(this  is what you
> mentioned):
> ```
> // Possible certificate files; stop after finding one.
> var certFiles = []string{
> "/etc/ssl/certs/ca-certificates.crt",//
> Debian/Ubuntu/Gentoo etc.
> "/etc/pki/tls/certs/ca-bundle.crt",  // Fedora/RHEL 6
> "/etc/ssl/ca-bundle.pem",// OpenSUSE
> "/etc/pki/tls/cacert.pem",   // OpenELEC
> "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // CentOS/RHEL 7
> "/etc/ssl/cert.pem", // Alpine Linux
> }
>
> // Possible directories with certificate files; all will be read.
> var certDirectories = []string{
> "/etc/ssl/certs",   // SLES10/SLES11,
> https://golang.org/issue/12139
> "/etc/pki/tls/certs",   // Fedora/RHEL
> "/system/etc/security/cacerts", // Android
> }
> ```
>
> Thanks,
> Zixuan
>
>
>
> Yunze Xu  于2023年2月24日周五 02:06写道:
>
> > I've figured out why the Python client does not suffer from this
> > issue. I use `strace` to print all system calls. Then I find the
> > Python client reads another cert file:
> >
> > ```
> > openat(AT_FDCWD,
> > "/usr/local/lib/python3.8/dist-packages/certifi/cacert.pem", O_RDONLY)
> > = 6
> > ```
> >
> > The correct cert comes from the `certifi` package.
> >
> > While the Node.js client first tries to read `/etc/ssl/cert.pem`, then
> > tries to read `/etc/pki/tls/certs/ca-bundle.crt` that is the libcurl's
> > bundle CA cert location. Both of them don't exist.
> >
> > ```
> > 1828 openat(AT_FDCWD, "/etc/ssl/cert.pem", O_RDONLY) = -1 ENOENT (No
> > such file or directory)
> > 1891 openat(AT_FDCWD, "/etc/pki/tls/certs/ca-bundle.crt", O_RDONLY) =
> > -1 ENOENT (No such file or directory)
> > ```
> >
> > Thanks,
> > Yunze
> >
> > On Thu, Feb 23, 2023 at 11:50 PM Yunze Xu  wrote:
> > >
> > > Hi all,
> > >
> > > Currently there are two ongoing releases: Python client 3.1.0 [1] and
> > > Node.js client 1.8.1 [2]. Both these two releases depend on the C++
> > > client 3.1.2, which fixes an issue that when performing OAuth2
> > > authentication with an issuer URL whose protocol is HTTPS, users can
> > > configure the tls cert file path to fix it.
> > >
> > > However, a regression was reported in the vote mail list of Node.js
> > > client [2]. In short, since Node.js client 1.8.0, users have to
> > > configure the tls cert file path explicitly, while the path could be
> > > detected automatically before. Since the Python client is also based
> > > on the C++ client and adopts a similar way to build the binaries, I've
> > > thought it should suffer the same issue. While the test result shows
> > > the Python client works well.
> > >
> > > So I created a repository [3] to:
> > > - Give an example for how to test Python and Node.js clients
> > > - Report my test results on various operating systems
> > >
> > > Please also help verify these two candidate releases.
> > >
> > > [1] https://lists.apache.org/thread/0mcdfw6ogof2bcl40m9p3gjzwolr8rw2
> > > [2] https://lists.apache.org/thread/n8jrg29nmozxqp588dbd8nkm7y18sphf
> > > [3] https://github.com/BewareMyPower/pulsar-tls-examples
> > >
> > > Thanks,
> > > Yunze
> >


Re: [DISCUSS] ClientVersion conflict across multiple language client

2023-02-23 Thread Zike Yang
> I think it makes sense to make the format
"pulsar--" for official clients. We could then
recommend third party clients replace "pulsar" with a relevant org
name. There isn't a way to enforce the version string though, so these
would only be recommendations.

+1 for this format.

> F#:
I'm not exactly sure how to interpret this input.
https://github.com/fsprojects/pulsar-client-dotnet/blob/5c6dcff0184f87005f55dcc847893a889aaee2ad/src/Pulsar.Client/Internal/ClientCnx.fs#L119

The format is like `2.11.0.0`. It's consistent with the current java
client version format.

> Because we filter out the official go client, I propose that we remove
the filtering of certain client versions:
https://github.com/apache/pulsar/pull/19616.

Thanks for your PR. I have approved.

Since there are no objections to this discussion, I will next make the
client version format of all clients(at least official clients)
consistent.

BR,
Zike Yang

On Fri, Feb 24, 2023 at 3:22 AM Michael Marshall  wrote:
>
> > Go:
> > ClientVersionString = "Pulsar Go " + Version
> > https://github.com/apache/pulsar-client-go/blob/dedbdc45c63b06e6a12356785418cd906d6bab3c/pulsar/internal/version.go#L43
>
> Because we filter out the official go client, I propose that we remove
> the filtering of certain client versions:
> https://github.com/apache/pulsar/pull/19616.
>
> Thanks,
> Michael
>
> On Thu, Feb 23, 2023 at 12:45 PM Michael Marshall  
> wrote:
> >
> > I found another data point in favor of moving in this direction. Our
> > protocol spec says the following about the client_version field:
> >
> > message CommandConnect {
> > "client_version" : "Pulsar-Client-Java-v1.15.2",
> > "auth_method_name" : "my-authentication-plugin",
> > "auth_data" : "my-auth-data",
> > "protocol_version" : 6
> > }
> >
> > * `client_version`: String-based identifier. Format is not enforced.
> >
> > https://github.com/apache/pulsar-site/blame/main/docs/developing-binary-protocol.md#L133
> >
> > I think it makes sense to make the format
> > "pulsar--" for official clients. We could then
> > recommend third party clients replace "pulsar" with a relevant org
> > name. There isn't a way to enforce the version string though, so these
> > would only be recommendations.
> >
> > For official clients, here are our version strings:
> >
> > Go:
> > ClientVersionString = "Pulsar Go " + Version
> > https://github.com/apache/pulsar-client-go/blob/dedbdc45c63b06e6a12356785418cd906d6bab3c/pulsar/internal/version.go#L43
> >
> > C++
> > connect->set_client_version(PULSAR_VERSION_STR);
> > https://github.com/apache/pulsar-client-cpp/blob/96507cecdc90f10eca5febc48a0623f72d338615/lib/Commands.cc#L269
> > std::string version = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
> > https://github.com/apache/pulsar-client-cpp/blob/05807bdaf3a4341b22efd7c71f7b00c47cc31413/lib/HTTPLookupService.cc#L195
> >
> > I took a quick survey of some existing third party clients. Here are
> > the results for how the version string is written:
> >
> > Rust:
> > client_version: String::from("2.0.1-incubating"),
> > https://github.com/streamnative/pulsar-rs/blob/eb87ef796bd8cb42fc72bb72ed14751fffa437e1/src/connection.rs#L1173
> >
> > Haskell:
> > & F.clientVersion .~ "Pulsar-Client-Haskell-v" <> T.pack (showVersion 
> > version)
> > https://github.com/cr-org/supernova/blob/602409a18f47a38541ba24f5e885199efd383f48/lib/src/Pulsar/Protocol/Commands.hs#L22
> >
> > PHP:
> > $cmd->setClientVersion(sprintf('ikilobyte/pulsar-client-php@v%s',
> > Client::VERSION_ID));
> > https://github.com/ikilobyte/pulsar-client-php/blob/e8847c62d3bf136886312a14a04f51c59ca99886/src/IO/StreamIO.php#L77
> >
> > F#:
> > I'm not exactly sure how to interpret this input.
> > https://github.com/fsprojects/pulsar-client-dotnet/blob/5c6dcff0184f87005f55dcc847893a889aaee2ad/src/Pulsar.Client/Internal/ClientCnx.fs#L119
> >
> > Scala:
> > All appear to wrap the Pulsar Java client (or don't have a
> > clientVersion string in their code base).
> >
> > Thanks,
> > Michael
> >
> > On Thu, Feb 23, 2023 at 11:39 AM Michael Marshall  
> > wrote:
> > >
> > > > A better solution is that we could allow the
> > > > user to specify the suffix of the client version.
> > >
> > > I don't think we need to make it configurable because the library
> > > owner, not the user, has full control when creating the Connect
> > > command.
> > >
> > > We can update our protocol spec to recommend appropriate usage and of the 
> > > field.
> > >
> > > Thanks,
> > > Michael
> > >
> > > On Thu, Feb 23, 2023 at 2:51 AM Zike Yang  wrote:
> > > >
> > > > > But I'm wondering if it's possible to support configuring the client
> > > > version string by users?
> > > >
> > > > I think it's possible. A better solution is that we could allow the
> > > > user to specify the suffix of the client version. For example, by
> > > > adding a new configuration like `clientVersionSuffix` to the
> > > > ClientConffiguration, the client_version would be like
> > > > `java-3.0.0-forked-v1.0.0`. In 

Re: [DISCUSS] Release 2.9.5

2023-02-23 Thread mattisonchao
Hi, All

I am sorry, Cong Zhao will takeover this release.

Best,
Mattison
On Feb 21, 2023, 15:11 +0800, Haiting Jiang , wrote:
> +1
>
> Haiting
>
> On Tue, Feb 21, 2023 at 12:11 PM Hang Chen  wrote:
> >
> > +1
> >
> > Thanks,
> > Hang
> >
> > Enrico Olivelli  于2023年2月20日周一 21:11写道:
> > > >
> > > > +1
> > > >
> > > > Enrico
> > > >
> > > > Il giorno lun 20 feb 2023 alle ore 10:41 Zike Yang  ha 
> > > > scritto:
> > > > > >
> > > > > > +1
> > > > > >
> > > > > > Thanks,
> > > > > > Zike Yang
> > > > > >
> > > > > > On Mon, Feb 20, 2023 at 12:57 PM  wrote:
> > > > > > > >
> > > > > > > > Hello, Pulsar community:
> > > > > > > >
> > > > > > > > I'd like to propose releasing Apache Pulsar 2.9.5. It's been 
> > > > > > > > about
> > > > > > > > two months since 2.9.4 was released.
> > > > > > > >
> > > > > > > > There are 54 PRs [0] needed to cherry-pick in branch-2.9. I will
> > > > > > > > cherry-pick these PRs for branch-2.9. Exclude some PRs that 
> > > > > > > > merge
> > > > > > > > directly into branch-2.9.
> > > > > > > >
> > > > > > > > There are 21 PRs [1] opened. I'll follow up on each of those 
> > > > > > > > PRs to
> > > > > > > > see if they will be completed soon or will need to be pushed to 
> > > > > > > > 2.9.6
> > > > > > > >
> > > > > > > > If you have any important fixes or any questions, please reply 
> > > > > > > > to this
> > > > > > > > email, and we will evaluate whether to include them in 2.9.5
> > > > > > > >
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Mattison
> > > > > > > >
> > > > > > > > [0] 
> > > > > > > > https://github.com/apache/pulsar/pulls?q=is%3Apr+label%3Arelease%2F2.9.5+-label%3Acherry-picked%2Fbranch-2.9+is%3Aclosed+
> > > > > > > > [1] 
> > > > > > > > https://github.com/apache/pulsar/pulls?q=is%3Aopen+is%3Apr+label%3Arelease%2F2.9.5


Re: [DISCUSS] PIP-247: Notifications for partitions update

2023-02-23 Thread Joe F
Why is this needed when we have notifications on regex sub changes? Aren't
the partition names a well-defined regex?

Joe

On Thu, Feb 23, 2023 at 8:52 PM houxiaoyu  wrote:

> Hi Asaf,
> thanks for your reminder.
>
> ## Changing
> I have updated the following changes to make sure the notification arrived
> successfully:
> 1. The watch success response `CommandWatchPartitionUpdateSuccess` will
> contain all the concerned topics of this watcher
> 2. The notification `CommandPartitionUpdate` will always contain all the
> concerned topics of this watcher.
> 3. The notification `CommandPartitionUpdate`contains a monotonically
> increased version.
> 4. A map `PartitonUpdateWatcherService#inFlightUpdate Pair>` will keep track of the updating
> 5. A timer will check the updating timeout through `inFlightUpdate`
> 6. The client acks `CommandPartitionUpdateResult` to broker when it
> finishes updating.
>
> ## Details
>
> The following mechanism could make sure the newest notification arrived
> successfully, copying the description from GH:
>
> A new class, `org.apache.pulsar.PartitonUpdateWatcherService` will keep
> track of watchers and will listen to the changes in the metadata. Whenever
> a topic partition updates it checks if any watchers should be notified and
> sends an update for all topics the watcher concerns through the ServerCnx.
> Then we will record this request into a map,
> `PartitonUpdateWatcherService#inFlightUpdate long/*timestamp*/>>`.  A timer will check this update timeout through
> inFlightUpdate .  We will query all the concerned topics's partition if
> this watcher has sent an update timeout and will resend it.
>
> The client acks `CommandPartitionUpdateResult` to broker when it finishes
> updating.  The broker handle `CommandPartitionUpdateResult` request:
>  - If CommandPartitionUpdateResult#version <
> PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version, broker
> ignores this ack.
>  -  If CommandPartitionUpdateResult#version ==
> PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version
> - If CommandPartitionUpdateResult#success is true,  broker just removes
> the watcherID from inFlightUpdate.
> - If CommandPartitionUpdateResult#success is false,  broker removes the
> watcherId from inFlightUpdate, and queries all the concerned topics's
> partition and resend.
>  - If CommandPartitionUpdateResult#version >
> PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version, this
> should not happen.
>
>  ## Edge cases
> - Broker restarts or crashes
> Client will reconnect to another broker, broker responses
> `CommandWatchPartitionUpdateSuccess` with watcher concerned topics's
> partitions.  We will call `PartitionsUpdateListener` if the connection
> opens.
> - Client acks fail or timeout
> Broker will resend the watcher concerned topics's partitions either client
> acks fail or acks timeout.
> - Partition updates before client acks.
> `CommandPartitionUpdate#version` monotonically increases every time it is
> updated. If Partition updates before client acks, a greater version will be
> put into `PartitonUpdateWatcherService#inFlightUpdate`.  The previous acks
> will be ignored because the version is less than the current version.
>
>
> Asaf Mesika  于2023年2月22日周三 21:33写道:
>
> > How about edge cases?
> > In Andra's PIP he took into account cases where updates were lost, so he
> > created a secondary poll. Not saying it's the best situation for your
> case
> > of course.
> > I'm saying that when a broker sends an update CommandPartitionUpdate, how
> > do you know it arrived successfully? From my memory, there is no ACK in
> the
> > protocol, saying "I'm the client, I got the update successfully" and only
> > then it removed the "dirty" flag for that topic, for this watcher ID.
> >
> > Are there any other edge cases we can have? Let's be exhaustive.
> >
> >
> >
> > On Wed, Feb 22, 2023 at 1:14 PM houxiaoyu  wrote:
> >
> > > Thanks for your great suggestion Enrico.
> > >
> > > I agreed with you. It's more reasonable to add a
> > > `supports_partition_update_watchers`  in `FeatureFlags`  to detect that
> > the
> > > connected broker supporting this feature , and add a new broker
> > > configuration property `enableNotificationForPartitionUpdate` with
> > default
> > > value true, which is much like PIP-145.
> > >
> > > I have updated the descriptions.
> > >
> > > Enrico Olivelli  于2023年2月22日周三 17:26写道:
> > >
> > > > I support this proposal.
> > > > Coping here my comments from GH:
> > > >
> > > > can't we enable this by default in case we detect that the connected
> > > > Broker supports it ?
> > > > I can't find any reason for not using this mechanism if it is
> > available.
> > > >
> > > > Maybe we can set the default to "true" and allow users to disable it
> > > > in case it impacts their systems in an unwanted way.
> > > >
> > > > Maybe It would be useful to have a way to disable the mechanism on
> the
> > > > broker side as well
> > > >
> > > > Enrico
> 

Re: [DISCUSS] PIP-247: Notifications for partitions update

2023-02-23 Thread houxiaoyu
Hi Asaf,
thanks for your reminder.

## Changing
I have updated the following changes to make sure the notification arrived
successfully:
1. The watch success response `CommandWatchPartitionUpdateSuccess` will
contain all the concerned topics of this watcher
2. The notification `CommandPartitionUpdate` will always contain all the
concerned topics of this watcher.
3. The notification `CommandPartitionUpdate`contains a monotonically
increased version.
4. A map `PartitonUpdateWatcherService#inFlightUpdate>` will keep track of the updating
5. A timer will check the updating timeout through `inFlightUpdate`
6. The client acks `CommandPartitionUpdateResult` to broker when it
finishes updating.

## Details

The following mechanism could make sure the newest notification arrived
successfully, copying the description from GH:

A new class, `org.apache.pulsar.PartitonUpdateWatcherService` will keep
track of watchers and will listen to the changes in the metadata. Whenever
a topic partition updates it checks if any watchers should be notified and
sends an update for all topics the watcher concerns through the ServerCnx.
Then we will record this request into a map,
`PartitonUpdateWatcherService#inFlightUpdate>`.  A timer will check this update timeout through
inFlightUpdate .  We will query all the concerned topics's partition if
this watcher has sent an update timeout and will resend it.

The client acks `CommandPartitionUpdateResult` to broker when it finishes
updating.  The broker handle `CommandPartitionUpdateResult` request:
 - If CommandPartitionUpdateResult#version <
PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version, broker
ignores this ack.
 -  If CommandPartitionUpdateResult#version ==
PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version
- If CommandPartitionUpdateResult#success is true,  broker just removes
the watcherID from inFlightUpdate.
- If CommandPartitionUpdateResult#success is false,  broker removes the
watcherId from inFlightUpdate, and queries all the concerned topics's
partition and resend.
 - If CommandPartitionUpdateResult#version >
PartitonUpdateWatcherService#inFlightUpdate.get(watcherID).version, this
should not happen.

 ## Edge cases
- Broker restarts or crashes
Client will reconnect to another broker, broker responses
`CommandWatchPartitionUpdateSuccess` with watcher concerned topics's
partitions.  We will call `PartitionsUpdateListener` if the connection
opens.
- Client acks fail or timeout
Broker will resend the watcher concerned topics's partitions either client
acks fail or acks timeout.
- Partition updates before client acks.
`CommandPartitionUpdate#version` monotonically increases every time it is
updated. If Partition updates before client acks, a greater version will be
put into `PartitonUpdateWatcherService#inFlightUpdate`.  The previous acks
will be ignored because the version is less than the current version.


Asaf Mesika  于2023年2月22日周三 21:33写道:

> How about edge cases?
> In Andra's PIP he took into account cases where updates were lost, so he
> created a secondary poll. Not saying it's the best situation for your case
> of course.
> I'm saying that when a broker sends an update CommandPartitionUpdate, how
> do you know it arrived successfully? From my memory, there is no ACK in the
> protocol, saying "I'm the client, I got the update successfully" and only
> then it removed the "dirty" flag for that topic, for this watcher ID.
>
> Are there any other edge cases we can have? Let's be exhaustive.
>
>
>
> On Wed, Feb 22, 2023 at 1:14 PM houxiaoyu  wrote:
>
> > Thanks for your great suggestion Enrico.
> >
> > I agreed with you. It's more reasonable to add a
> > `supports_partition_update_watchers`  in `FeatureFlags`  to detect that
> the
> > connected broker supporting this feature , and add a new broker
> > configuration property `enableNotificationForPartitionUpdate` with
> default
> > value true, which is much like PIP-145.
> >
> > I have updated the descriptions.
> >
> > Enrico Olivelli  于2023年2月22日周三 17:26写道:
> >
> > > I support this proposal.
> > > Coping here my comments from GH:
> > >
> > > can't we enable this by default in case we detect that the connected
> > > Broker supports it ?
> > > I can't find any reason for not using this mechanism if it is
> available.
> > >
> > > Maybe we can set the default to "true" and allow users to disable it
> > > in case it impacts their systems in an unwanted way.
> > >
> > > Maybe It would be useful to have a way to disable the mechanism on the
> > > broker side as well
> > >
> > > Enrico
> > >
> > > Il giorno mer 22 feb 2023 alle ore 10:22 houxiaoyu
> > >  ha scritto:
> > > >
> > > > Hi Pulsar community:
> > > >
> > > > I opened a PIP to discuss "Notifications for partitions update"
> > > >
> > > > ### Motivation
> > > >
> > > > Pulsar client will poll brokers at fix time for checking the
> partitions
> > > > update if we publish/subscribe the partitioned topics with
> > > > 

[DISCUSS] PIP-250: Add proxyVersion to CommandConnect

2023-02-23 Thread Michael Marshall
Hi Pulsar Community,

In talking with Zike Yang on
https://github.com/apache/pulsar/pull/19540, we identified that it
would be helpful for the proxy to forward its own version when
connecting to the broker. Here is a related PIP to improve the
connection information available to operators.

Issue: https://github.com/apache/pulsar/issues/19623
Implementation: https://github.com/apache/pulsar/pull/19618

I look forward to your feedback!

Thanks,
Michael

Text of PIP copied below:

### Motivation

When clients connect through the proxy, it is valuable to know which
version of the proxy connected to the broker. That information isn't
currently logged or reported in any easily identifiable way. The only
way to get information about the connection is to infer which proxy
forwarded a connection based on matching up the IP address in the
logs.

An additional change proposed in the implementation is to log this new
information along with the `clientVersion`, `clientProtocolVersion`,
and relevant authentication role information. This information will
improve debug-ability and could also serve as a form of audit logging.

### Goal

Improve the value of the broker's logs and metrics about connections
to simplify debugging and to make it easier for Pulsar operators to
understand how clients are connecting to their clusters.

### API Changes

Add the following:

```proto
message CommandConnect {
 // Other fields omitted
optional string proxy_version = 11; // Version of the proxy.
Should only be forwarded by a proxy.
}
```

### Implementation

Initial implementation: https://github.com/apache/pulsar/pull/19618

### Alternatives

The `CommandAuthResponse` has a `client_version` field. It's possible
that someone would want this `proxy_version` field on that message.
However, I think we should not continue down the path of duplicating
fields in the connection handshake protocol.

### Anything else?

Future work could include exposing this `proxyVersion` and
`clientVersion` information via Prometheus metrics.


Re: TLS regression verification for Python client 3.1.0 and Node.js client 1.8.1

2023-02-23 Thread Zixuan Liu
This is not very friendly to explicitly set the ca file.

Can we dynamically search the system ca file? and then go to set the ca
file to the libcurl.

The following are ca files from golang codebase(this  is what you
mentioned):
```
// Possible certificate files; stop after finding one.
var certFiles = []string{
"/etc/ssl/certs/ca-certificates.crt",//
Debian/Ubuntu/Gentoo etc.
"/etc/pki/tls/certs/ca-bundle.crt",  // Fedora/RHEL 6
"/etc/ssl/ca-bundle.pem",// OpenSUSE
"/etc/pki/tls/cacert.pem",   // OpenELEC
"/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // CentOS/RHEL 7
"/etc/ssl/cert.pem", // Alpine Linux
}

// Possible directories with certificate files; all will be read.
var certDirectories = []string{
"/etc/ssl/certs",   // SLES10/SLES11,
https://golang.org/issue/12139
"/etc/pki/tls/certs",   // Fedora/RHEL
"/system/etc/security/cacerts", // Android
}
```

Thanks,
Zixuan



Yunze Xu  于2023年2月24日周五 02:06写道:

> I've figured out why the Python client does not suffer from this
> issue. I use `strace` to print all system calls. Then I find the
> Python client reads another cert file:
>
> ```
> openat(AT_FDCWD,
> "/usr/local/lib/python3.8/dist-packages/certifi/cacert.pem", O_RDONLY)
> = 6
> ```
>
> The correct cert comes from the `certifi` package.
>
> While the Node.js client first tries to read `/etc/ssl/cert.pem`, then
> tries to read `/etc/pki/tls/certs/ca-bundle.crt` that is the libcurl's
> bundle CA cert location. Both of them don't exist.
>
> ```
> 1828 openat(AT_FDCWD, "/etc/ssl/cert.pem", O_RDONLY) = -1 ENOENT (No
> such file or directory)
> 1891 openat(AT_FDCWD, "/etc/pki/tls/certs/ca-bundle.crt", O_RDONLY) =
> -1 ENOENT (No such file or directory)
> ```
>
> Thanks,
> Yunze
>
> On Thu, Feb 23, 2023 at 11:50 PM Yunze Xu  wrote:
> >
> > Hi all,
> >
> > Currently there are two ongoing releases: Python client 3.1.0 [1] and
> > Node.js client 1.8.1 [2]. Both these two releases depend on the C++
> > client 3.1.2, which fixes an issue that when performing OAuth2
> > authentication with an issuer URL whose protocol is HTTPS, users can
> > configure the tls cert file path to fix it.
> >
> > However, a regression was reported in the vote mail list of Node.js
> > client [2]. In short, since Node.js client 1.8.0, users have to
> > configure the tls cert file path explicitly, while the path could be
> > detected automatically before. Since the Python client is also based
> > on the C++ client and adopts a similar way to build the binaries, I've
> > thought it should suffer the same issue. While the test result shows
> > the Python client works well.
> >
> > So I created a repository [3] to:
> > - Give an example for how to test Python and Node.js clients
> > - Report my test results on various operating systems
> >
> > Please also help verify these two candidate releases.
> >
> > [1] https://lists.apache.org/thread/0mcdfw6ogof2bcl40m9p3gjzwolr8rw2
> > [2] https://lists.apache.org/thread/n8jrg29nmozxqp588dbd8nkm7y18sphf
> > [3] https://github.com/BewareMyPower/pulsar-tls-examples
> >
> > Thanks,
> > Yunze
>


Re: [DISCUSS]Add an internal class to `TransactionBufferStats` to record the snapshot status uniformly.

2023-02-23 Thread Xiangying Meng
Hi Asaf,
> How are those stats exposed to the user?
IMO, these stats should be exposed to the user.
The users can use it to check if the transaction snapshot is
running correctly.

Thanks,
Xiangying

On Wed, Feb 22, 2023 at 9:06 PM Asaf Mesika  wrote:

> How are those stats exposed to the user?
>
>
> On Mon, Feb 20, 2023 at 6:01 AM Xiangying Meng 
> wrote:
>
> > Hi, Community,
> > We plan to add an internal class to `TransactionBufferStats` to record
> the
> > snapshot status uniformly.
> > As we all know, the current transaction buffer(TB) filters the messages
> > sent using the aborted transaction by storing the aborted ID in TB.
> > Then TB will periodically store these aborted txn IDs in a bookie entry
> in
> > the form of snapshots so that TB can recover faster when recovering.
> > But as more and more people use transactions, we found that in some
> extreme
> > cases, a bookie entry may not be able to store all aborted transaction
> IDs.
> > So in PIP196 , we
> > implemented the multiple-snapshot function.
> > As the transaction buffer snapshot mechanism becomes increasingly
> complex,
> > the only information related to the transaction snapshot is
> > `lastSnapshotTimestamps`; That is not enough, we need to add more info to
> > record the snapshot stats.
> > So I suggest adding an internal class SnapshotStats to
> > TransactionBufferStats to record the snapshot status uniformly.
> >
> > The modification could be :
> > ```java
> > public class TransactionBufferStats {
> > ...
> > public long lastSnapshotTimestamps;
> > ...
> > }
> > ```
> > ```java
> > public class TransactionBufferStats {
> > ...
> > //public long lastSnapshotTimestamps;
> > ...
> > public SnapshotStats snapshotStats;
> >
> > public static class SnapshotStats {
> > public long segmentsSize;
> >
> > public long unsealedAbortTxnIDs;
> >
> >
> > public long lastSnapshotTimestamps;
> > }
> > }
> >
> > ```
> > Thanks.
> > Xiangying
> >
>


Re: [DISCUSS] PIP-249: Pulsar website redesign

2023-02-23 Thread Yu
Hi @asafm and @emidio-cardeira thanks for your great work!

You've contributed new designs (with colors/elements/... that have not been
used) for Pulsar, which is a big step for the community.

Preiviously we've collected some rules at Pulsar Design Style Guide [1],
which are used to create illustrations in docs.

If this new look is accepted, can you document the design rules/best
practice? They should be available in the Contribution Guide [2].

In this way, other contributors can follow and create designs in high
quality and consistent style.

Thanks!

~

[1]
https://docs.google.com/document/d/16Hp7Sc86MQtL0m8fc2w_TrcKXAuglwRwHmdmwfk00mI/edit#heading=h.b8ogodj5sj

[2] https://pulsar.apache.org/contribute/


On Fri, Feb 24, 2023 at 5:27 AM Asaf Mesika  wrote:

> Hi,
>
> PIP issue: https://github.com/apache/pulsar/issues/19611
>
> One of the key elements that attracted me to Pulsar was how awesome it is -
> built in 2012 yet fits the new strategy of being cloud-native - simply
> incredible. It's not only that - it has so many outstanding features,
> making it a cutting edge, modern powerhouse in the messaging system
> category.
>
> Yet, its site does not reflect that.
> It does the opposite.
> People new to Pulsar, not knowing it as we do in the community, would think
> it's an old, unmaintained technology, and maybe not even bother reading the
> landing page.
>
> This is why I embarked on a journey to improve Pulsar web-site, in bite
> sizes. My first step was improving the landing page copy for the elevator
> pitch, opening paragraph, and the feature descriptions (In this PR
> )
>
> Today, together with my friend, co-worker and designer Emidio, I would like
> to propose the second step: revamp the landing page structure and most
> importantly - the website look and feel.
>
> The PIP  contains:
> * The screenshots for it
> * A "live" Figma view for it
> * Detailed explanation of what was changed and why.
>
> I think you'll love it.
>
> Happy to hear your thoughts.
>
>
> Asaf Mesika
>


[DISCUSS] PIP-249: Pulsar website redesign

2023-02-23 Thread Asaf Mesika
Hi,

PIP issue: https://github.com/apache/pulsar/issues/19611

One of the key elements that attracted me to Pulsar was how awesome it is -
built in 2012 yet fits the new strategy of being cloud-native - simply
incredible. It's not only that - it has so many outstanding features,
making it a cutting edge, modern powerhouse in the messaging system
category.

Yet, its site does not reflect that.
It does the opposite.
People new to Pulsar, not knowing it as we do in the community, would think
it's an old, unmaintained technology, and maybe not even bother reading the
landing page.

This is why I embarked on a journey to improve Pulsar web-site, in bite
sizes. My first step was improving the landing page copy for the elevator
pitch, opening paragraph, and the feature descriptions (In this PR
)

Today, together with my friend, co-worker and designer Emidio, I would like
to propose the second step: revamp the landing page structure and most
importantly - the website look and feel.

The PIP  contains:
* The screenshots for it
* A "live" Figma view for it
* Detailed explanation of what was changed and why.

I think you'll love it.

Happy to hear your thoughts.


Asaf Mesika


Re: [DISCUSS] ClientVersion conflict across multiple language client

2023-02-23 Thread Michael Marshall
> Go:
> ClientVersionString = "Pulsar Go " + Version
> https://github.com/apache/pulsar-client-go/blob/dedbdc45c63b06e6a12356785418cd906d6bab3c/pulsar/internal/version.go#L43

Because we filter out the official go client, I propose that we remove
the filtering of certain client versions:
https://github.com/apache/pulsar/pull/19616.

Thanks,
Michael

On Thu, Feb 23, 2023 at 12:45 PM Michael Marshall  wrote:
>
> I found another data point in favor of moving in this direction. Our
> protocol spec says the following about the client_version field:
>
> message CommandConnect {
> "client_version" : "Pulsar-Client-Java-v1.15.2",
> "auth_method_name" : "my-authentication-plugin",
> "auth_data" : "my-auth-data",
> "protocol_version" : 6
> }
>
> * `client_version`: String-based identifier. Format is not enforced.
>
> https://github.com/apache/pulsar-site/blame/main/docs/developing-binary-protocol.md#L133
>
> I think it makes sense to make the format
> "pulsar--" for official clients. We could then
> recommend third party clients replace "pulsar" with a relevant org
> name. There isn't a way to enforce the version string though, so these
> would only be recommendations.
>
> For official clients, here are our version strings:
>
> Go:
> ClientVersionString = "Pulsar Go " + Version
> https://github.com/apache/pulsar-client-go/blob/dedbdc45c63b06e6a12356785418cd906d6bab3c/pulsar/internal/version.go#L43
>
> C++
> connect->set_client_version(PULSAR_VERSION_STR);
> https://github.com/apache/pulsar-client-cpp/blob/96507cecdc90f10eca5febc48a0623f72d338615/lib/Commands.cc#L269
> std::string version = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
> https://github.com/apache/pulsar-client-cpp/blob/05807bdaf3a4341b22efd7c71f7b00c47cc31413/lib/HTTPLookupService.cc#L195
>
> I took a quick survey of some existing third party clients. Here are
> the results for how the version string is written:
>
> Rust:
> client_version: String::from("2.0.1-incubating"),
> https://github.com/streamnative/pulsar-rs/blob/eb87ef796bd8cb42fc72bb72ed14751fffa437e1/src/connection.rs#L1173
>
> Haskell:
> & F.clientVersion .~ "Pulsar-Client-Haskell-v" <> T.pack (showVersion version)
> https://github.com/cr-org/supernova/blob/602409a18f47a38541ba24f5e885199efd383f48/lib/src/Pulsar/Protocol/Commands.hs#L22
>
> PHP:
> $cmd->setClientVersion(sprintf('ikilobyte/pulsar-client-php@v%s',
> Client::VERSION_ID));
> https://github.com/ikilobyte/pulsar-client-php/blob/e8847c62d3bf136886312a14a04f51c59ca99886/src/IO/StreamIO.php#L77
>
> F#:
> I'm not exactly sure how to interpret this input.
> https://github.com/fsprojects/pulsar-client-dotnet/blob/5c6dcff0184f87005f55dcc847893a889aaee2ad/src/Pulsar.Client/Internal/ClientCnx.fs#L119
>
> Scala:
> All appear to wrap the Pulsar Java client (or don't have a
> clientVersion string in their code base).
>
> Thanks,
> Michael
>
> On Thu, Feb 23, 2023 at 11:39 AM Michael Marshall  
> wrote:
> >
> > > A better solution is that we could allow the
> > > user to specify the suffix of the client version.
> >
> > I don't think we need to make it configurable because the library
> > owner, not the user, has full control when creating the Connect
> > command.
> >
> > We can update our protocol spec to recommend appropriate usage and of the 
> > field.
> >
> > Thanks,
> > Michael
> >
> > On Thu, Feb 23, 2023 at 2:51 AM Zike Yang  wrote:
> > >
> > > > But I'm wondering if it's possible to support configuring the client
> > > version string by users?
> > >
> > > I think it's possible. A better solution is that we could allow the
> > > user to specify the suffix of the client version. For example, by
> > > adding a new configuration like `clientVersionSuffix` to the
> > > ClientConffiguration, the client_version would be like
> > > `java-3.0.0-forked-v1.0.0`. In this way, we could always remain the
> > > part `java-3.0.0`. But of course, we have no control over the behavior
> > > of third-party clients.
> > >
> > >
> > > Thanks,
> > > Zike Yang
> > >
> > > On Wed, Feb 22, 2023 at 5:52 AM Michael Marshall  
> > > wrote:
> > > >
> > > > +1 - I think this is a helpful change. We already do this in the Java
> > > > Admin http client when we set the user agent [0].
> > > >
> > > > > But I'm wondering if it's possible to support configuring the client
> > > > > version string by users?
> > > >
> > > > This is always the case because we support third party clients. In my
> > > > view, this field is primarily helpful for troubleshooting and for
> > > > getting weakly trusted stats on usage in a given cluster. Operators
> > > > could even use this information to determine and alert if any clients
> > > > are connecting with known vulnerabilities.
> > > >
> > > > Thanks,
> > > > Michael
> > > >
> > > > [0] 
> > > > https://github.com/apache/pulsar/blob/d8569cd4ec6da14f8b2b9338db1ed2f6a3eacf0a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java#L105
> > > >
> > > > On Tue, Feb 21, 2023 at 9:50 

Re: [DISCUSS] ClientVersion conflict across multiple language client

2023-02-23 Thread Michael Marshall
I found another data point in favor of moving in this direction. Our
protocol spec says the following about the client_version field:

message CommandConnect {
"client_version" : "Pulsar-Client-Java-v1.15.2",
"auth_method_name" : "my-authentication-plugin",
"auth_data" : "my-auth-data",
"protocol_version" : 6
}

* `client_version`: String-based identifier. Format is not enforced.

https://github.com/apache/pulsar-site/blame/main/docs/developing-binary-protocol.md#L133

I think it makes sense to make the format
"pulsar--" for official clients. We could then
recommend third party clients replace "pulsar" with a relevant org
name. There isn't a way to enforce the version string though, so these
would only be recommendations.

For official clients, here are our version strings:

Go:
ClientVersionString = "Pulsar Go " + Version
https://github.com/apache/pulsar-client-go/blob/dedbdc45c63b06e6a12356785418cd906d6bab3c/pulsar/internal/version.go#L43

C++
connect->set_client_version(PULSAR_VERSION_STR);
https://github.com/apache/pulsar-client-cpp/blob/96507cecdc90f10eca5febc48a0623f72d338615/lib/Commands.cc#L269
std::string version = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
https://github.com/apache/pulsar-client-cpp/blob/05807bdaf3a4341b22efd7c71f7b00c47cc31413/lib/HTTPLookupService.cc#L195

I took a quick survey of some existing third party clients. Here are
the results for how the version string is written:

Rust:
client_version: String::from("2.0.1-incubating"),
https://github.com/streamnative/pulsar-rs/blob/eb87ef796bd8cb42fc72bb72ed14751fffa437e1/src/connection.rs#L1173

Haskell:
& F.clientVersion .~ "Pulsar-Client-Haskell-v" <> T.pack (showVersion version)
https://github.com/cr-org/supernova/blob/602409a18f47a38541ba24f5e885199efd383f48/lib/src/Pulsar/Protocol/Commands.hs#L22

PHP:
$cmd->setClientVersion(sprintf('ikilobyte/pulsar-client-php@v%s',
Client::VERSION_ID));
https://github.com/ikilobyte/pulsar-client-php/blob/e8847c62d3bf136886312a14a04f51c59ca99886/src/IO/StreamIO.php#L77

F#:
I'm not exactly sure how to interpret this input.
https://github.com/fsprojects/pulsar-client-dotnet/blob/5c6dcff0184f87005f55dcc847893a889aaee2ad/src/Pulsar.Client/Internal/ClientCnx.fs#L119

Scala:
All appear to wrap the Pulsar Java client (or don't have a
clientVersion string in their code base).

Thanks,
Michael

On Thu, Feb 23, 2023 at 11:39 AM Michael Marshall  wrote:
>
> > A better solution is that we could allow the
> > user to specify the suffix of the client version.
>
> I don't think we need to make it configurable because the library
> owner, not the user, has full control when creating the Connect
> command.
>
> We can update our protocol spec to recommend appropriate usage and of the 
> field.
>
> Thanks,
> Michael
>
> On Thu, Feb 23, 2023 at 2:51 AM Zike Yang  wrote:
> >
> > > But I'm wondering if it's possible to support configuring the client
> > version string by users?
> >
> > I think it's possible. A better solution is that we could allow the
> > user to specify the suffix of the client version. For example, by
> > adding a new configuration like `clientVersionSuffix` to the
> > ClientConffiguration, the client_version would be like
> > `java-3.0.0-forked-v1.0.0`. In this way, we could always remain the
> > part `java-3.0.0`. But of course, we have no control over the behavior
> > of third-party clients.
> >
> >
> > Thanks,
> > Zike Yang
> >
> > On Wed, Feb 22, 2023 at 5:52 AM Michael Marshall  
> > wrote:
> > >
> > > +1 - I think this is a helpful change. We already do this in the Java
> > > Admin http client when we set the user agent [0].
> > >
> > > > But I'm wondering if it's possible to support configuring the client
> > > > version string by users?
> > >
> > > This is always the case because we support third party clients. In my
> > > view, this field is primarily helpful for troubleshooting and for
> > > getting weakly trusted stats on usage in a given cluster. Operators
> > > could even use this information to determine and alert if any clients
> > > are connecting with known vulnerabilities.
> > >
> > > Thanks,
> > > Michael
> > >
> > > [0] 
> > > https://github.com/apache/pulsar/blob/d8569cd4ec6da14f8b2b9338db1ed2f6a3eacf0a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java#L105
> > >
> > > On Tue, Feb 21, 2023 at 9:50 AM Yunze Xu  
> > > wrote:
> > > >
> > > > +1
> > > >
> > > > But I'm wondering if it's possible to support configuring the client
> > > > version string by users? For example, if users forked their own
> > > > client, they might want to differ the client version with the official
> > > > client. The disadvantage could be that users can configure any version
> > > > string as they want.
> > > >
> > > > Thanks,
> > > > Yunze
> > > >
> > > > On Tue, Feb 21, 2023 at 7:23 PM Enrico Olivelli  
> > > > wrote:
> > > > >
> > > > > +1
> > > > >
> > > > > Enrico
> > > > >
> > > > > Il giorno mar 21 feb 2023 alle ore 10:23 Baodi 

Re: TLS regression verification for Python client 3.1.0 and Node.js client 1.8.1

2023-02-23 Thread Yunze Xu
I've figured out why the Python client does not suffer from this
issue. I use `strace` to print all system calls. Then I find the
Python client reads another cert file:

```
openat(AT_FDCWD,
"/usr/local/lib/python3.8/dist-packages/certifi/cacert.pem", O_RDONLY)
= 6
```

The correct cert comes from the `certifi` package.

While the Node.js client first tries to read `/etc/ssl/cert.pem`, then
tries to read `/etc/pki/tls/certs/ca-bundle.crt` that is the libcurl's
bundle CA cert location. Both of them don't exist.

```
1828 openat(AT_FDCWD, "/etc/ssl/cert.pem", O_RDONLY) = -1 ENOENT (No
such file or directory)
1891 openat(AT_FDCWD, "/etc/pki/tls/certs/ca-bundle.crt", O_RDONLY) =
-1 ENOENT (No such file or directory)
```

Thanks,
Yunze

On Thu, Feb 23, 2023 at 11:50 PM Yunze Xu  wrote:
>
> Hi all,
>
> Currently there are two ongoing releases: Python client 3.1.0 [1] and
> Node.js client 1.8.1 [2]. Both these two releases depend on the C++
> client 3.1.2, which fixes an issue that when performing OAuth2
> authentication with an issuer URL whose protocol is HTTPS, users can
> configure the tls cert file path to fix it.
>
> However, a regression was reported in the vote mail list of Node.js
> client [2]. In short, since Node.js client 1.8.0, users have to
> configure the tls cert file path explicitly, while the path could be
> detected automatically before. Since the Python client is also based
> on the C++ client and adopts a similar way to build the binaries, I've
> thought it should suffer the same issue. While the test result shows
> the Python client works well.
>
> So I created a repository [3] to:
> - Give an example for how to test Python and Node.js clients
> - Report my test results on various operating systems
>
> Please also help verify these two candidate releases.
>
> [1] https://lists.apache.org/thread/0mcdfw6ogof2bcl40m9p3gjzwolr8rw2
> [2] https://lists.apache.org/thread/n8jrg29nmozxqp588dbd8nkm7y18sphf
> [3] https://github.com/BewareMyPower/pulsar-tls-examples
>
> Thanks,
> Yunze


Re: [DISCUSS] ClientVersion conflict across multiple language client

2023-02-23 Thread Michael Marshall
> A better solution is that we could allow the
> user to specify the suffix of the client version.

I don't think we need to make it configurable because the library
owner, not the user, has full control when creating the Connect
command.

We can update our protocol spec to recommend appropriate usage and of the field.

Thanks,
Michael

On Thu, Feb 23, 2023 at 2:51 AM Zike Yang  wrote:
>
> > But I'm wondering if it's possible to support configuring the client
> version string by users?
>
> I think it's possible. A better solution is that we could allow the
> user to specify the suffix of the client version. For example, by
> adding a new configuration like `clientVersionSuffix` to the
> ClientConffiguration, the client_version would be like
> `java-3.0.0-forked-v1.0.0`. In this way, we could always remain the
> part `java-3.0.0`. But of course, we have no control over the behavior
> of third-party clients.
>
>
> Thanks,
> Zike Yang
>
> On Wed, Feb 22, 2023 at 5:52 AM Michael Marshall  wrote:
> >
> > +1 - I think this is a helpful change. We already do this in the Java
> > Admin http client when we set the user agent [0].
> >
> > > But I'm wondering if it's possible to support configuring the client
> > > version string by users?
> >
> > This is always the case because we support third party clients. In my
> > view, this field is primarily helpful for troubleshooting and for
> > getting weakly trusted stats on usage in a given cluster. Operators
> > could even use this information to determine and alert if any clients
> > are connecting with known vulnerabilities.
> >
> > Thanks,
> > Michael
> >
> > [0] 
> > https://github.com/apache/pulsar/blob/d8569cd4ec6da14f8b2b9338db1ed2f6a3eacf0a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java#L105
> >
> > On Tue, Feb 21, 2023 at 9:50 AM Yunze Xu  
> > wrote:
> > >
> > > +1
> > >
> > > But I'm wondering if it's possible to support configuring the client
> > > version string by users? For example, if users forked their own
> > > client, they might want to differ the client version with the official
> > > client. The disadvantage could be that users can configure any version
> > > string as they want.
> > >
> > > Thanks,
> > > Yunze
> > >
> > > On Tue, Feb 21, 2023 at 7:23 PM Enrico Olivelli  
> > > wrote:
> > > >
> > > > +1
> > > >
> > > > Enrico
> > > >
> > > > Il giorno mar 21 feb 2023 alle ore 10:23 Baodi Shi 
> > > > ha scritto:
> > > > >
> > > > >  +1, Good idea.
> > > > >
> > > > > Thanks,
> > > > > Baodi Shi
> > > > >
> > > > >
> > > > > 在 2023年2月21日 15:24:45 上,avinash kala  写道:
> > > > >
> > > > > > +1, sounds good.
> > > > > >
> > > > > > On Tue, Feb 21, 2023, 12:39 PM Zixuan Liu  wrote:
> > > > > >
> > > > > > +1, good idea!
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Zixuan
> > > > > >
> > > > > >
> > > > > > Zike Yang  于2023年2月21日周二 11:33写道:
> > > > > >
> > > > > >
> > > > > > > Hi, all
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > Currently, the Pulsar broker uses the field `client_version` to 
> > > > > > > get
> > > > > >
> > > > > > > the version of the client. The client will send the 
> > > > > > > client_version to
> > > > > >
> > > > > > > the broker through `CommandConnect` [0] during the connect or
> > > > > >
> > > > > > > `CommandAuthResponse ` [1] during the auth challenge. We could 
> > > > > > > get the
> > > > > >
> > > > > > > client_version from the admin using `pulsar-admin topics stats 
> > > > > > > xxx`
> > > > > >
> > > > > > > [2].
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > But the `client_version ` only shows the version information. And
> > > > > >
> > > > > > > clients in different languages use their own separate version 
> > > > > > > numbers.
> > > > > >
> > > > > > > This can lead to conflict. For example, the java client 2.10.2 
> > > > > > > uses
> > > > > >
> > > > > > > the same value `2.10.2` as the C++ client 2.10.2. And C++ client 
> > > > > > > 3.0.0
> > > > > >
> > > > > > > may conflict with the future java client 3.0.0. The information of
> > > > > >
> > > > > > > `client_version` is incomplete. We could not determine what
> > > > > >
> > > > > > > language(or specific library) the client uses. This raises
> > > > > >
> > > > > > > inconvenience for debugging.
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > I would like to propose adding the client language information to 
> > > > > > > the
> > > > > >
> > > > > > > `client_version`. Like:
> > > > > >
> > > > > > > * `java-2.11.1` for Java client 2.11
> > > > > >
> > > > > > > * `cpp-3.1.2` for C++ client 3.1.2
> > > > > >
> > > > > > > * `go-0.9.0` for go client 0.9.0
> > > > > >
> > > > > > > We can easily do that because the `client_version` is a string 
> > > > > > > type.
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > In addition, the Nodejs client and Python client all use the
> > > > > >
> > > > > > > client_version of C++ client they depend on. So it will 

TLS regression verification for Python client 3.1.0 and Node.js client 1.8.1

2023-02-23 Thread Yunze Xu
Hi all,

Currently there are two ongoing releases: Python client 3.1.0 [1] and
Node.js client 1.8.1 [2]. Both these two releases depend on the C++
client 3.1.2, which fixes an issue that when performing OAuth2
authentication with an issuer URL whose protocol is HTTPS, users can
configure the tls cert file path to fix it.

However, a regression was reported in the vote mail list of Node.js
client [2]. In short, since Node.js client 1.8.0, users have to
configure the tls cert file path explicitly, while the path could be
detected automatically before. Since the Python client is also based
on the C++ client and adopts a similar way to build the binaries, I've
thought it should suffer the same issue. While the test result shows
the Python client works well.

So I created a repository [3] to:
- Give an example for how to test Python and Node.js clients
- Report my test results on various operating systems

Please also help verify these two candidate releases.

[1] https://lists.apache.org/thread/0mcdfw6ogof2bcl40m9p3gjzwolr8rw2
[2] https://lists.apache.org/thread/n8jrg29nmozxqp588dbd8nkm7y18sphf
[3] https://github.com/BewareMyPower/pulsar-tls-examples

Thanks,
Yunze


Re: [VOTE] Pulsar Client Python Release 3.1.0 Candidate 3

2023-02-23 Thread Enrico Olivelli
Thank you Yunze for double checking.

I don't have time to test the release, so I am voting +0 (and not -1)

Enrico

Il giorno gio 23 feb 2023 alle ore 12:57 Yunze Xu
 ha scritto:
>
> Hi Enrico,
>
> I will test more operation systems and open a discussion soon. For
> now, I just tested the TLS encryption and token authentication, and I
> found this issue does not exist as expected for both Python and
> Node.js clients.
> - Windows: Only Python client works
> - Ubuntu: Both work
> - macOS: Not tested yet
>
> But the OAuth2 authentication case doesn't work. It's caused by
> https://github.com/apache/pulsar/pull/16064 and fixed by
> https://github.com/apache/pulsar-client-cpp/pull/190.
>
> When the protocol of the issuer URL is HTTPS:
> - Before #16064, OAuth2 authentication works by skipping verifying the
> peer, it's dangerous for security reasons
> - After #16064 and before #190, there is no way to perform OAuth2 
> authentication
> - After #190, users can configure the `tls_trust_certs_file_path` to do that.
>
> So I think it should not be treated as a regression and should not
> block this release. We can support automatically detecting the CA
> certs for OAuth2 authentication later.
>
> The regression should be the case when the TLS encryption is enabled.
> And as I've said at the beginning, I'm going to do more tests and open
> a discussion.
>
> Thanks,
> Yunze
>
> On Thu, Feb 23, 2023 at 6:06 PM Enrico Olivelli  wrote:
> >
> > Does this version have the same problems about TLS CA Certs as the
> > NodeJS client ?
> > In that case I would cast a -1
> >
> > Enrico
> >
> > Il giorno gio 23 feb 2023 alle ore 01:21 Matteo Merli
> >  ha scritto:
> > >
> > > +1 bindind
> > > --
> > > Matteo Merli
> > > 
> > >
> > >
> > > On Fri, Feb 17, 2023 at 3:27 AM Yunze Xu 
> > > wrote:
> > >
> > > > This is the 3rd release candidate for Apache Pulsar Client Python,
> > > > version 3.1.0.
> > > >
> > > > It fixes the following issues:
> > > > https://github.com/apache/pulsar-client-python/milestone/2?closed=1
> > > >
> > > > *** Please download, test and vote on this release. This vote will
> > > > stay open for at least 72 hours ***
> > > >
> > > > Python wheels:
> > > >
> > > > https://dist.apache.org/repos/dist/dev/pulsar/pulsar-client-python-3.1.0-candidate-3/
> > > >
> > > > The supported python versions are 3.7, 3.8, 3.9, 3.10 and 3.11. The
> > > > supported platforms and architectures are:
> > > > - Windows x86_64 (windows/)
> > > > - glibc-based Linux x86_64 (linux-glibc-x86_64/)
> > > > - glibc-based Linux arm64 (linux-glibc-arm64/)
> > > > - musl-based Linux x86_64 (linux-musl-x86_64/)
> > > > - musl-based Linux arm64 (linux-musl-arm64/)
> > > > - macOS universal 2 (macos/)
> > > >
> > > > You can download the wheel (the `.whl` file) according to your own OS
> > > > and Python version
> > > > and install the wheel:
> > > > - Windows: `py -m pip install *.whl --force-reinstall`
> > > > - Linux or macOS: `python3 -m pip install *.whl --force-reinstall`
> > > >
> > > > The tag to be voted upon: v3.1.0-candidate-3
> > > > (9ed92ecee632c42b81a3198b8824d70d080af7f0)
> > > >
> > > > https://github.com/apache/pulsar-client-python/releases/tag/v3.1.0-candidate-3
> > > >
> > > > Pulsar's KEYS file containing PGP keys you use to sign the release:
> > > > https://dist.apache.org/repos/dist/release/pulsar/KEYS
> > > >
> > > > Please download the Python wheels and follow the README to test.
> > > >


Re: [VOTE] Pulsar Client Python Release 3.1.0 Candidate 3

2023-02-23 Thread Yunze Xu
Hi Enrico,

I will test more operation systems and open a discussion soon. For
now, I just tested the TLS encryption and token authentication, and I
found this issue does not exist as expected for both Python and
Node.js clients.
- Windows: Only Python client works
- Ubuntu: Both work
- macOS: Not tested yet

But the OAuth2 authentication case doesn't work. It's caused by
https://github.com/apache/pulsar/pull/16064 and fixed by
https://github.com/apache/pulsar-client-cpp/pull/190.

When the protocol of the issuer URL is HTTPS:
- Before #16064, OAuth2 authentication works by skipping verifying the
peer, it's dangerous for security reasons
- After #16064 and before #190, there is no way to perform OAuth2 authentication
- After #190, users can configure the `tls_trust_certs_file_path` to do that.

So I think it should not be treated as a regression and should not
block this release. We can support automatically detecting the CA
certs for OAuth2 authentication later.

The regression should be the case when the TLS encryption is enabled.
And as I've said at the beginning, I'm going to do more tests and open
a discussion.

Thanks,
Yunze

On Thu, Feb 23, 2023 at 6:06 PM Enrico Olivelli  wrote:
>
> Does this version have the same problems about TLS CA Certs as the
> NodeJS client ?
> In that case I would cast a -1
>
> Enrico
>
> Il giorno gio 23 feb 2023 alle ore 01:21 Matteo Merli
>  ha scritto:
> >
> > +1 bindind
> > --
> > Matteo Merli
> > 
> >
> >
> > On Fri, Feb 17, 2023 at 3:27 AM Yunze Xu 
> > wrote:
> >
> > > This is the 3rd release candidate for Apache Pulsar Client Python,
> > > version 3.1.0.
> > >
> > > It fixes the following issues:
> > > https://github.com/apache/pulsar-client-python/milestone/2?closed=1
> > >
> > > *** Please download, test and vote on this release. This vote will
> > > stay open for at least 72 hours ***
> > >
> > > Python wheels:
> > >
> > > https://dist.apache.org/repos/dist/dev/pulsar/pulsar-client-python-3.1.0-candidate-3/
> > >
> > > The supported python versions are 3.7, 3.8, 3.9, 3.10 and 3.11. The
> > > supported platforms and architectures are:
> > > - Windows x86_64 (windows/)
> > > - glibc-based Linux x86_64 (linux-glibc-x86_64/)
> > > - glibc-based Linux arm64 (linux-glibc-arm64/)
> > > - musl-based Linux x86_64 (linux-musl-x86_64/)
> > > - musl-based Linux arm64 (linux-musl-arm64/)
> > > - macOS universal 2 (macos/)
> > >
> > > You can download the wheel (the `.whl` file) according to your own OS
> > > and Python version
> > > and install the wheel:
> > > - Windows: `py -m pip install *.whl --force-reinstall`
> > > - Linux or macOS: `python3 -m pip install *.whl --force-reinstall`
> > >
> > > The tag to be voted upon: v3.1.0-candidate-3
> > > (9ed92ecee632c42b81a3198b8824d70d080af7f0)
> > >
> > > https://github.com/apache/pulsar-client-python/releases/tag/v3.1.0-candidate-3
> > >
> > > Pulsar's KEYS file containing PGP keys you use to sign the release:
> > > https://dist.apache.org/repos/dist/release/pulsar/KEYS
> > >
> > > Please download the Python wheels and follow the README to test.
> > >


Re: [VOTE] Pulsar Client Python Release 3.1.0 Candidate 3

2023-02-23 Thread Enrico Olivelli
Does this version have the same problems about TLS CA Certs as the
NodeJS client ?
In that case I would cast a -1

Enrico

Il giorno gio 23 feb 2023 alle ore 01:21 Matteo Merli
 ha scritto:
>
> +1 bindind
> --
> Matteo Merli
> 
>
>
> On Fri, Feb 17, 2023 at 3:27 AM Yunze Xu 
> wrote:
>
> > This is the 3rd release candidate for Apache Pulsar Client Python,
> > version 3.1.0.
> >
> > It fixes the following issues:
> > https://github.com/apache/pulsar-client-python/milestone/2?closed=1
> >
> > *** Please download, test and vote on this release. This vote will
> > stay open for at least 72 hours ***
> >
> > Python wheels:
> >
> > https://dist.apache.org/repos/dist/dev/pulsar/pulsar-client-python-3.1.0-candidate-3/
> >
> > The supported python versions are 3.7, 3.8, 3.9, 3.10 and 3.11. The
> > supported platforms and architectures are:
> > - Windows x86_64 (windows/)
> > - glibc-based Linux x86_64 (linux-glibc-x86_64/)
> > - glibc-based Linux arm64 (linux-glibc-arm64/)
> > - musl-based Linux x86_64 (linux-musl-x86_64/)
> > - musl-based Linux arm64 (linux-musl-arm64/)
> > - macOS universal 2 (macos/)
> >
> > You can download the wheel (the `.whl` file) according to your own OS
> > and Python version
> > and install the wheel:
> > - Windows: `py -m pip install *.whl --force-reinstall`
> > - Linux or macOS: `python3 -m pip install *.whl --force-reinstall`
> >
> > The tag to be voted upon: v3.1.0-candidate-3
> > (9ed92ecee632c42b81a3198b8824d70d080af7f0)
> >
> > https://github.com/apache/pulsar-client-python/releases/tag/v3.1.0-candidate-3
> >
> > Pulsar's KEYS file containing PGP keys you use to sign the release:
> > https://dist.apache.org/repos/dist/release/pulsar/KEYS
> >
> > Please download the Python wheels and follow the README to test.
> >


Re: [DISCUSS] ClientVersion conflict across multiple language client

2023-02-23 Thread Zike Yang
> But I'm wondering if it's possible to support configuring the client
version string by users?

I think it's possible. A better solution is that we could allow the
user to specify the suffix of the client version. For example, by
adding a new configuration like `clientVersionSuffix` to the
ClientConffiguration, the client_version would be like
`java-3.0.0-forked-v1.0.0`. In this way, we could always remain the
part `java-3.0.0`. But of course, we have no control over the behavior
of third-party clients.


Thanks,
Zike Yang

On Wed, Feb 22, 2023 at 5:52 AM Michael Marshall  wrote:
>
> +1 - I think this is a helpful change. We already do this in the Java
> Admin http client when we set the user agent [0].
>
> > But I'm wondering if it's possible to support configuring the client
> > version string by users?
>
> This is always the case because we support third party clients. In my
> view, this field is primarily helpful for troubleshooting and for
> getting weakly trusted stats on usage in a given cluster. Operators
> could even use this information to determine and alert if any clients
> are connecting with known vulnerabilities.
>
> Thanks,
> Michael
>
> [0] 
> https://github.com/apache/pulsar/blob/d8569cd4ec6da14f8b2b9338db1ed2f6a3eacf0a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java#L105
>
> On Tue, Feb 21, 2023 at 9:50 AM Yunze Xu  wrote:
> >
> > +1
> >
> > But I'm wondering if it's possible to support configuring the client
> > version string by users? For example, if users forked their own
> > client, they might want to differ the client version with the official
> > client. The disadvantage could be that users can configure any version
> > string as they want.
> >
> > Thanks,
> > Yunze
> >
> > On Tue, Feb 21, 2023 at 7:23 PM Enrico Olivelli  wrote:
> > >
> > > +1
> > >
> > > Enrico
> > >
> > > Il giorno mar 21 feb 2023 alle ore 10:23 Baodi Shi 
> > > ha scritto:
> > > >
> > > >  +1, Good idea.
> > > >
> > > > Thanks,
> > > > Baodi Shi
> > > >
> > > >
> > > > 在 2023年2月21日 15:24:45 上,avinash kala  写道:
> > > >
> > > > > +1, sounds good.
> > > > >
> > > > > On Tue, Feb 21, 2023, 12:39 PM Zixuan Liu  wrote:
> > > > >
> > > > > +1, good idea!
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Zixuan
> > > > >
> > > > >
> > > > > Zike Yang  于2023年2月21日周二 11:33写道:
> > > > >
> > > > >
> > > > > > Hi, all
> > > > >
> > > > > >
> > > > >
> > > > > > Currently, the Pulsar broker uses the field `client_version` to get
> > > > >
> > > > > > the version of the client. The client will send the client_version 
> > > > > > to
> > > > >
> > > > > > the broker through `CommandConnect` [0] during the connect or
> > > > >
> > > > > > `CommandAuthResponse ` [1] during the auth challenge. We could get 
> > > > > > the
> > > > >
> > > > > > client_version from the admin using `pulsar-admin topics stats xxx`
> > > > >
> > > > > > [2].
> > > > >
> > > > > >
> > > > >
> > > > > > But the `client_version ` only shows the version information. And
> > > > >
> > > > > > clients in different languages use their own separate version 
> > > > > > numbers.
> > > > >
> > > > > > This can lead to conflict. For example, the java client 2.10.2 uses
> > > > >
> > > > > > the same value `2.10.2` as the C++ client 2.10.2. And C++ client 
> > > > > > 3.0.0
> > > > >
> > > > > > may conflict with the future java client 3.0.0. The information of
> > > > >
> > > > > > `client_version` is incomplete. We could not determine what
> > > > >
> > > > > > language(or specific library) the client uses. This raises
> > > > >
> > > > > > inconvenience for debugging.
> > > > >
> > > > > >
> > > > >
> > > > > > I would like to propose adding the client language information to 
> > > > > > the
> > > > >
> > > > > > `client_version`. Like:
> > > > >
> > > > > > * `java-2.11.1` for Java client 2.11
> > > > >
> > > > > > * `cpp-3.1.2` for C++ client 3.1.2
> > > > >
> > > > > > * `go-0.9.0` for go client 0.9.0
> > > > >
> > > > > > We can easily do that because the `client_version` is a string type.
> > > > >
> > > > > >
> > > > >
> > > > > > In addition, the Nodejs client and Python client all use the
> > > > >
> > > > > > client_version of C++ client they depend on. So it will use `3.1.2` 
> > > > > > as
> > > > >
> > > > > > the client_verion in Nodejs client 1.8.0. This is incorrect, and I
> > > > >
> > > > > > think we need to fix them. We don't need to print the C++ client
> > > > >
> > > > > > version because we can get that information if we know the Nodejs or
> > > > >
> > > > > > Python client version.
> > > > >
> > > > > >
> > > > >
> > > > > > What do you think? Please share your thoughts if you have any ideas.
> > > > >
> > > > > >
> > > > >
> > > > > > [0]
> > > > >
> > > > > >
> > > > >
> > > > >
> > > > > https://github.com/apache/pulsar/blob/e0b50c9ec5f12d0fb8275f235d8ac00e87a9099e/pulsar-common/src/main/proto/PulsarApi.proto#L269
> > > > >
> > > > > > [1]
> > > > >
> > > > > >
> > > 

Re: [Vote] PIP-245: Make subscriptions of non-persistent topic non-durable

2023-02-23 Thread 太上玄元道君
Hi Rajan,

Even we don't persistent NonPersistentSubscription's state, but it has a
`isDurable` field
```
public NonPersistentSubscription(NonPersistentTopic topic, String
subscriptionName, boolean isDurable,
 Map properties) {
this.topic = topic;
this.topicName = topic.getName();
this.subName = subscriptionName;
this.fullName = MoreObjects.toStringHelper(this).add("topic",
topicName).add("name", subName).toString();
IS_FENCED_UPDATER.set(this, FALSE);
this.lastActive = System.currentTimeMillis();
this.isDurable = isDurable;
this.subscriptionProperties = properties != null
? Collections.unmodifiableMap(properties) :
Collections.emptyMap();
}
```
When users create a `Consumer`, the `durable` will set to be `true`.

And in `removeConsumer` method of `NonPersistentSubscription`, after
consumers disconnected, we can remove the `durable` subscription
automatically.
```
public synchronized void removeConsumer(Consumer consumer, boolean
isResetCursor) throws BrokerServiceException {
updateLastActive();
if (dispatcher != null) {
dispatcher.removeConsumer(consumer);
}
// preserve accumulative stats form removed consumer
ConsumerStatsImpl stats = consumer.getStats();
bytesOutFromRemovedConsumers.add(stats.bytesOutCounter);
msgOutFromRemovedConsumer.add(stats.msgOutCounter);
if (!isDurable) {
topic.unsubscribe(subName);
}

// invalid consumer remove will throw an exception
// decrement usage is triggered only for valid consumer close
topic.decrementUsageCount();
if (log.isDebugEnabled()) {
log.debug("[{}] [{}] [{}] Removed consumer -- count: {}",
topic.getName(), subName, consumer.consumerName(),
topic.currentUsageCount());
}
}
```

The PIP is for the purpose of deprecate `durable` subscriptions on
NonPersistentTopic for the following reasons:
1. We don't persistent NonPersistentSubscription's state, it's totally
in-memory state,  keep the `isDurable` field is meaningless and may takes
misunderstanding
2. The expired subscriptions auto clean like wrote in the PIP.

Thanks,
Tao Jiuming

Rajan Dhabalia  于2023年2月23日周四 12:33写道:

> > But for NonPersistentTopic, creating a Durable subscription is
> meaningless, NonPersistentSubscription doesn't have a ManagedCursor to
> persistent its data. After its consumer disconnected, the subscription
> couldn't be removed automatically if we didn't set the value of
> subscriptionExpirationTimeMinutes greater than 0.
> This is not correct. Non-Persistent topics don't create durable
> subscriptions but It creates NonPersistent Subscription without storing its
> state.
>
> > if we set the value of subscriptionExpirationTimeMinutes greater than 0,
> it may lead to data loss(The durable subscriptions of PersistentTopic also
> can be removed).
> Non-Persistent topics don't provide data persistent or dispatch guarantee
> and it's by design so, let's not try to change semantics of non-persistent
> topic and there could be data loss in non-persistent topic.
>
> > And the Non-durable subscriptions will be removed automatically after all
> the consumers disconnected, it's the existing logic.
> Why can't we do this in the existing NonPersistent Subscription? I really
> don't understand the purpose of this PIP?
>
> Thanks,
> Rajan
>
> On Sun, Feb 12, 2023 at 10:56 PM Jiuming Tao  >
> wrote:
>
> > Hi all,
> >
> > I would like to start a VOTE on `PIP-245: Make subscriptions of
> > non-persistent topic non-durable`.
> >
> > Motivation:
> >
> > There are two types of subscriptions for a topic: Durable and
> Non-durable.
> >
> > We create a Consumer with a Durable subscription and a Reader with a
> > Non-durable subscription.
> >
> > But for NonPersistentTopic, creating a Durable subscription is
> > meaningless, NonPersistentSubscription doesn't have a ManagedCursor to
> > persistent its data. After its consumer disconnected, the subscription
> > couldn't be removed automatically if we didn't set the value of
> > subscriptionExpirationTimeMinutes greater than 0.
> >
> > For subscriptionExpirationTimeMinutes, it controls the subscription
> > expiration of NonPersistentTopic and PersistentTopic, if we set the value
> > of subscriptionExpirationTimeMinutes greater than 0, it may lead to data
> > loss(The durable subscriptions of PersistentTopic also can be removed).
> >
> > And the Non-durable subscriptions will be removed automatically after all
> > the consumers disconnected, it's the existing logic.
> >
> > For the purpose of removing the subscriptions which have no active
> > consumers of NonPersistentTopic and the above reasons, we can make all
> the
> > subscriptions of a NonPersistentTopic Non-durable.
> >
> >
> >
> > For more details, you can read:
> > https://github.com/apache/pulsar/issues/19448 <
>