Is it better to introduce a service URL detect interval to the service URL
provider?
I think the `updateServiceUrl` is not the initial purpose of exposing to
the Client API.
It looks like users just provide the interval of checking whether the
service URL is changed.
The Pulsar client will check it automatically. Using updateServiceUrl can
also achieve the purpose,
but users need to provide a fake service URL first or fetch the service URL
before
creating the client.
Another reason we need service URL provider API is that one team will
usually
provide an extra pulsar client lib with the service URL provider
implementation.
The lib
can be used across multiple teams. So from the user's perspective, they
only need to apply
a service URL provided to the client, they don't care about what is the
service URL and how to
update it.
Thanks,
Penghui
On Thu, Jan 19, 2023 at 3:43 PM Baodi Shi <ba...@apache.org> wrote:
> Hi, Yunze:
>
> Obviously, the `ServiceUrlProvider` config is redundant.
> >
>
> Agree. In fact, The client already provides the updateServiceUrl method,
> which the user can use to implement a dynamic update service URL. As for
> how the user implements it and how to close his resources, I think it can
> be left to the user.
>
>
> 在 2023年1月19日 15:16:52 上，Yunze Xu <y...@streamnative.io.invalid> 写道：
>
> > Hi all,
> >
> > Currently we have a `ServiceUrlProvider` interface to configure when
> > constructing a PulsarClient in `ClientBuilder#serviceUrlProvider`.
> > From the beginning, I thought the `getServiceUrl` method is called
> > each time the service URL is used, e.g. topic metadata lookup.
> > However, the `getServiceUrl` method is only called when constructing
> > the PulsarClient object. To update the PulsarClient's internal service
> > URL, `PulsarClient#updateServiceUrl` must be called. Therefore, if we
> > want to implement a `ServiceUrlProvider` that retrieves the latest
> > service URL from a database, I have to implement it like:
> >
> > ```java
> > class DataBaseServiceUrlProvider implements ServiceUrlProvider {
> >
> > private final ScheduledExecutorService executor =
> > Executors.newSingleThreadScheduledExecutor();
> >
> > @Override
> > public void initialize(PulsarClient client) {
> > executor.schedule(() -> {
> > try {
> > client.updateServiceUrl(readServiceUrlFromDB()/* a
> > fake method */);
> > } catch (PulsarClientException e) {
> > throw new RuntimeException(e);
> > }
> > }, 1, TimeUnit.SECONDS);
> > }
> >
> > @Override
> > public String getServiceUrl() {
> > return "pulsar://localhost:6650";
> > }
> >
> > @Override
> > public void close() {
> > executor.shutdown();
> > }
> > }
> > ```
> >
> > The key point is, if we didn't call `client.updateServiceUrl` and only
> > modified the returned value of `getServiceUrl` periodically, the
> > internal service URL would never be updated.
> >
> > Based on the provider above, the following two code snippets could be
> > nearly the same.
> >
> > ```java
> > var client = PulsarClient.builder().serviceUrlProvider(new
> > DataBaseServiceUrlProvider()).build();
> > /* ... */
> > client.close();
> > ```
> >
> > ```java
> > var client =
> > PulsarClient.builder().serviceUrl("pulsar://localhost:6650").build();
> > var provider = new DataBaseServiceUrlProvider();
> > provider.initialize(client);
> > /* ... */
> > provider.close();
> > client.close();
> > ```
> >
> > Obviously, the `ServiceUrlProvider` config is redundant.
> >
> > PIP-121 implements the `AutoClusterFailover` as the service URL
> > provider. However, it also calls the following methods periodically:
> > - PulsarClientImpl#updateAuthentication
> > - PulsarClientImpl#updateTlsTrustCertsFilePath
> >
> > It's unnatural and intuitive to say a service URL provider could
> > modify the internal states of `PulsarClient`, including:
> > - the service URL
> > - the authentication
> > - the TLS trust certificate file
> > - ...
> >
> > BTW, the implementation of PIP-121 [1] is different from the design [2].
> >
> > [1] https://github.com/apache/pulsar/pull/13316
> > [2] https://github.com/apache/pulsar/issues/13315
> >
> > Thanks,
> > Yunze
> >
>
