Re: [DISCUSS] PIP-186: Introduce two phase deletion protocol based on system topic

2023-02-05 Thread Yan Zhao
On 2023/02/03 18:14:53 Heesung Sohn wrote:
> There are some cases to trigger it.
> 1. A cursor be removed.
> 2. Close the current ledger and create a new ledger.
> 3. Consumer ack the message, the slowest cursor move forward.
> 4. User trigger truncateTopic by admin.
> 
> I see that this pip is for the internal ledger deletion cases(1-3 above),
> and it appears that such internal deletion operations do not require
> pre-validation or status checks(and no additional iops on the metadata
> store). I agree that we need a separate pip for async admin operations.

This pip is also applied to case 4. All cases will invoke 
`trimConsumedLedgersInBackground`.
This pip acts inside the method.


Re: [DISCUSS] PIP-186: Introduce two phase deletion protocol based on system topic

2023-02-05 Thread Yan Zhao
On 2023/02/03 20:05:59 Heesung Sohn wrote:
> For these internal requesters,
> 1. A cursor be removed.
> 2. Close the current ledger and create a new ledger.
> 3. Consumer ack the message, the slowest cursor move forward.
> 
> How do we prevent these callers from requesting duplicate requests for the
> same resources(ledgers) (how do we handle racing conditions between the
> requesters and consumers? )
These 3 case all invoke `trimConsumedLedgersInBackground`. 
```
executor.executeOrdered(name, safeRun(() -> internalTrimLedgers(isTruncate, 
promise)));
```
The managed ledger only load by one broker. And the executor is thread-safe. 

> It seems like there could be duplicate requests. Are we relying on these
> methods to dedup messages?
> 1. the idempotency of the deletion operations
> 2. enough delay in the requesters' scan cycles
> 3. enable dedup in the system topic
There is a case that will cause duplicate requests. The ManagedLedger wants to 
delete ledger 1, after sending it to the system topic, the broker shut down and 
didn't persist the left ledgers to the metadata store. In the next trigger, 
ledger 1 will be sent to the system topic again.
Here we use `the idempotency of the deletion operations` to handle it.



Re: [DISCUSS] Topic name restriction

2023-02-05 Thread Asaf Mesika
Thanks Mattison.

I don't understand the idea suggested of making the validation rules
configurable.
If understand correctly:
* "-partition" is not something you want configurable - it should always be
validated
* System topics - once we agree on a naming convention going forward, it
should always be validated.

I'm ok with adding configuration for the user so they can validate rules of
their own, maybe even per tenant.


On Fri, Feb 3, 2023 at 11:44 AM  wrote:

