You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Heesung Sohn <he...@apache.org> on 2024/01/26 02:56:56 UTC
[DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Hi dev,
I would like to discuss this change with the community.
Motivation:
By this BlueGreenClusterMigration PIP-188(
https://github.com/apache/pulsar/pull/17962), `redirectedClusterURI` member
var has been introduced in the pulsar client to hold the new cluster's
endpoint. Accordingly, pulsar client also added a state of a map of
LookupService to lookup the migrated clusters. If redirectedClusterURI is
not null, then clients look up the mapped,
redirectedClusterURI->LookupService, when clients connect to migrated
brokers.
Proposal:
In fact, this logic can be simplified by directly updating the
existing lookup service's service url. Then, we don't need to track the new
lookup services in a separate map and remove `redirectedClusterURI`,
either.
Hers is the sample code for this proposal:
https://github.com/heesung-sn/pulsar/pull/59/
Pros:
- The removal of redirectedClusterURI and the lookup service map can
simplify the client code
- It works better with other logics: Bundle Transfer and
AutoClusterFailover, as these features currently only use the original
lookup service.
Cons:
- To consume backlog from the blue (old) cluster, new clients need to be
created with the old service url(pointing to the old cluster) because the
existing clients' lookup service will be already updated, pointing to the
new cluster.
However, I think this(requiring a new client with old service url, old
cluster) makes sense because all of the existing pub/sub will anyway be
reconnected to the new clusters as per the broker's close commands. So, to
consume the backlog from the old cluster, new client applications need to
start with the old service url anyway.
Example:
https://github.com/heesung-sn/pulsar/pull/59/files#diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312
Thanks,
Heesung
Re: [E] Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Heesung Sohn <he...@apache.org>.
Here is the sample code to clean this lookup/urlLookupMap access(by
following Option A-1).
https://github.com/heesung-sn/pulsar/pull/60
Plz let me know if this looks good. Then, I will raise a PR, and
accordingly, I will apply a similar change to other clients(cpp and golang)
when adding the blue-green migration logic.
Regards,
Heesung
On Sun, Jan 28, 2024 at 3:23 PM Heesung Sohn <he...@apache.org> wrote:
> I see. Thank you for clarifying this requirement.
>
> Then, I am wondering if we need to refactor the current
> `lookup/urlLookupMap` access code to avoid using `lookup` when the
> `redirectedClusterURI` is set at the consumers/producer layer. (it appears
> that we still use `lookup` in the following code at the PulsarClientImpl
> even when the consumers/producer might have migrated already(set
> redirectedClusterURI already). Maybe I missed some other logic here, and
> please also correct me if this behavior is fine.
>
> Some of the `lookup` access examples:
>
>
> https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L574
>
>
> https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L951
>
>
> https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L989
>
>
> https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L999
>
>
> If we need to make the clients use `urlLookupMap` after
> producers/customers are migrated, then I guess we may need to consider the
> following options.
>
> Option A-1:
> Pass `optional redirectedClusterURI` var to the above PulsarClientImpl
> funcs to select the lookup or urlLookupMap inside.
>
> Option A-2:
> - Remove urlLookupMap from PulsarClientImpl
> - Instead, add serviceNameResolverMap in LookupService
> - Pass `optional redirectedClusterURI` var to the above PulsarClientImpl
> funcs
> - Pass `optional redirectedClusterURI` var to the above LookupService funcs
>
>
> Regards,
> Heesung
>
>
>
> On Sat, Jan 27, 2024 at 10:33 AM Rajan Dhabalia
> <rd...@yahooinc.com.invalid> wrote:
>
>> Hi,
>>
>> Updating ServiceUrl in PulsarClient will make all producers and
>> consumers of all topics to use that new url which is incorrect because
>> redirection state can be different for each topic and even producer
>> and consumer of the same topic. So, we can not update main serviceUrl
>> but client should maintain redirection url for each producer and
>> consumer based on its state decided by the server.
>>
>>
>> Sent from my iPhone
>>
>> > On Jan 26, 2024, at 9:12 AM, Heesung Sohn <he...@apache.org> wrote:
>> >
>> > To add more context, I am trying to add this blue-green migration
>> logic in
>> > the cpp and golang client, and before copying the current logic, I am
>> > wondering if we can simplify it.
>> >
>> > Again, since all of the producers and consumers from the same client and
>> > topic need to migrate eventually,
>> > I think directly updating the lookup service of that shared client to
>> the
>> > new cluster seems harmless.
>> > I might be missing some other requirements here, so please correct me.
>> > If we conclude keeping this redirectedClusterURI in HandlerState here, I
>> > will go ahead and copy the current java client logic to other clients.
>> >
>> > Also, I updated the sample code for better synchronization and dedup.
>> >
>> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rArkuM1fbw$
>> >
>> > Thanks,
>> > Heesung
>> >
>> >
>> >
>> >> On Thu, Jan 25, 2024 at 10:51 PM Heesung Sohn <he...@apache.org>
>> wrote:
>> >>
>> >> I am sorry, but the current implementation of blue-green migration is
>> for
>> >> all topics (producers and customers), not topic-specific. Could you
>> correct
>> >> me if I am wrong here?
>> >> So, for the current requirement(all topics migration), can't we just
>> >> update the original lookup service for that client because all of the
>> >> topics (of consumers and producers) will eventually migrate to the new
>> >> cluster?
>> >> Having said this, having redirectedClusterURI in HandlerState appears
>> to
>> >> be futuristic.
>> >>
>> >> Heesung
>> >>
>> >>
>> >>
>> >> On Thu, Jan 25, 2024 at 10:36 PM Heesung Sohn <he...@apache.org>
>> wrote:
>> >>
>> >>>> This might not work because same PulsarClient can be used to create
>> >>> multiple topics and producer/consumer and migration state can be
>> different
>> >>> per topic and even for produce/consumer (of same topic) so, changing
>> >>> service url will not work for blue-green migration redirection for all
>> >>> topics served by that PulsarClient.
>> >>>
>> >>> Good point on this topic (and producer and consumer) level migration
>> >>> requirement. I was under the impression that the migration happens
>> for all
>> >>> topics from the old to the new cluster.
>> >>>
>> >>> For this requirement, I think we should keep redirectedClusterURI in
>> >>> HandlerState.
>> >>>
>> >>> Thanks for this quick response.
>> >>> Heesung
>> >>>
>> >>>
>> >>> On Thu, Jan 25, 2024 at 10:05 PM Rajan Dhabalia <
>> dhabalia.me@gmail.com>
>> >>> wrote:
>> >>>
>> >>>> This might not work because same PulsarClient can be used to create
>> >>>> multiple topics and producer/consumer and migration state can be
>> different
>> >>>> per topic and even for produce/consumer (of same topic) so, changing
>> >>>> service url will not work for blue-green migration redirection for
>> all
>> >>>> topics served by that PulsarClient.
>> >>>>
>> >>>> Sent from my iPhone
>> >>>>
>> >>>>> On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org>
>> wrote:
>> >>>>>
>> >>>>> Hi dev,
>> >>>>>
>> >>>>> I would like to discuss this change with the community.
>> >>>>>
>> >>>>> Motivation:
>> >>>>> By this BlueGreenClusterMigration PIP-188(
>> >>>>>
>> https://urldefense.com/v3/__https://github.com/apache/pulsar/pull/17962__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rAom047YrQ$
>> ), `redirectedClusterURI`
>> >>>> member
>> >>>>> var has been introduced in the pulsar client to hold the new
>> cluster's
>> >>>>> endpoint. Accordingly, pulsar client also added a state of a map of
>> >>>>> LookupService to lookup the migrated clusters. If
>> redirectedClusterURI
>> >>>> is
>> >>>>> not null, then clients look up the mapped,
>> >>>>> redirectedClusterURI->LookupService, when clients connect to
>> migrated
>> >>>>> brokers.
>> >>>>>
>> >>>>> Proposal:
>> >>>>> In fact, this logic can be simplified by directly updating the
>> >>>>> existing lookup service's service url. Then, we don't need to track
>> >>>> the new
>> >>>>> lookup services in a separate map and remove `redirectedClusterURI`,
>> >>>>> either.
>> >>>>>
>> >>>>> Hers is the sample code for this proposal:
>> >>>>>
>> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rArkuM1fbw$
>> >>>>>
>> >>>>> Pros:
>> >>>>> - The removal of redirectedClusterURI and the lookup service map can
>> >>>>> simplify the client code
>> >>>>> - It works better with other logics: Bundle Transfer and
>> >>>>> AutoClusterFailover, as these features currently only use the
>> original
>> >>>>> lookup service.
>> >>>>>
>> >>>>> Cons:
>> >>>>> - To consume backlog from the blue (old) cluster, new clients need
>> to
>> >>>> be
>> >>>>> created with the old service url(pointing to the old cluster)
>> because
>> >>>> the
>> >>>>> existing clients' lookup service will be already updated, pointing
>> to
>> >>>> the
>> >>>>> new cluster.
>> >>>>>
>> >>>>> However, I think this(requiring a new client with old service url,
>> old
>> >>>>> cluster) makes sense because all of the existing pub/sub will
>> anyway be
>> >>>>> reconnected to the new clusters as per the broker's close commands.
>> >>>> So, to
>> >>>>> consume the backlog from the old cluster, new client applications
>> need
>> >>>> to
>> >>>>> start with the old service url anyway.
>> >>>>>
>> >>>>> Example:
>> >>>>>
>> >>>>
>> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/files*diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312__;Iw!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rAoXqs6_Dg$
>> >>>>>
>> >>>>> Thanks,
>> >>>>> Heesung
>> >>>>
>> >>>
>>
>
Re: [E] Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Heesung Sohn <he...@apache.org>.
I see. Thank you for clarifying this requirement.
Then, I am wondering if we need to refactor the current
`lookup/urlLookupMap` access code to avoid using `lookup` when the
`redirectedClusterURI` is set at the consumers/producer layer. (it appears
that we still use `lookup` in the following code at the PulsarClientImpl
even when the consumers/producer might have migrated already(set
redirectedClusterURI already). Maybe I missed some other logic here, and
please also correct me if this behavior is fine.
Some of the `lookup` access examples:
https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L574
https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L951
https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L989
https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java#L999
If we need to make the clients use `urlLookupMap` after
producers/customers are migrated, then I guess we may need to consider the
following options.
Option A-1:
Pass `optional redirectedClusterURI` var to the above PulsarClientImpl
funcs to select the lookup or urlLookupMap inside.
Option A-2:
- Remove urlLookupMap from PulsarClientImpl
- Instead, add serviceNameResolverMap in LookupService
- Pass `optional redirectedClusterURI` var to the above PulsarClientImpl
funcs
- Pass `optional redirectedClusterURI` var to the above LookupService funcs
Regards,
Heesung
On Sat, Jan 27, 2024 at 10:33 AM Rajan Dhabalia
<rd...@yahooinc.com.invalid> wrote:
> Hi,
>
> Updating ServiceUrl in PulsarClient will make all producers and
> consumers of all topics to use that new url which is incorrect because
> redirection state can be different for each topic and even producer
> and consumer of the same topic. So, we can not update main serviceUrl
> but client should maintain redirection url for each producer and
> consumer based on its state decided by the server.
>
>
> Sent from my iPhone
>
> > On Jan 26, 2024, at 9:12 AM, Heesung Sohn <he...@apache.org> wrote:
> >
> > To add more context, I am trying to add this blue-green migration logic
> in
> > the cpp and golang client, and before copying the current logic, I am
> > wondering if we can simplify it.
> >
> > Again, since all of the producers and consumers from the same client and
> > topic need to migrate eventually,
> > I think directly updating the lookup service of that shared client to the
> > new cluster seems harmless.
> > I might be missing some other requirements here, so please correct me.
> > If we conclude keeping this redirectedClusterURI in HandlerState here, I
> > will go ahead and copy the current java client logic to other clients.
> >
> > Also, I updated the sample code for better synchronization and dedup.
> >
> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rArkuM1fbw$
> >
> > Thanks,
> > Heesung
> >
> >
> >
> >> On Thu, Jan 25, 2024 at 10:51 PM Heesung Sohn <he...@apache.org>
> wrote:
> >>
> >> I am sorry, but the current implementation of blue-green migration is
> for
> >> all topics (producers and customers), not topic-specific. Could you
> correct
> >> me if I am wrong here?
> >> So, for the current requirement(all topics migration), can't we just
> >> update the original lookup service for that client because all of the
> >> topics (of consumers and producers) will eventually migrate to the new
> >> cluster?
> >> Having said this, having redirectedClusterURI in HandlerState appears to
> >> be futuristic.
> >>
> >> Heesung
> >>
> >>
> >>
> >> On Thu, Jan 25, 2024 at 10:36 PM Heesung Sohn <he...@apache.org>
> wrote:
> >>
> >>>> This might not work because same PulsarClient can be used to create
> >>> multiple topics and producer/consumer and migration state can be
> different
> >>> per topic and even for produce/consumer (of same topic) so, changing
> >>> service url will not work for blue-green migration redirection for all
> >>> topics served by that PulsarClient.
> >>>
> >>> Good point on this topic (and producer and consumer) level migration
> >>> requirement. I was under the impression that the migration happens for
> all
> >>> topics from the old to the new cluster.
> >>>
> >>> For this requirement, I think we should keep redirectedClusterURI in
> >>> HandlerState.
> >>>
> >>> Thanks for this quick response.
> >>> Heesung
> >>>
> >>>
> >>> On Thu, Jan 25, 2024 at 10:05 PM Rajan Dhabalia <dhabalia.me@gmail.com
> >
> >>> wrote:
> >>>
> >>>> This might not work because same PulsarClient can be used to create
> >>>> multiple topics and producer/consumer and migration state can be
> different
> >>>> per topic and even for produce/consumer (of same topic) so, changing
> >>>> service url will not work for blue-green migration redirection for all
> >>>> topics served by that PulsarClient.
> >>>>
> >>>> Sent from my iPhone
> >>>>
> >>>>> On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org>
> wrote:
> >>>>>
> >>>>> Hi dev,
> >>>>>
> >>>>> I would like to discuss this change with the community.
> >>>>>
> >>>>> Motivation:
> >>>>> By this BlueGreenClusterMigration PIP-188(
> >>>>>
> https://urldefense.com/v3/__https://github.com/apache/pulsar/pull/17962__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rAom047YrQ$
> ), `redirectedClusterURI`
> >>>> member
> >>>>> var has been introduced in the pulsar client to hold the new
> cluster's
> >>>>> endpoint. Accordingly, pulsar client also added a state of a map of
> >>>>> LookupService to lookup the migrated clusters. If
> redirectedClusterURI
> >>>> is
> >>>>> not null, then clients look up the mapped,
> >>>>> redirectedClusterURI->LookupService, when clients connect to migrated
> >>>>> brokers.
> >>>>>
> >>>>> Proposal:
> >>>>> In fact, this logic can be simplified by directly updating the
> >>>>> existing lookup service's service url. Then, we don't need to track
> >>>> the new
> >>>>> lookup services in a separate map and remove `redirectedClusterURI`,
> >>>>> either.
> >>>>>
> >>>>> Hers is the sample code for this proposal:
> >>>>>
> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rArkuM1fbw$
> >>>>>
> >>>>> Pros:
> >>>>> - The removal of redirectedClusterURI and the lookup service map can
> >>>>> simplify the client code
> >>>>> - It works better with other logics: Bundle Transfer and
> >>>>> AutoClusterFailover, as these features currently only use the
> original
> >>>>> lookup service.
> >>>>>
> >>>>> Cons:
> >>>>> - To consume backlog from the blue (old) cluster, new clients need to
> >>>> be
> >>>>> created with the old service url(pointing to the old cluster) because
> >>>> the
> >>>>> existing clients' lookup service will be already updated, pointing to
> >>>> the
> >>>>> new cluster.
> >>>>>
> >>>>> However, I think this(requiring a new client with old service url,
> old
> >>>>> cluster) makes sense because all of the existing pub/sub will anyway
> be
> >>>>> reconnected to the new clusters as per the broker's close commands.
> >>>> So, to
> >>>>> consume the backlog from the old cluster, new client applications
> need
> >>>> to
> >>>>> start with the old service url anyway.
> >>>>>
> >>>>> Example:
> >>>>>
> >>>>
> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/files*diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312__;Iw!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rAoXqs6_Dg$
> >>>>>
> >>>>> Thanks,
> >>>>> Heesung
> >>>>
> >>>
>
Re: [E] Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Rajan Dhabalia <rd...@yahooinc.com.INVALID>.
Hi,
Updating ServiceUrl in PulsarClient will make all producers and
consumers of all topics to use that new url which is incorrect because
redirection state can be different for each topic and even producer
and consumer of the same topic. So, we can not update main serviceUrl
but client should maintain redirection url for each producer and
consumer based on its state decided by the server.
Sent from my iPhone
> On Jan 26, 2024, at 9:12 AM, Heesung Sohn <he...@apache.org> wrote:
>
> To add more context, I am trying to add this blue-green migration logic in
> the cpp and golang client, and before copying the current logic, I am
> wondering if we can simplify it.
>
> Again, since all of the producers and consumers from the same client and
> topic need to migrate eventually,
> I think directly updating the lookup service of that shared client to the
> new cluster seems harmless.
> I might be missing some other requirements here, so please correct me.
> If we conclude keeping this redirectedClusterURI in HandlerState here, I
> will go ahead and copy the current java client logic to other clients.
>
> Also, I updated the sample code for better synchronization and dedup.
> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rArkuM1fbw$
>
> Thanks,
> Heesung
>
>
>
>> On Thu, Jan 25, 2024 at 10:51 PM Heesung Sohn <he...@apache.org> wrote:
>>
>> I am sorry, but the current implementation of blue-green migration is for
>> all topics (producers and customers), not topic-specific. Could you correct
>> me if I am wrong here?
>> So, for the current requirement(all topics migration), can't we just
>> update the original lookup service for that client because all of the
>> topics (of consumers and producers) will eventually migrate to the new
>> cluster?
>> Having said this, having redirectedClusterURI in HandlerState appears to
>> be futuristic.
>>
>> Heesung
>>
>>
>>
>> On Thu, Jan 25, 2024 at 10:36 PM Heesung Sohn <he...@apache.org> wrote:
>>
>>>> This might not work because same PulsarClient can be used to create
>>> multiple topics and producer/consumer and migration state can be different
>>> per topic and even for produce/consumer (of same topic) so, changing
>>> service url will not work for blue-green migration redirection for all
>>> topics served by that PulsarClient.
>>>
>>> Good point on this topic (and producer and consumer) level migration
>>> requirement. I was under the impression that the migration happens for all
>>> topics from the old to the new cluster.
>>>
>>> For this requirement, I think we should keep redirectedClusterURI in
>>> HandlerState.
>>>
>>> Thanks for this quick response.
>>> Heesung
>>>
>>>
>>> On Thu, Jan 25, 2024 at 10:05 PM Rajan Dhabalia <dh...@gmail.com>
>>> wrote:
>>>
>>>> This might not work because same PulsarClient can be used to create
>>>> multiple topics and producer/consumer and migration state can be different
>>>> per topic and even for produce/consumer (of same topic) so, changing
>>>> service url will not work for blue-green migration redirection for all
>>>> topics served by that PulsarClient.
>>>>
>>>> Sent from my iPhone
>>>>
>>>>> On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org> wrote:
>>>>>
>>>>> Hi dev,
>>>>>
>>>>> I would like to discuss this change with the community.
>>>>>
>>>>> Motivation:
>>>>> By this BlueGreenClusterMigration PIP-188(
>>>>> https://urldefense.com/v3/__https://github.com/apache/pulsar/pull/17962__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rAom047YrQ$ ), `redirectedClusterURI`
>>>> member
>>>>> var has been introduced in the pulsar client to hold the new cluster's
>>>>> endpoint. Accordingly, pulsar client also added a state of a map of
>>>>> LookupService to lookup the migrated clusters. If redirectedClusterURI
>>>> is
>>>>> not null, then clients look up the mapped,
>>>>> redirectedClusterURI->LookupService, when clients connect to migrated
>>>>> brokers.
>>>>>
>>>>> Proposal:
>>>>> In fact, this logic can be simplified by directly updating the
>>>>> existing lookup service's service url. Then, we don't need to track
>>>> the new
>>>>> lookup services in a separate map and remove `redirectedClusterURI`,
>>>>> either.
>>>>>
>>>>> Hers is the sample code for this proposal:
>>>>> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/__;!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rArkuM1fbw$
>>>>>
>>>>> Pros:
>>>>> - The removal of redirectedClusterURI and the lookup service map can
>>>>> simplify the client code
>>>>> - It works better with other logics: Bundle Transfer and
>>>>> AutoClusterFailover, as these features currently only use the original
>>>>> lookup service.
>>>>>
>>>>> Cons:
>>>>> - To consume backlog from the blue (old) cluster, new clients need to
>>>> be
>>>>> created with the old service url(pointing to the old cluster) because
>>>> the
>>>>> existing clients' lookup service will be already updated, pointing to
>>>> the
>>>>> new cluster.
>>>>>
>>>>> However, I think this(requiring a new client with old service url, old
>>>>> cluster) makes sense because all of the existing pub/sub will anyway be
>>>>> reconnected to the new clusters as per the broker's close commands.
>>>> So, to
>>>>> consume the backlog from the old cluster, new client applications need
>>>> to
>>>>> start with the old service url anyway.
>>>>>
>>>>> Example:
>>>>>
>>>> https://urldefense.com/v3/__https://github.com/heesung-sn/pulsar/pull/59/files*diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312__;Iw!!Op6eflyXZCqGR5I!GOoO9eJK-kfDowteNFB-KOZpNcOrlY32vOe5Z86oxNS9wuxZjjqfwunkBYgxHjtBLEpnjga7rAoXqs6_Dg$
>>>>>
>>>>> Thanks,
>>>>> Heesung
>>>>
>>>
Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Heesung Sohn <he...@apache.org>.
To add more context, I am trying to add this blue-green migration logic in
the cpp and golang client, and before copying the current logic, I am
wondering if we can simplify it.
Again, since all of the producers and consumers from the same client and
topic need to migrate eventually,
I think directly updating the lookup service of that shared client to the
new cluster seems harmless.
I might be missing some other requirements here, so please correct me.
If we conclude keeping this redirectedClusterURI in HandlerState here, I
will go ahead and copy the current java client logic to other clients.
Also, I updated the sample code for better synchronization and dedup.
https://github.com/heesung-sn/pulsar/pull/59/
Thanks,
Heesung
On Thu, Jan 25, 2024 at 10:51 PM Heesung Sohn <he...@apache.org> wrote:
> I am sorry, but the current implementation of blue-green migration is for
> all topics (producers and customers), not topic-specific. Could you correct
> me if I am wrong here?
> So, for the current requirement(all topics migration), can't we just
> update the original lookup service for that client because all of the
> topics (of consumers and producers) will eventually migrate to the new
> cluster?
> Having said this, having redirectedClusterURI in HandlerState appears to
> be futuristic.
>
> Heesung
>
>
>
> On Thu, Jan 25, 2024 at 10:36 PM Heesung Sohn <he...@apache.org> wrote:
>
>> > This might not work because same PulsarClient can be used to create
>> multiple topics and producer/consumer and migration state can be different
>> per topic and even for produce/consumer (of same topic) so, changing
>> service url will not work for blue-green migration redirection for all
>> topics served by that PulsarClient.
>>
>> Good point on this topic (and producer and consumer) level migration
>> requirement. I was under the impression that the migration happens for all
>> topics from the old to the new cluster.
>>
>> For this requirement, I think we should keep redirectedClusterURI in
>> HandlerState.
>>
>> Thanks for this quick response.
>> Heesung
>>
>>
>> On Thu, Jan 25, 2024 at 10:05 PM Rajan Dhabalia <dh...@gmail.com>
>> wrote:
>>
>>> This might not work because same PulsarClient can be used to create
>>> multiple topics and producer/consumer and migration state can be different
>>> per topic and even for produce/consumer (of same topic) so, changing
>>> service url will not work for blue-green migration redirection for all
>>> topics served by that PulsarClient.
>>>
>>> Sent from my iPhone
>>>
>>> > On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org> wrote:
>>> >
>>> > Hi dev,
>>> >
>>> > I would like to discuss this change with the community.
>>> >
>>> > Motivation:
>>> > By this BlueGreenClusterMigration PIP-188(
>>> > https://github.com/apache/pulsar/pull/17962), `redirectedClusterURI`
>>> member
>>> > var has been introduced in the pulsar client to hold the new cluster's
>>> > endpoint. Accordingly, pulsar client also added a state of a map of
>>> > LookupService to lookup the migrated clusters. If redirectedClusterURI
>>> is
>>> > not null, then clients look up the mapped,
>>> > redirectedClusterURI->LookupService, when clients connect to migrated
>>> > brokers.
>>> >
>>> > Proposal:
>>> > In fact, this logic can be simplified by directly updating the
>>> > existing lookup service's service url. Then, we don't need to track
>>> the new
>>> > lookup services in a separate map and remove `redirectedClusterURI`,
>>> > either.
>>> >
>>> > Hers is the sample code for this proposal:
>>> > https://github.com/heesung-sn/pulsar/pull/59/
>>> >
>>> > Pros:
>>> > - The removal of redirectedClusterURI and the lookup service map can
>>> > simplify the client code
>>> > - It works better with other logics: Bundle Transfer and
>>> > AutoClusterFailover, as these features currently only use the original
>>> > lookup service.
>>> >
>>> > Cons:
>>> > - To consume backlog from the blue (old) cluster, new clients need to
>>> be
>>> > created with the old service url(pointing to the old cluster) because
>>> the
>>> > existing clients' lookup service will be already updated, pointing to
>>> the
>>> > new cluster.
>>> >
>>> > However, I think this(requiring a new client with old service url, old
>>> > cluster) makes sense because all of the existing pub/sub will anyway be
>>> > reconnected to the new clusters as per the broker's close commands.
>>> So, to
>>> > consume the backlog from the old cluster, new client applications need
>>> to
>>> > start with the old service url anyway.
>>> >
>>> > Example:
>>> >
>>> https://github.com/heesung-sn/pulsar/pull/59/files#diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312
>>> >
>>> > Thanks,
>>> > Heesung
>>>
>>
Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Heesung Sohn <he...@apache.org>.
I am sorry, but the current implementation of blue-green migration is for
all topics (producers and customers), not topic-specific. Could you correct
me if I am wrong here?
So, for the current requirement(all topics migration), can't we just update
the original lookup service for that client because all of the topics (of
consumers and producers) will eventually migrate to the new cluster?
Having said this, having redirectedClusterURI in HandlerState appears to be
futuristic.
Heesung
On Thu, Jan 25, 2024 at 10:36 PM Heesung Sohn <he...@apache.org> wrote:
> > This might not work because same PulsarClient can be used to create
> multiple topics and producer/consumer and migration state can be different
> per topic and even for produce/consumer (of same topic) so, changing
> service url will not work for blue-green migration redirection for all
> topics served by that PulsarClient.
>
> Good point on this topic (and producer and consumer) level migration
> requirement. I was under the impression that the migration happens for all
> topics from the old to the new cluster.
>
> For this requirement, I think we should keep redirectedClusterURI in
> HandlerState.
>
> Thanks for this quick response.
> Heesung
>
>
> On Thu, Jan 25, 2024 at 10:05 PM Rajan Dhabalia <dh...@gmail.com>
> wrote:
>
>> This might not work because same PulsarClient can be used to create
>> multiple topics and producer/consumer and migration state can be different
>> per topic and even for produce/consumer (of same topic) so, changing
>> service url will not work for blue-green migration redirection for all
>> topics served by that PulsarClient.
>>
>> Sent from my iPhone
>>
>> > On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org> wrote:
>> >
>> > Hi dev,
>> >
>> > I would like to discuss this change with the community.
>> >
>> > Motivation:
>> > By this BlueGreenClusterMigration PIP-188(
>> > https://github.com/apache/pulsar/pull/17962), `redirectedClusterURI`
>> member
>> > var has been introduced in the pulsar client to hold the new cluster's
>> > endpoint. Accordingly, pulsar client also added a state of a map of
>> > LookupService to lookup the migrated clusters. If redirectedClusterURI
>> is
>> > not null, then clients look up the mapped,
>> > redirectedClusterURI->LookupService, when clients connect to migrated
>> > brokers.
>> >
>> > Proposal:
>> > In fact, this logic can be simplified by directly updating the
>> > existing lookup service's service url. Then, we don't need to track the
>> new
>> > lookup services in a separate map and remove `redirectedClusterURI`,
>> > either.
>> >
>> > Hers is the sample code for this proposal:
>> > https://github.com/heesung-sn/pulsar/pull/59/
>> >
>> > Pros:
>> > - The removal of redirectedClusterURI and the lookup service map can
>> > simplify the client code
>> > - It works better with other logics: Bundle Transfer and
>> > AutoClusterFailover, as these features currently only use the original
>> > lookup service.
>> >
>> > Cons:
>> > - To consume backlog from the blue (old) cluster, new clients need to be
>> > created with the old service url(pointing to the old cluster) because
>> the
>> > existing clients' lookup service will be already updated, pointing to
>> the
>> > new cluster.
>> >
>> > However, I think this(requiring a new client with old service url, old
>> > cluster) makes sense because all of the existing pub/sub will anyway be
>> > reconnected to the new clusters as per the broker's close commands. So,
>> to
>> > consume the backlog from the old cluster, new client applications need
>> to
>> > start with the old service url anyway.
>> >
>> > Example:
>> >
>> https://github.com/heesung-sn/pulsar/pull/59/files#diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312
>> >
>> > Thanks,
>> > Heesung
>>
>
Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Heesung Sohn <he...@apache.org>.
> This might not work because same PulsarClient can be used to create
multiple topics and producer/consumer and migration state can be different
per topic and even for produce/consumer (of same topic) so, changing
service url will not work for blue-green migration redirection for all
topics served by that PulsarClient.
Good point on this topic (and producer and consumer) level migration
requirement. I was under the impression that the migration happens for all
topics from the old to the new cluster.
For this requirement, I think we should keep redirectedClusterURI in
HandlerState.
Thanks for this quick response.
Heesung
On Thu, Jan 25, 2024 at 10:05 PM Rajan Dhabalia <dh...@gmail.com>
wrote:
> This might not work because same PulsarClient can be used to create
> multiple topics and producer/consumer and migration state can be different
> per topic and even for produce/consumer (of same topic) so, changing
> service url will not work for blue-green migration redirection for all
> topics served by that PulsarClient.
>
> Sent from my iPhone
>
> > On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org> wrote:
> >
> > Hi dev,
> >
> > I would like to discuss this change with the community.
> >
> > Motivation:
> > By this BlueGreenClusterMigration PIP-188(
> > https://github.com/apache/pulsar/pull/17962), `redirectedClusterURI`
> member
> > var has been introduced in the pulsar client to hold the new cluster's
> > endpoint. Accordingly, pulsar client also added a state of a map of
> > LookupService to lookup the migrated clusters. If redirectedClusterURI is
> > not null, then clients look up the mapped,
> > redirectedClusterURI->LookupService, when clients connect to migrated
> > brokers.
> >
> > Proposal:
> > In fact, this logic can be simplified by directly updating the
> > existing lookup service's service url. Then, we don't need to track the
> new
> > lookup services in a separate map and remove `redirectedClusterURI`,
> > either.
> >
> > Hers is the sample code for this proposal:
> > https://github.com/heesung-sn/pulsar/pull/59/
> >
> > Pros:
> > - The removal of redirectedClusterURI and the lookup service map can
> > simplify the client code
> > - It works better with other logics: Bundle Transfer and
> > AutoClusterFailover, as these features currently only use the original
> > lookup service.
> >
> > Cons:
> > - To consume backlog from the blue (old) cluster, new clients need to be
> > created with the old service url(pointing to the old cluster) because the
> > existing clients' lookup service will be already updated, pointing to the
> > new cluster.
> >
> > However, I think this(requiring a new client with old service url, old
> > cluster) makes sense because all of the existing pub/sub will anyway be
> > reconnected to the new clusters as per the broker's close commands. So,
> to
> > consume the backlog from the old cluster, new client applications need to
> > start with the old service url anyway.
> >
> > Example:
> >
> https://github.com/heesung-sn/pulsar/pull/59/files#diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312
> >
> > Thanks,
> > Heesung
>
Re: [DISCUSS] PIP-188(BlueGreenClusterMigration) Remove redirectedClusterURI in Pulsar Client
Posted by Rajan Dhabalia <dh...@gmail.com>.
This might not work because same PulsarClient can be used to create multiple topics and producer/consumer and migration state can be different per topic and even for produce/consumer (of same topic) so, changing service url will not work for blue-green migration redirection for all topics served by that PulsarClient.
Sent from my iPhone
> On Jan 25, 2024, at 6:57 PM, Heesung Sohn <he...@apache.org> wrote:
>
> Hi dev,
>
> I would like to discuss this change with the community.
>
> Motivation:
> By this BlueGreenClusterMigration PIP-188(
> https://github.com/apache/pulsar/pull/17962), `redirectedClusterURI` member
> var has been introduced in the pulsar client to hold the new cluster's
> endpoint. Accordingly, pulsar client also added a state of a map of
> LookupService to lookup the migrated clusters. If redirectedClusterURI is
> not null, then clients look up the mapped,
> redirectedClusterURI->LookupService, when clients connect to migrated
> brokers.
>
> Proposal:
> In fact, this logic can be simplified by directly updating the
> existing lookup service's service url. Then, we don't need to track the new
> lookup services in a separate map and remove `redirectedClusterURI`,
> either.
>
> Hers is the sample code for this proposal:
> https://github.com/heesung-sn/pulsar/pull/59/
>
> Pros:
> - The removal of redirectedClusterURI and the lookup service map can
> simplify the client code
> - It works better with other logics: Bundle Transfer and
> AutoClusterFailover, as these features currently only use the original
> lookup service.
>
> Cons:
> - To consume backlog from the blue (old) cluster, new clients need to be
> created with the old service url(pointing to the old cluster) because the
> existing clients' lookup service will be already updated, pointing to the
> new cluster.
>
> However, I think this(requiring a new client with old service url, old
> cluster) makes sense because all of the existing pub/sub will anyway be
> reconnected to the new clusters as per the broker's close commands. So, to
> consume the backlog from the old cluster, new client applications need to
> start with the old service url anyway.
>
> Example:
> https://github.com/heesung-sn/pulsar/pull/59/files#diff-3c745333eaeae9e8a87e92495fbf97b7c952c034b8c18ca92156f07cd5b1f5afR308-R312
>
> Thanks,
> Heesung