init: order dynamic resource initialization to make RTDS always be first. Take 2.#10989
Conversation
…rst. Take 2. Signed-off-by: Yan Avlasov <yavlasov@google.com>
…ization Signed-off-by: Yan Avlasov <yavlasov@google.com>
|
Giving it a try now. |
|
@yanavlasov so far so good 👍 |
snowp
left a comment
There was a problem hiding this comment.
Thanks this looks like a reasonable fix to the bug, just a couple comments
mattklein123
left a comment
There was a problem hiding this comment.
Thanks LGTM other than existing small comments.
/wait
| enum class State { | ||
| // Initial state. During this state all static clusters are loaded. Any primary clusters | ||
| // are immediately initialized. | ||
| // are immediately begin initialization. |
There was a problem hiding this comment.
? I think what was there before made more sense?
There was a problem hiding this comment.
I was actually confused by this since 'immediately initializedmeant to meimmediately complete initialization` (and I have also assumed that initialized cluster have completed discovery of endpoints). Which is not the case for DNS based clusters these with health checks. I can revert this if I'm confusing the meaning of this statement.
There was a problem hiding this comment.
Ah OK, I think just a grammar issue (remove "are"):
"... immediately begin initialization"
There was a problem hiding this comment.
All your base are belong to us
Signed-off-by: Yan Avlasov <yavlasov@google.com>
Commit Message:
Order dynamic resource initialization such that RTDS is completely initialized before starting to load rest of xDS resources.
Additional Description:
This PR puts back the original commit with xDS ordering and fixes the bug where cluster manager did not handle the case of asynchronous completion of primary cluster initialization.
Risk Level: High (changes to xDS and cluster initialization)
Testing: Unit Tests, Integration Tests and repro case
Docs Changes: N/A
Release Notes: N/A
Fixes #9709