> Hi, Asaf
>
> We are using the regular expression to check the name.
> "^[-=:.\\w]*$"
> The \w means [A-Za-z0-9_] , which includes underscores.
>
> Best,
> Mattison
> On Feb 2, 2023, 23:01 +0800, Asaf Mesika , wrote:
> > NamedEntity is not allowing underscores - does it make sense?
> >
> >
> >
> > On Thu, Feb 2, 2023 at 8:35 AM Michael Marshall 
> > wrote:
> >
> > > Thanks for starting this thread, Mattison.
> > >
> > > > > The topic name character validation is already done by
> > > > > `NamedEntity#checkName`.
> > >
> > > Based on my reading of the code, only the tenant and the namespace
> > > names are validated using that method. There is a call [0] that looks
> > > like it validates topic names, but that method is only called by
> > > tests.
> > >
> > > > > But I have a concern that whether we should
> > > > > treat all topics that start with the long underscore ("__") as
> system
> > > > > topics?
> > >
> > > This is a reasonable concern, and my primary motivation in proposing
> > > this change is to make it easier for the broker to handle system
> > > topics, which often get unique treatment.
> > >
> > > I wrote on this topic in several replies on this thread from a year ago
> > > [1].
> > >
> > > In the context of PIP 242, we're introducing a config to optionally
> > > enforce strict topic names. As such, we could rely on the config to
> > > either use the "cheap" check to see if the topic starts with __ or we
> > > could use the more expensive check to determine if the topic name is
> > > one of many possible system topic names. Because we want to maintain
> > > backwards compatibility, we cannot completely get rid of the old
> > > logic. I like self describing names because they are elegant and
> > > efficient.
> > >
> > > > > If yes, how would you like to allow users to access the system
> topics?
> > >
> > > I proposed some ideas at the end of that thread [1]. We should have a
> > > clear definition of system topics and how they are or are not accessed
> by
> > > users. Ultimately, we continue to create new system topics without
> > > reserving a designated naming structure and without defining how these
> > > topics ought to be interacted with, as Yunze points out. Note that any
> > > system topic we introduce could conflict with existing user topics, so
> > > proactively reserving a set of names makes it easier for forwards
> > > compatibility.
> > >
> > > Thanks,
> > > Michael
> > >
> > > [0]
> > >
> https://github.com/apache/pulsar/blob/b880b1d240ade864181935aa360bfca03a5aa67f/pulsar-common/src/main/java/org/apache/pulsar/common/naming/NamespaceName.java#L159
> > > [1] https://lists.apache.org/thread/pj4n4wzm3do8nkc52l7g7obh0sktzm17
> > >
> > >
> > > On Wed, Feb 1, 2023 at 11:28 PM r...@apache.org <
> ranxiaolong...@gmail.com>
> > > wrote:
> > > > >
> > > > > Hi Mattison:
> > > > >
> > > > > Agree with Yong's idea. We can expose `disallowed topic` as a
> > > configuration
> > > > > to the user side, and a more flexible way is to expose it as a
> > > > > namespace-level policy. This can ensure that there is no need to do
> > > special
> > > > > processing on customized keywords in the future, and the expected
> effect
> > > > > can be achieved by modifying the configuration.
> > > > >
> > > > > Think Yunze's concerns are justified for the system topic. Is it
> okay if
> > > we
> > > > > use hard code? Because the identification of any keyword is likely
> to be
> > > > > hit by the user. The hard code method is used to filter out system
> topics
> > > > > and not allow users to operate during delete and create operations.
> > > > >
> > > > > --
> > > > > Thanks
> > > > > Xiaolong Ran
> > > > >
> > > > > Dave Fisher  于2023年2月2日周四 11:26写道:
> > > > >
> > > > > > >
> > > > > > >
> > > > > > > Sent from my iPhone
> > > > > > >
> > > > > > > > > On Feb 1, 2023, at 6:52 PM, Yong Zhang <
> zhangyong1025...@gmail.com>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Mattison,
> > > > > > > > >
> > > > > > > > > I agree with you about restricting the topic name.
> > > > > > > > >
> > > > > > > > > How about using a blacklist way to restrict it?
> > > > > > >
> > > > > > > If we do then please call it by another name like “disallowed”.
> > > > > > >
> > > > > > > > >
> > > > > > > > > We can have a blacklist on the topic name restriction and
> make it
> > > > > > > > > configurable. Add the keywords you mentioned in the default
> > > > > > > configuration.
> > > > > > > > > That would have a more general way to block a topic name
> crea

Re: [DISCUSS] Redundant ServiceUrlProvider design and improper use of PIP-121

2023-02-05 Thread Baodi Shi
>
> Not completely different IMO. When we want to update the service URL,
>
> we usually want to connect to another cluster. However, the cluster
>
> could have configured authentication, TLS for security, but we cannot
>
> guarantee two clusters share the same TLS certificates or
>
> authentication configs. I think we should group these necessary
>
> information into a common class like `ConnectInfo`. Then, just expose
>
> a method to update the `ConnectInfo` so that code could be more
>
> scalable.
>
>
It makes sense to me.

Alternatively, can we directly support rebuilding PulsarClient?

PulsarClient client = PulsarClient.builder()
.serviceUrl("old url")
.tlsTrustCertsFilePath()
.statsInterval(1, TimeUnit.SECONDS)
.build();

client.newBuilder()
.serviceUrl("new url")
// tls relate
.tlsTrustCertsFilePath()
.statsInterval(2, TimeUnit.SECONDS)
.build();


When calling rebuild, the new configuration overwrites the old
configuration, and if there are no settings, the old configuration is used.

在 2023年2月2日 23:02:21 上,Asaf Mesika  写道:

