AlexStocks commented on code in PR #135:
URL:
https://github.com/apache/dubbo-go-pixiu-samples/pull/135#discussion_r3161591159
##########
dubbogo/simple/direct/server/app/server.go:
##########
@@ -38,7 +38,7 @@ var survivalTimeout = int(3e9)
// export DUBBO_GO_CONFIG_PATH="../profiles/dev/server.yml"
// export APP_LOG_CONF_FILE="../profiles/dev/log.yml"
func main() {
- config.Load()
+ dubbo.Load()
Review Comment:
[P1] `dubbo.Load()` 这里已经有返回值了，但当前直接忽略，`errcheck`
也已经命中这一行。配置文件缺失或解析失败时，样例会继续往下执行并在更后面以更难定位的方式崩掉。这里需要显式检查并处理错误；同样的问题在
`dubbotripleproxy/server/dubbo/app/server.go:38` 和
`dubbotripleproxy/server/triple/app/server.go:38` 也重复出现。
##########
dubbotripleproxy/server/dubbo/app/server.go:
##########
@@ -35,7 +35,7 @@ import (
var survivalTimeout = int(3e9)
func main() {
- config.Load()
+ dubbo.Load()
Review Comment:
[P1] 这里把 `config.Load()` 替换成了有返回值的 `dubbo.Load()`，但没有检查 error。当前
`golangci-lint` 已经在这行报
`errcheck`；一旦样例配置加载失败，进程会继续执行，问题会被延后暴露。需要和旧实现一样在入口处把配置加载失败明确处理掉。
##########
dubbotripleproxy/server/triple/app/server.go:
##########
@@ -35,7 +35,7 @@ import (
var survivalTimeout = int(3e9)
func main() {
- config.Load()
+ dubbo.Load()
Review Comment:
[P1] 这里同样忽略了 `dubbo.Load()` 的返回错误，`golangci-lint` 已经直接报了
`errcheck`。入口函数吞掉配置加载错误会让样例在运行期以更隐蔽的方式失败，建议在这里显式返回/`panic`。
##########
shutdown/dubbo/test/pixiu_test.go:
##########
@@ -39,26 +39,25 @@ import (
"github.com/stretchr/testify/assert"
)
-func newDubboRefConf(iface, protocol string) dconfig.ReferenceConfig {
- refConf := dconfig.ReferenceConfig{
- InterfaceName: iface,
- Cluster: "failover",
- RegistryIDs: []string{"zk"},
- Protocol: protocol,
- Generic: "true",
- URL: "dubbo://127.0.0.1:8889/" + iface,
- Group: "test",
- Version: "1.0.0",
- RequestTimeout: "10s",
+func newDubboGenericService(iface, protocol string) *generic.GenericService {
+ cli, err := client.NewClient()
+ if err != nil {
+ panic(err)
}
- rootConfig := dconfig.NewRootConfigBuilder().
- Build()
- if err := dconfig.Load(dconfig.WithRootConfig(rootConfig)); err != nil {
+
+ svc, err := cli.NewGenericService(
Review Comment:
[P1] 这里切到 `client.NewGenericService` 之后，样例行为已经回归了。实际执行 `go test ./...`
时，`TestDubboListenShutdown` 会失败，日志里能复现 `session Already Closed`，最终 `count` 只能到
1 而不是断言里的 0。说明这个新建 generic client 的方式和原来
`ReferenceConfig.GenericLoad(dubbo.io)` 并不等价，当前改法已经把 shutdown
样例跑坏了；需要先补齐与旧配置路径一致的 generic 直连行为，再改测试。
##########
shutdown/triple/test/pixiu_test.go:
##########
@@ -43,29 +43,25 @@ import (
var count int32
-func newTripleRefConf(iface, protocol string) dconfig.ReferenceConfig {
-
- refConf := dconfig.ReferenceConfig{
- InterfaceName: iface,
- Cluster: "failover",
- RegistryIDs: []string{"zk"},
- Protocol: protocol,
- Generic: "true",
- Group: "test",
- Version: "1.0.0",
- URL: "tri://127.0.0.1:9999/" + iface + "?" +
constant.SerializationKey + "=hessian2",
- RequestTimeout: "10s",
+func newTripleGenericService(iface, protocol string) *generic.GenericService {
+ cli, err := client.NewClient()
+ if err != nil {
+ panic(err)
}
- rootConfig := dconfig.NewRootConfigBuilder().
- Build()
- if err := dconfig.Load(dconfig.WithRootConfig(rootConfig)); err != nil {
+ svc, err := cli.NewGenericService(
Review Comment:
[P1] Triple 这边也有同样的回归。`go test ./...` 里 `TestTripleListenShutdown`
现在稳定失败，前两次调用会直接报 `Requested dubbo rpc invocation route not found`，最后 `count` 只到
2 而不是断言期望的 0。也就是说把 `ReferenceConfig.GenericLoad(dubbo.io)` 换成
`client.NewGenericService(...)` 后，请求在样例启动阶段就已经打不通了，shutdown 断言自然不再成立。
--
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]
