Demogorgon314 commented on code in PR #20748:
URL: https://github.com/apache/pulsar/pull/20748#discussion_r1280060503
##########
pip/pip-281.md:
##########
@@ -0,0 +1,233 @@
+<!--
+RULES
+* Never place a link to an external site like Google Doc. The proposal should
be in this issue entirely.
+* Use a spelling and grammar checker tools if available for you (there are
plenty of free ones).
+
+PROPOSAL HEALTH CHECK
+I can read the design document and understand the problem statement and what
you plan to change *without* resorting to a couple of hours of code reading
just to start having a high level understanding of the change.
+
+THIS COMMENTS
+Please remove them when done.
+-->
+
+# Background knowledge
+
+- Pulsar broker load balancer periodically unloads bundles from overloaded
brokers. During this unload process, previous owner brokers close topic
sessions(e.g. producers, subscriptions(consumers), managed ledgers). When
re-assigned, new owner brokers recreate the topic sessions.
+
+- Pulsar clients request `CommandLookupTopic` to lookup or assign owner
brokers for topics and connect to them.
+
+- PIP-192, the extensible load balancer introduced the bundle state channel
that event-sources this unloading process in a state machine manner, from
`releasing,` `assigned`, to `owned` state order. At `releasing,` the owner
broker "releases" the bundle ownership(close topic sessions).
+
+- PIP-192, the extensible load balancer introduced TransferShedder, a new
shedding strategy, which pre-assigns new owner brokers beforehand.
+
+
+# Motivation
+
+- When unloading closes many topic sessions, then many clients need to request
CommandLookupTopic at the same time, which could cause many lookup requests on
brokers. This unloading process can be further optimized if we can let the
client directly connect to the new owner broker without following
`CommandLookupTopic` requests.
+- In the new load balancer(pip-192), since the owner broker is already known,
we can modify the close command protocol to pass the new destination broker URL
and skip the lookup requests.
+- Also, when unloading, we can gracefully shutdown ledgers -- we always close
old managed ledgers first and then recreate it on the new owner without
conflicts.
+
+# Goals
+- Remove clients' lookup requests in the unload protocol
+- Gracefully shutdown managed ledgers before new owners create them.
+
+## In Scope
+
+<!--
+What this PIP intend to achieve once It's integrated into Pulsar.
+Why does it benefit Pulsar.
+-->
+
+- This change will be added in the extensible load balancer.
+
+## Out of Scope
+
+<!--
+Describe what you have decided to keep out of scope, perhaps left for a
different PIP/s.
+-->
+
+- This won't change the existing load balancer behavior(modular load manager).
+
+
+
+# High Level Design
+
+<!--
+Describe the design of your solution in *high level*.
+Describe the solution end to end, from a birds-eye view.
+Don't go into implementation details in this section.
+
+I should be able to finish reading from beginning of the PIP to here
(including) and understand the feature and
+how you intend to solve it, end to end.
+
+DON'T
+* Avoid code snippets, unless it's essential to explain your intent.
+-->
+
+Current Unload and Lookup Sequence in Extensible Load Balancer
+```mermaid
+sequenceDiagram
+ participant Clients
+ participant Owner Broker
+ participant New Owner Broker
+ participant Leader Broker
+ Leader Broker ->> Owner Broker: "state:Releasing:" close topic
+ Owner Broker ->> Owner Broker: close broker topic sessions
+ Owner Broker ->> Clients: close producers and consumers
+ Clients ->> Clients: reconnecting (inital delay 100ms)
+ Owner Broker ->> New Owner Broker: "state:Assign:" assign new ownership
+ New Owner Broker ->> Owner Broker: "state:Owned:" ack new ownership
+ Clients ->> Owner Broker: lookup
+ Owner Broker ->> Clients: redirect
+ Clients ->> New Owner Broker: lookup
+ New Owner Broker ->> Clients: return(connected)
+```
+
+Proposed Unload Sequence in Extensible Load Balancer without Lookup
+```mermaid
+sequenceDiagram
+ participant Clients
+ participant Owner Broker
+ participant New Owner Broker
+ participant Leader Broker
+ Leader Broker ->> Owner Broker: "state:Releasing:" close topic
+ Owner Broker ->> Owner Broker: close broker topic sessions(e.g ledgers)
without disconnecting producers/consumers(fenced)
+ Clients -->> Owner Broker: message pubs are ignored
+ Owner Broker ->> New Owner Broker: "state:Assign:" assign new ownership
+ New Owner Broker ->> Owner Broker: "state:Owned:" ack new ownership
+ Owner Broker ->> Owner Broker: close the fenced broker topic sessions
+ Owner Broker ->> Clients: close producers and consumers (with
newOwnerBrokerUrl)
+ Clients ->> New Owner Broker: immediately connect
+```
+
+
+# Detailed Design
+
+## Design & Implementation Details
+
+<!--
+This is the section where you dive into the details. It can be:
+* Concrete class names and their roles and responsibility, including methods.
+* Code snippets of existing code.
+* Interface names and its methods.
+* ...
+-->
+
+- Modify CommandCloseProducer, CommandCloseConsumer to pass optional
brokerServiceUrls
+```
+message CommandCloseProducer {
Review Comment:
Ok, that makes sense. Thanks for the explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