> Ok, makes sense.
>
>
> On Thu, Feb 2, 2023 at 4:02 PM Yunze Xu 
> wrote:
>
> Not completely different IMO. When we want to update the service URL,
>
> we usually want to connect to another cluster. However, the cluster
>
> could have configured authentication, TLS for security, but we cannot
>
> guarantee two clusters share the same TLS certificates or
>
> authentication configs. I think we should group these necessary
>
> information into a common class like `ConnectInfo`. Then, just expose
>
> a method to update the `ConnectInfo` so that code could be more
>
> scalable.
>
>
> Thanks,
>
> Yunze
>
>
> On Thu, Feb 2, 2023 at 9:39 PM Asaf Mesika  wrote:
>
> >
>
> > Ok, the last two methods IMO are a completely different topic.
>
> >
>
> > I wonder what the maintainers think about this.
>
> >
>
> > On Mon, Jan 30, 2023 at 4:16 AM Yunze Xu 
>
> > wrote:
>
> >
>
> > > In summary, the core issue is that ServiceUrlProvider could be
>
> > > misused. A user might want to implement its own ServiceUrlProvider by
>
> > > changing the returned value of `get` method but it actually does not
>
> > > work. The built-in implementations like AutoFailover actually work by
>
> > > these `updateXXX` methods. So it's meaningless to provide them as
>
> > > ServiceUrlProvider implementations.
>
> > >
>
> > > To answer your question directly: totally yes. But not only
>
> > > `setServiceUrl` (the name is `updateServiceUrl` in code),  the
>
> > > `updateAuthentication` and `updateTlsTrustCertsFilePath` methods,
>
> > > which are not exposed to `PulsarClient`, should also be exposed.
>
> > >
>
> > > Thanks,
>
> > > Yunze
>
> > >
>
> > > On Sun, Jan 29, 2023 at 11:57 PM Asaf Mesika 
>
> > > wrote:
>
> > > >
>
> > > > Yunze - in summary - your proposal is to get rid of the
>
> > > ServiceUrlProvider
>
> > > > right? You just want to have setServiceUrl on the client instead?
>
> > > >
>
> > > >
>
> > > > On Wed, Jan 25, 2023 at 2:03 PM Yunze Xu
>
> 
>
> > > > wrote:
>
> > > >
>
> > > > > It makes sense to me. So the better way is still providing the
>
> > > > > `updateXXX` methods to `PulsarClient`. As for how to detect the
>
> > > > > connection info (e.g. service URL) changes, it's determined by the
>
> > > > > user's implementation. For example, the current AutoClusterFailover
>
> > > > > polls with a fixed interval.
>
> > > > >
>
> > > > > We can also implement a notification mechanism based on the
>
> > > > > `updateXXX` methods. For example, assuming we have a producer that
>
> > > > > writes the latest service URL to a topic and a consumer that reads
>
> the
>
> > > > > latest service URL from that topic. Then, the consumer could call
>
> > > > > `pulsarClient.updateXXX` once it receives the latest service URL.
>
> > > > >
>
> > > > > Thanks,
>
> > > > > Yunze
>
> > > > >
>
> > > > > On Tue, Jan 24, 2023 at 4:32 PM Asaf Mesika 
> >
>
> > > wrote:
>
> > > > > >
>
> > > > > > If I understand correctly, the idea of having the ability to
>
> change
>
> > > > > > serviceUrl is to support switching over between clusters
>
> dynamically?
>
> > > > > > If that is the case, doesn't it make sense that it will trigger
>
> the
>
> > > > > change
>
> > > > > > to the client instead of the client polling it and check it self
>
> if
>
> > > > > > something has changed?
>
> > > > > >
>
> > > > > >
>
> > > > > > On Mon, Jan 23, 2023 at 5:00 AM Yunze Xu
>
> > > 
>
> > > > > > wrote:
>
> > > > > >
>
> > > > > > > Yes. The poll happens in the client's internal thread.
>
> > > > > > >
>
> > > > > > > Thanks,
>
> > > > > > > Yunze
>
> > > > > > >
>
> > > > > > > On Sun, Jan 22, 2023 at 6:56 PM Asaf Mesika <
>
> asaf.mes...@gmail.com
>
> > > >
>
> > > > > wrote:
>
> > > > > > > >
>
> > > > > > > > and the client will poll the ConnectInfoProvider and check if
>
> >

Re: [DISCUSS] PIP-186: Introduce two phase deletion protocol based on system topic

2023-02-05 Thread Asaf Mesika
On Fri, Feb 3, 2023 at 1:48 PM Yan Zhao  wrote:

> > If you persisted the message successfully by the producer and the broker
> > was terminated before being able to delete the ledger from the metadata
> > service?
> If the broker is terminated, the consumer won't ack the message, the
> message will be re-consume later.


