heesung-sn commented on code in PR #19730:
URL: https://github.com/apache/pulsar/pull/19730#discussion_r1132844370
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/store/TableViewLoadDataStoreImpl.java:
##########
@@ -61,30 +70,58 @@ public CompletableFuture<Void> removeAsync(String key) {
@Override
public Optional<T> get(String key) {
+ validateTableViewStart();
Review Comment:
> ExtensibleLoadManager.start() and playLeader() are complete
I meant this `complete` as everything initialized correctly without an error.
Yes. Currently, if `topBundlesLoadDataStore.startTableView();` fails, the
leader broker still proceeds, and the TransferShedder will constantly make
UnloadDecision(fail).
However, lazy-init(tableview.start ) in validateTableViewStart could also
fail and lead to the same situation (TransferShedder will constantly make
UnloadDecision(fail)). Besides, this lazy-init can lead to unknown
behaviors(many consumers on this topBundlesLoad topic) when other brokers
access the tableview.
The bigger question is how Pulsar handles system state change failures. In
this case, the state change is from playLeader or playFollower.
I can think of the following options:
Option 1: retry-forever
Option 2: fail-fast(shutdown broker)
Option 3: ignore (leader or followers will run in the invalid state, and
there will be many error logs and metrics. Eventually, the operator needs to
capture this and fix it, probably by broker restart)
AFAIK, upon system state change failures, other systems immediately fail
fast with meaningful logs and even with coredump. However, I don't think this
`fail-fast` is the practice in the Pulsar project.
I can update the code for option 1 (retry-forever).
Please let me know if you have more concerns.
--
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]
