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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
