Better handle the error

sundy-li commented 5 years ago

你好，我发现代码里面有很多glog.Fataf代码，比如在拉取kafka数据的时候，出现了异常应该是返回异常，而不是在代码里面强制panic

示例日志

E1208 14:31:40.464005   27254 group_consumer.go:397] failed to send heartbeat:The group is rebalancing, so a rejoin is needed.
E1208 14:31:40.495822   27254 simple_consumer.go:246] fetch error:read tcp4 10.68.9.150:34666->10.68.33.108:7560: use of closed network connection
E1208 14:31:40.543143   27254 simple_consumer.go:246] fetch error:read tcp4 10.68.9.150:30924->10.68.33.103:7560: use of closed network connection
E1208 14:31:40.596464   27254 simple_consumer.go:246] fetch error:read tcp4 10.68.9.150:30024->10.68.33.110:7560: use of closed network connection
E1208 14:32:09.100145   27254 group_consumer.go:397] failed to send heartbeat:The group is rebalancing, so a rejoin is needed.
E1208 14:32:09.101009   27254 brokers.go:236] get metadata of [metric_kafka_union] from 10.68.33.104:7560 error: EOF
E1208 14:32:09.304125   27254 brokers.go:236] get metadata of [metric_kafka_union] from 10.68.33.105:7560 error: EOF
E1208 14:32:09.504353   27254 brokers.go:236] get metadata of [metric_kafka_union] from 10.68.33.110:7560 error: EOF
E1208 14:32:09.704588   27254 brokers.go:236] get metadata of [metric_kafka_union] from 10.68.33.103:7560 error: EOF
F1208 14:34:09.553053   27254 simple_consumer.go:199] could not get offset metric_kafka_union[54]:EOF
goroutine 19398578 [running]:
github.com/golang/glog.stacks(0xc4403f0f00, 0xc46c5c2c30, 0x66, 0xc6)
    /Users/sundy/pan/gopath/src/github.com/golang/glog/glog.go:769 +0xcf
github.com/golang/glog.(*loggingT).output(0xfc13a0, 0xc400000003, 0xc4ad4db130, 0xf6b2cd, 0x12, 0xc7, 0x0)
    /Users/sundy/pan/gopath/src/github.com/golang/glog/glog.go:720 +0x32d
github.com/golang/glog.(*loggingT).printf(0xfc13a0, 0xc400000003, 0xbb5d1e, 0x1e, 0xc424ab5ee0, 0x3, 0x3)
    /Users/sundy/pan/gopath/src/github.com/golang/glog/glog.go:655 +0x14b
github.com/golang/glog.Fatalf(0xbb5d1e, 0x1e, 0xc424ab5ee0, 0x3, 0x3)

childe commented 5 years ago

你说的很对~

虽然有些error有fatal的理由, 也最好不要这样做, 而是传error出来给用户自行判断.

我去改

childe commented 5 years ago

处理了绝大部分, 还有两个地方保留了 fatal.

simple_consumer.go:                     glog.Fatalf("invalid config: %s", invallidOffsetsStorageConfig)
simple_consumer.go:                                             glog.Fatal("buffers still open??")

第一个是因为, 首先, 这个配置在group consumer生成的时候会返回错误, 所以正常来说走不到这一步. 如果真的到这里了(目前想不到怎么可能), 那这个错误是致命的, 因为offset没地方存储. 如果帮用户默认纠错, 很可能会有误会.

第二个其实也是debug性质的, 我认为代码到不了这里, 万一有fatal, 需要我去debug代码哪里有问题了.

sundy-li commented 5 years ago

👍

sundy-li commented 5 years ago

第一个我觉得可以在consumer初始化进行检查，而不要在运行消费的时候检查，我新建了一个pr，请过目。

childe commented 5 years ago

谢谢你的PR~

第一个其实我在初始化的时候检查了(今天下午提交的 f72dc1fc0064db5ee9f38de8d56c125fdb12d6c7 里面), 如果配置不对, 直接返回错误, 所以我说"正常说走不到fatal那里".

这个配置只在 GroupConsumer 里面才可以配置, 因为其他的 simple consumer 和 console consumer 其实都不会做 offset commit .

childe / healer

Better handle the error #3