Let me quote the text I was replying to:

> When pulsar wants to delete a ledger, ManagedLedger uses
> ledgerDeletionService to send a message, the message content contains the
> waiting delete ledger info. After sending success, delete the ledger id
> from the metadata store.


If I understand correctly, the following steps will happen:
1. Producer writes a message to system topic, containing the ledger
information to delete.
2. Upon success of (1), the ledger ID is deleted from the metadata store.

You reply was about the consumer, my question was about the producer, so
I'm writing my reply here again (full reply):

If you persisted the message successfully by the producer and the broker
was terminated before being able to delete the ledger (ID) from the
metadata service?


>
> > I recommend having the logic to delete the ledger be done in the message
> > consumer side:
> > - if the ledger exists in the MD store, delete it.
> > - send delete command to BK
> > Both as I understand are idempotent. If for some reason one action was
> done
> > but the other wasn't - ZK down, BK down, broker hosting the consumer
> > terminated - either the message will not be acknowledged, or you
> negatively
> > acknowledge.
> We send a delete command to the broker, it will connect to the
> corresponding broker which loads the topic. The corresponding broker
> received the command, then passes the command to ManagedLedger, the
> ManagedLedger does the actual delete operation.
> If the consumer does the delete operation, it's a little unreasonable. The
> ledger manager should be `ManagedLedger`, let it do the delete will be
> better.
>

You say the consumer will send a delete command to the broker: By what
means? Sending a message on a topic, or running an RPC request by adding a
new command to the PulsarApi.proto file?
If the latter - shouldn't this be documented?

Let me assume the consumer which receives the message, will run an Pulsar
API command to instruct the broker assigned to this topic to delete the
ledger - Why the implementation of this command in the assigned broker
can't have both the deletion of the ledger ID from the ZK, AND deletion
from BK? Why do we need to have the ZK deletion done on the producer side,
after we produced the message?


