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 <hees...@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 <hees...@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 <hees...@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...@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 <hees...@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 >>>> >>>
