[GitHub] [druid] gianm commented on a change in pull request #10779: [Bug Fix] Broker will not wait for its SQL metadata view to fully initialize before starting up, even though set awaitInitializationOnStart true

[GitBox <gi...@apache.org>]

gianm commented on a change in pull request #10779:
URL: https://github.com/apache/druid/pull/10779#discussion_r560770693



##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java
##########
@@ -242,7 +242,9 @@ public void start() throws InterruptedException
                       break;
                     }
 
-                    if (isServerViewInitialized) {
+                    // lastFailure != 0L means exceptions happened before and there're some refresh work was not completed.
+                    // so that even ServerView is initialized, we can't let broker complete initialization.
+                    if (isServerViewInitialized && lastFailure == 0L) {

Review comment:
       I think this is saying that we will mark as initialized when all of these are true:
   
   (a) no refresh is needed
   (b) the server view is initialized
   (c) there wasn't a refresh that just failed
   
   Sounds good to me - thanks for the fix!
   
   Do you think you could add a test for this case too? Maybe something like DruidSchemaTest, but that creates a DruidSchema using a QueryLifecycleFactory where the first query fails fail, and then after that they start succeeding. We'd want to make sure that when `awaitInitialization()` returns, things are really initialized.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org