> > General question: When a ledger is persisted to ZK, where is the ledger
> > metadata persisted in ZK (more specifically it's metadata, which includes
> > the component?
> > Is it also used when building out the key (path) in ZK?
>
>
> https://github.com/apache/bookkeeper/blob/901f76ce4c4f9f771363424dbb60da4d590ad122/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataImpl.java#L74
>
> It's the content in the zk node. when creating a ledger by bookkeeper, it
> will create a path like `/ledgers/00//L`, the path value is an
> instance of LedgerMetadataImpl.
> The bookkeeper LedgerMetadataImpl, the customMetadata stores the user's
> metadata.
>
> If the ledger is for Ledger, the customMetadata store:
> `key:application, value:pulsar`
> `key:component, value:managed-ledger`
> `key:pulsar/managed-ledger, value: ledgerName`
>
> If the ledger is for Cursor, the customMetadata store:
> `key:application, value:pulsar`
> `key:component, value:managed-ledger`
> `key:pulsar/managed-ledger, value: ledgerName`
> `key:pulsar/cursor, value: curSorName`
>
> If the ledger is for schema, the customMetadata store:
> `key:application, value:pulsar`
> `key:component, value:schema`
> `key:pulsar/schemaId, value: schemaId`
>
> So when we get the ledger metadata from bookkeeper, we can get the ledger
> source.
>

Thanks for the info!


>
> > Isn't the type saved together with the ledger in ZK?
> We need to differ it, the same ledger may store both on the bk side and
> the offload side.
> If a ledger want to delete the bk data and the offload data, it should
> publishes two message to the system topic. The broker needs it to determine
> whether to delete offload or bk.

So you're saying that the information whether this ledger is either only in
BK, or only in offload storage or both is NOT encoded in the Ledger custom
metadata properties of the ledger?
The offloader saves that information for that ledger in another place in
ZK?

If this ledger is a Cursor ledger, how do you know where to delete it
inside ZK if you don't include the cursor name in the
PendingDeleteLedgerInfo message?


>
> > It's for the offloaded ledger, when we want to delete the offload ledger,
> > > we need offloadedContextUuid, here we can simplify it to
> offloadContextUuid.
>
> > Sounds much better. Maybe offloadedLedgerUUID? (why context?)
> Agree.
>
> >
> >  Are you encoding

Re: [DISCUSS] Redundant ServiceUrlProvider design and improper use of PIP-121

2023-02-05 Thread Asaf Mesika
Can you please write rebuild example?

On Sun, Feb 5, 2023 at 4:05 PM Baodi Shi  wrote:

> >
> > Not completely different IMO. When we want to update the service URL,
> >
> > we usually want to connect to another cluster. However, the cluster
> >
> > could have configured authentication, TLS for security, but we cannot
> >
> > guarantee two clusters share the same TLS certificates or
> >
> > authentication configs. I think we should group these necessary
> >
> > information into a common class like `ConnectInfo`. Then, just expose
> >
> > a method to update the `ConnectInfo` so that code could be more
> >
> > scalable.
> >
> >
> It makes sense to me.
>
> Alternatively, can we directly support rebuilding PulsarClient?
>
> PulsarClient client = PulsarClient.builder()
> .serviceUrl("old url")
> .tlsTrustCertsFilePath()
> .statsInterval(1, TimeUnit.SECONDS)
> .build();
>
> client.newBuilder()
> .serviceUrl("new url")
> // tls relate
> .tlsTrustCertsFilePath()
> .statsInterval(2, TimeUnit.SECONDS)
> .build();
>
>
> When calling rebuild, the new configuration overwrites the old
> configuration, and if there are no settings, the old configuration is used.
>
> 在 2023年2月2日 23:02:21 上,Asaf Mesika  写道:
>
> > Ok, makes sense.
> >
> >
> > On Thu, Feb 2, 2023 at 4:02 PM Yunze Xu 
> > wrote:
> >
> > Not completely different IMO. When we want to update the service URL,
> >
> > we usually want to connect to another cluster. However, the cluster
> >
> > could have configured authentication, TLS for security, but we cannot
> >
> > guarantee two clusters share the same TLS certificates or
> >
> > authentication configs. I think we should group these necessary
> >
> > information into a common class like `ConnectInfo`. Then, just expose
> >
> > a method to update the `ConnectInfo` so that code could be more
> >
> > scalable.
> >
> >
> > Thanks,
> >
> > Yunze
> >
> >
> > On Thu, Feb 2, 2023 at 9:39 PM Asaf Mesika 
> wrote:
> >
> > >
> >
> > > Ok, the last two methods IMO are a completely different topic.
> >
> > >
> >
> > > I wonder what the maintainers think about this.
> >
> > >
> >
> > > On Mon, Jan 30, 2023 at 4:16 AM Yunze Xu  >
> >
> > > wrote:
> >
> > >
> >
> > > > In summary, the core issue is that ServiceUrlProvider could be
> >
> > > > misused. A user might want to implement its own ServiceUrlProvider by
> >
> > > > changing the returned value of `get` method but it actually does not
> >
> > > > work. The built-in implementations like AutoFailover actually work by
> >
> > > > these `updateXXX` methods. So it's meaningless to provide them as
> >
> > > > ServiceUrlProvider implementations.
> >
> > > >
> >
> > > > To answer your question directly: totally yes. But not only
> >
> > > > `setServiceUrl` (the name is `updateServiceUrl` in code),  the
> >
> > > > `updateAuthentication` and `updateTlsTrustCertsFilePath` methods,
> >
> > > > which are not exposed to `PulsarClient`, should also be exposed.
> >
> > > >
> >
> > > > Thanks,
> >
> > > > Yunze
> >
> > > >
> >
> > > > On Sun, Jan 29, 2023 at 11:57 PM Asaf Mesika 
> >
> > > > wrote:
> >
> > > > >
> >
> > > > > Yunze - in summary - your proposal is to get rid of the
> >
> > > > ServiceUrlProvider
> >
> > > > > right? You just want to have setServiceUrl on the client instead?
> >
> > > > >
> >
> > > > >
> >
> > > > > On Wed, Jan 25, 2023 at 2:03 PM Yunze Xu
> >
> > 
> >
> > > > > wrote:
> >
> > > > >
> >
> > > > > > It makes sense to me. So the better way is still providing the
> >
> > > > > > `updateXXX` methods to `PulsarClient`. As for how to detect the
> >
> > > > > > connection info (e.g. service URL) changes, it's determined by
> the
> >
> > > > > > user's implementation. For example, the current
> AutoClusterFailover
> >
> > > > > > polls with a fixed interval.
> >
> > > > > >
> >
> > > > > > We can also implement a notification mechanism based on the
> >
> > > > > > `updateXXX` methods. For example, assuming we have a producer
> that
> >
> > > > > > writes the latest service URL to a topic and a consumer that
> reads
> >
> > the
> >
> > > > > > latest service URL from that topic. Then, the consumer could call
> >
> > > > > > `pulsarClient.updateXXX` once it receives the latest service URL.
> >
> > > > > >
> >
> > > > > > Thanks,
> >
> > > > > > Yunze
> >
> > > > > >
> >
> > > > > > On Tue, Jan 24, 2023 at 4:32 PM Asaf Mesika <
> asaf.mes...@gmail.com
> >
> > >
> >
> > > > wrote:
> >
> > > > > > >
> >
> > > > > > > If I understand correctly, the idea of having the ability to
> >
> > change
> >
> > > > > > > serviceUrl is to support switching over between clusters
> >
> > dynamically?
> >
> > > > > > > If that is the case, doesn't it make sense that it will trigger
> >
> > the
> >
> > > > > > change
> >
> > > > > > > to the client instead of the client polling it and check it
> self
> >
> > if
> >
> > > > > 

Re: [DISCUSS] Redundant ServiceUrlProvider design and improper use of PIP-121

2023-02-05 Thread Baodi Shi
 I thought about it for a moment, and this is not a good idea.

Using newBuild may be understood by the user as recreating the client
object. It is still intuitive to add the `updateXXX` method.


Thanks,
Baodi Shi


在 2023年2月5日 23:01:57 上,Asaf Mesika  写道:

> Can you please write rebuild example?
>
> On Sun, Feb 5, 2023 at 4:05 PM Baodi Shi  wrote:
>
> >
>
> > Not completely different IMO. When we want to update the service URL,
>
> >
>
> > we usually want to connect to another cluster. However, the cluster
>
> >
>
> > could have configured authentication, TLS for security, but we cannot
>
> >
>
> > guarantee two clusters share the same TLS certificates or
>
> >
>
> > authentication configs. I think we should group these necessary
>
> >
>
> > information into a common class like `ConnectInfo`. Then, just expose
>
> >
>
> > a method to update the `ConnectInfo` so that code could be more
>
> >
>
> > scalable.
>
> >
>
> >
>
> It makes sense to me.
>
>
> Alternatively, can we directly support rebuilding PulsarClient?
>
>
> PulsarClient client = PulsarClient.builder()
>
> .serviceUrl("old url")
>
> .tlsTrustCertsFilePath()
>
> .statsInterval(1, TimeUnit.SECONDS)
>
> .build();
>
>
> client.newBuilder()
>
> .serviceUrl("new url")
>
> // tls relate
>
> .tlsTrustCertsFilePath()
>
> .statsInterval(2, TimeUnit.SECONDS)
>
> .build();
>
>
>
> When calling rebuild, the new configuration overwrites the old
>
> configuration, and if there are no settings, the old configuration is used.
>
>
> 在 2023年2月2日 23:02:21 上,Asaf Mesika  写道:
>
>
> > Ok, makes sense.
>
> >
>
> >
>
> > On Thu, Feb 2, 2023 at 4:02 PM Yunze Xu 
>
> > wrote:
>
> >
>
> > Not completely different IMO. When we want to update the service URL,
>
> >
>
> > we usually want to connect to another cluster. However, the cluster
>
> >
>
> > could have configured authentication, TLS for security, but we cannot
>
> >
>
> > guarantee two clusters share the same TLS certificates or
>
> >
>
> > authentication configs. I think we should group these necessary
>
> >
>
> > information into a common class like `ConnectInfo`. Then, just expose
>
> >
>
> > a method to update the `ConnectInfo` so that code could be more
>
> >
>
> > scalable.
>
> >
>
> >
>
> > Thanks,
>
> >
>
> > Yunze
>
> >
>
> >
>
> > On Thu, Feb 2, 2023 at 9:39 PM Asaf Mesika 
>
> wrote:
>
> >
>
> > >
>
> >
>
> > > Ok, the last two methods IMO are a completely different topic.
>
> >
>
> > >
>
> >
>
> > > I wonder what the maintainers think about this.
>
> >
>
> > >
>
> >
>
> > > On Mon, Jan 30, 2023 at 4:16 AM Yunze Xu 
> >
>
> >
>
> > > wrote:
>
> >
>
> > >
>
> >
>
> > > > In summary, the core issue is that ServiceUrlProvider could be
>
> >
>
> > > > misused. A user might want to implement its own ServiceUrlProvider by
>
> >
>
> > > > changing the returned value of `get` method but it actually does not
>
> >
>
> > > > work. The built-in implementations like AutoFailover actually work by
>
> >
>
> > > > these `updateXXX` methods. So it's meaningless to provide them as
>
> >
>
> > > > ServiceUrlProvider implementations.
>
> >
>
> > > >
>
> >
>
> > > > To answer your question directly: totally yes. But not only
>
> >
>
> > > > `setServiceUrl` (the name is `updateServiceUrl` in code),  the
>
> >
>
> > > > `updateAuthentication` and `updateTlsTrustCertsFilePath` methods,
>
> >
>
> > > > which are not exposed to `PulsarClient`, should also be exposed.
>
> >
>
> > > >
>
> >
>
> > > > Thanks,
>
> >
>
> > > > Yunze
>
> >
>
> > > >
>
> >
>
> > > > On Sun, Jan 29, 2023 at 11:57 PM Asaf Mesika 
>
> >
>
> > > > wrote:
>
> >
>
> > > > >
>
> >
>
> > > > > Yunze - in summary - your proposal is to get rid of the
>
> >
>
> > > > ServiceUrlProvider
>
> >
>
> > > > > right? You just want to have setServiceUrl on the client instead?
>
> >
>
> > > > >
>
> >
>
> > > > >
>
> >
>
> > > > > On Wed, Jan 25, 2023 at 2:03 PM Yunze Xu
>
> >
>
> > 
>
> >
>
> > > > > wrote:
>
> >
>
> > > > >
>
> >
>
> > > > > > It makes sense to me. So the better way is still providing the
>
> >
>
> > > > > > `updateXXX` methods to `PulsarClient`. As for how to detect the
>
> >
>
> > > > > > connection info (e.g. service URL) changes, it's determined by
>
> the
>
> >
>
> > > > > > user's implementation. For example, the current
>
> AutoClusterFailover
>
> >
>
> > > > > > polls with a fixed interval.
>
> >
>
> > > > > >
>
> >
>
> > > > > > We can also implement a notification mechanism based on the
>
> >
>
> > > > > > `updateXXX` methods. For example, assuming we have a producer
>
> that
>
> >
>
> > > > > > writes the latest service URL to a topic and a consumer that
>
> reads
>
> >
>
> > the
>
> >
>
> > > > > > latest service URL from that topic. Then, the consumer could call
>
> >
>
> > > > > > `pulsarClient.updateXXX` once it receives the latest service URL.
>
> >
>
> > > > > >
>
> >
>
> > > > > > 

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

2023-02-05 Thread Zike Yang
Hi, Yunze

When I use the python 3.7.9 to install the whl file. It reports a
platform mismatch error.

```
➜  macos python --version
Python 3.7.9
➜  macos python -m pip install
pulsar_client-3.1.0-cp37-cp37m-macosx_10_15_universal2.whl
ERROR: pulsar_client-3.1.0-cp37-cp37m-macosx_10_15_universal2.whl is
not a supported wheel on this platform.
```

Python 3.10 and 3.9 works fine for me. My macOS version is 12.6
Could you take a look at this?

Thanks,
Zike Yang

On Sat, Feb 4, 2023 at 11:52 PM guo jiwei  wrote:
>
> +1 (binding)
>
> - Checked the signature
> - Install the python .whl file (pip3 install
> pulsar_client-3.1.0-cp310-cp310-macosx_10_15_universal2.whl) on macOS
> - Start the standalone (master)
> - Start consumer (python3 ./examples/consumer.py)
> - Start producer (python3 ./examples/producer.py )
>
> Regards
> Jiwei Guo (Tboy)
>
> On Sat, Feb 4, 2023 at 6:11 PM r...@apache.org  
> wrote:
> >
> > +1 (non-binding)
> >
> > python version:3
> >
> > 1. run Go SDK producer and use python SDK consumer to receive messages;
> > 2. run Python SDK consumer and use Go SDK producer to send messages;
> >
> > --
> > Thanks
> > Xiaolong Ran
> >
> > 丛搏  于2023年2月3日周五 13:39写道:
> >
> > > +1(binding)
> > > os: mac 12.6
> > > python: 3.9.6
> > >
> > > python3 ./examples/consumer.py
> > > python3 ./examples/producer.py
> > >
> > > Thanks,
> > > Bo
> > >
> > > Matteo Merli  于2023年2月3日周五 03:26写道:
> > > >
> > > > +1
> > > > --
> > > > Matteo Merli
> > > > 
> > > >
> > > > On Thu, Feb 2, 2023 at 5:57 AM Yunze Xu 
> > > wrote:
> > > > >
> > > > > This is the 2nd 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-2/
> > > > >
> > > > > 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/)
> > > > >
> > > > > The tag to be voted upon: v3.1.0-candidate-2
> > > > > (fda50867a9c7bf927309527fade2f53eb3907bed)
> > > > >
> > > https://github.com/apache/pulsar-client-python/releases/tag/v3.1.0-candidate-2
> > > > >
> > > > > Pulsar's KEYS file containing PGP keys you use to sign the release:
> > > > > https://dist.apache.org/repos/dist/dev/pulsar/KEYS
> > > > >
> > > > > Please download the Python wheels and follow the README to test.
> > >


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

