Similarityoung commented on code in PR #994:
URL: https://github.com/apache/dubbo-go-pixiu/pull/994#discussion_r3610361755
##########
pkg/config/xds/xds.go:
##########
@@ -107,34 +116,51 @@ func (a *Xds) readDubboServiceFromListener() ([]string,
error) {
return dubboServices, nil
}
-func (a *Xds) Start() {
+func (a *Xds) Start() error {
if a.dynamicResourceMg == nil { // if dm is nil, then config not
initialized.
logger.Infof("can not get dynamic resource manager. maybe the
config has not initialized")
- return
+ return nil
}
apiclient.Init(a.clusterMg)
+ // lds and cds are independent dynamic resource watches: a failure in
one must not skip the
+ // other's initialization. Collect each error and fail startup if any
occurred, so a missing
+ // config or xDS connection failure surfaces to the caller instead of
leaving the process in a
+ // false-healthy state with parts of the dynamic config unavailable.
+ var errs []error
+
// lds fetch just run on init phase.
if a.dynamicResourceMg.GetLds() != nil {
- a.lds = &LdsManager{
- DiscoverApi:
a.createApiManager(a.dynamicResourceMg.GetLds(), a.dynamicResourceMg.GetNode(),
constant.ListenerType),
- listenerMg: a.listenerMg,
- }
- if err := a.lds.Delta(); err != nil {
- logger.Errorf("can not fetch lds err is %+v", err)
+ discoverApi, err :=
a.createApiManager(a.dynamicResourceMg.GetLds(), a.dynamicResourceMg.GetNode(),
constant.ListenerType)
+ if err != nil {
+ errs = append(errs, errors.Wrap(err, "create LDS api
manager"))
+ } else {
+ a.lds = &LdsManager{
+ DiscoverApi: discoverApi,
+ listenerMg: a.listenerMg,
+ }
+ if err := a.lds.Delta(); err != nil {
+ errs = append(errs, errors.Wrap(err, "fetch
lds"))
+ }
}
}
// catch the ongoing cds config change.
if a.dynamicResourceMg.GetCds() != nil {
- a.cds = &CdsManager{
- DiscoverApi:
a.createApiManager(a.dynamicResourceMg.GetCds(), a.dynamicResourceMg.GetNode(),
constant.ClusterType),
- clusterMg: a.clusterMg,
- }
- if err := a.cds.Delta(); err != nil {
- logger.Errorf("can not fetch lds")
+ discoverApi, err :=
a.createApiManager(a.dynamicResourceMg.GetCds(), a.dynamicResourceMg.GetNode(),
constant.ClusterType)
+ if err != nil {
+ errs = append(errs, errors.Wrap(err, "create CDS api
manager"))
Review Comment:
当 LDS 和 CDS 像仓库示例一样共用一个 xDS cluster 且首次 gRPC Dial 失败时，LDS 分支会记录错误并继续执行这里的
CDS 分支。 `GRPCCluster.GetConnection` 的 `sync.Once` 已完成却没有保存首次错误，第二次调用返回 `(nil,
nil)`，随后 `DeltaExtensionConfigs` 对 nil `ClientConn` 调用 `NewStream` 并再次
panic，新增的启动错误传播因此失效。 请持久化连接初始化错误或允许安全重试，并增加 LDS/CDS 共用不可达 cluster 时只返回错误、不发生
panic 的测试。
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