2023-02-05 Thread Yunze Xu
This wheel works for me. I tested the following commands on macOS Monterey 12.6:

```bash
brew install python@3.7
python3.7 -m pip install 
pulsar_client-3.1.0-cp37-cp37m-macosx_10_15_universal2.whl 
```

Here are the actual versions:

```bash
% python3.7 --version   
Python 3.7.16
% python3.7 -m pip --version
 
pip 22.3.1 from /usr/local/lib/python3.7/site-packages/pip (python 3.7)
```

Could you try replacing `python -m pip` with `python3.7 -m pip`?

Thanks,
Yunze




> On Feb 6, 2023, at 10:13, Zike Yang  wrote:
> 
> Hi, Yunze
> 
> When I use the python 3.7.9 to install the whl file. It reports a
> platform mismatch error.
> 
> ```
> ➜  macos python --version
> Python 3.7.9
> ➜  macos python -m pip install
> pulsar_client-3.1.0-cp37-cp37m-macosx_10_15_universal2.whl
> ERROR: pulsar_client-3.1.0-cp37-cp37m-macosx_10_15_universal2.whl is
> not a supported wheel on this platform.
> ```
> 
> Python 3.10 and 3.9 works fine for me. My macOS version is 12.6
> Could you take a look at this?
> 
> Thanks,
> Zike Yang
> 
> On Sat, Feb 4, 2023 at 11:52 PM guo jiwei  wrote:
>> 
>> +1 (binding)
>> 
>> - Checked the signature
>> - Install the python .whl file (pip3 install
>> pulsar_client-3.1.0-cp310-cp310-macosx_10_15_universal2.whl) on macOS
>> - Start the standalone (master)
>> - Start consumer (python3 ./examples/consumer.py)
>> - Start producer (python3 ./examples/producer.py )
>> 
>> Regards
>> Jiwei Guo (Tboy)
>> 
>> On Sat, Feb 4, 2023 at 6:11 PM r...@apache.org  
>> wrote:
>>> 
>>> +1 (non-binding)
>>> 
>>> python version:3
>>> 
>>> 1. run Go SDK producer and use python SDK consumer to receive messages;
>>> 2. run Python SDK consumer and use Go SDK producer to send messages;
>>> 
>>> --
>>> Thanks
>>> Xiaolong Ran
>>> 
>>> 丛搏  于2023年2月3日周五 13:39写道:
>>> 
 +1(binding)
 os: mac 12.6
 python: 3.9.6
 
 python3 ./examples/consumer.py
 python3 ./examples/producer.py
 
 Thanks,
 Bo
 
 Matteo Merli  于2023年2月3日周五 03:26写道:
> 
> +1
> --
> Matteo Merli
> 
> 
> On Thu, Feb 2, 2023 at 5:57 AM Yunze Xu 
 wrote:
>> 
>> This is the 2nd 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-2/
>> 
>> 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/)
>> 
>> The tag to be voted upon: v3.1.0-candidate-2
>> (fda50867a9c7bf927309527fade2f53eb3907bed)
>> 
 https://github.com/apache/pulsar-client-python/releases/tag/v3.1.0-candidate-2
>> 
>> Pulsar's KEYS file containing PGP keys you use to sign the release:
>> https://dist.apache.org/repos/dist/dev/pulsar/KEYS
>> 
>> Please download the Python wheels and follow the README to test.