重构driver接口

[dxyinme]

当前的driver接口有一些冗余设计，现在提出新的设计

type DriverV2 interface {
    Init(serviceName string, timeout time.Duration, logger dlog.Logger)
    GetServiceNodeList() ([]string, error)
    Start() error
}

旧版driver对每个driver的新建，都要使用特定的Conf，这其实是没必要的，我们完全可以只传入对应的client即可。

如此设计的原因：

1. 对于registerNode/SetXXX等只需要在初始化时候调用的接口，我们使用Init来封装，降低接口复杂度
2. 减少非必要的重复配置

[dxyinme]

修改例子https://github.com/dxyinme/dcron/blob/master/driver/v2/redisdriver.go

[libi]

可以增加一个 NodeUpdateNotify()(nodeList <-chan []string) , driver返回一个通知管道， 当节点有变化时主动通知dcron更新hash。

[dxyinme]

可以增加一个 NodeUpdateNotify()(nodeList <-chan []string) , driver返回一个通知管道， 当节点有变化时主动通知dcron更新hash。

我觉得可以直接改为

type DriverV2 interface {
    Init(serviceName string, timeout time.Duration, logger dlog.Logger)
    Start() (err error)
        NodeID() string
        NodeUpdateNotify() (nodesChan chan []string)
}

[libi]

start 和 channel 返回放一起的话，就会要求节点启动时生成channel, 单个方法内的功能耦合度有点高。当调用方需要获取 通知管道时就只能调用Start逻辑或者提前把channel保存起来。

[Casper-Mars]

请问为什么还需要Init？是有什么场景Start满足不了吗？好像timeout、logger这种可以作为option吧

type DriverV2 interface {
    Start(ctx context.Context, serviceName string) (err error)
        Stop(ctx context.Context) (err error)
        NodeID(ctx context.Context) string
        NodeUpdateNotify(ctx context.Context) (nodesChan chan []string)
}

[dxyinme]

``> 请问为什么还需要Init？是有什么场景Start满足不了吗？好像timeout、logger这种可以作为option吧

type DriverV2 interface {
  Start(ctx context.Context, serviceName string) (err error)
        Stop(ctx context.Context) (err error)
        NodeID(ctx context.Context) string
        NodeUpdateNotify(ctx context.Context) (nodesChan chan []string)
}

对不起没有及时更新设计，目前的接口设计应该是这样的

type DriverV2 interface {
    // init driver
    Init(serviceName string, opts ...Option)
    // get nodeID
    NodeID() string
    // get nodes
    GetNodes() (nodes []string, err error)
    Start() (err error)
    Stop() (err error)

    withOption(opt Option) (err error)
}

timeout，和logger确实可以被作为option，这点正在改进；Start 和 Init 拆分开的原因是因为driver可以被Stop后重新Start，此时如果serviceName在start内传入其实是不合理的。所以将Start和Init拆分开了

[Casper-Mars]

``> 请问为什么还需要Init？是有什么场景Start满足不了吗？好像timeout、logger这种可以作为option吧

type DriverV2 interface {
    Start(ctx context.Context, serviceName string) (err error)
        Stop(ctx context.Context) (err error)
        NodeID(ctx context.Context) string
        NodeUpdateNotify(ctx context.Context) (nodesChan chan []string)
}

对不起没有及时更新设计，目前的接口设计应该是这样的

type DriverV2 interface {
  // init driver
  Init(serviceName string, timeout time.Duration, logger dlog.Logger)
  // get nodeID
  NodeID() string
  // get nodes
  GetNodes() (nodes []string, err error)
  Start() (err error)
  Stop() (err error)
}

timeout，和logger确实可以被作为option，这点正在改进；Start 和 Init 拆分开的原因是因为driver可以被Stop后重新Start，此时如果serviceName在start内传入其实是不合理的。所以将Start和Init拆分开了

噢噢，明白了，不过为什么不用上下文ctx？我看现在redis的实现，都是直接用background的，不需要一个超时的限制吗？

[libi]

可以给 Start 和 Stop增加 ctx ，用来控制启动停止时可能的执行超时（驱动连接重试超时等）。 但是这两个 ctx 与驱动内部每次请求控制超时的 ctx 还不是一回事，驱动内部实现应该根据传入的超时参数通过 生成新的ctx或者驱动参数 来控制超时。

[Casper-Mars]

可以给 Start 和 Stop增加 ctx ，用来控制启动停止时可能的执行超时（驱动连接重试超时等）。 但是这两个 ctx 与驱动内部每次请求控制超时的 ctx 还不是一回事，驱动内部实现应该根据传入的超时参数通过 生成新的ctx或者驱动参数 来控制超时。

讨论的不是驱动接口吗？抽象层的设计应该不需要管具体实现吧，设计抽象层考虑的不是扩展性、通用性、可读性、SLOID原则之类吗？接口定义有传入ctx，但是具体实现也是可以不用。而且让调用方管理上下文扩展性更好吧。

[dxyinme]

可以给 Start 和 Stop增加 ctx ，用来控制启动停止时可能的执行超时（驱动连接重试超时等）。 但是这两个 ctx 与驱动内部每次请求控制超时的 ctx 还不是一回事，驱动内部实现应该根据传入的超时参数通过 生成新的ctx或者驱动参数 来控制超时。

讨论的不是驱动接口吗？抽象层的设计应该不需要管具体实现吧，设计抽象层考虑的不是扩展性、通用性、可读性、SLOID原则之类吗？接口定义有传入ctx，但是具体实现也是可以不用。而且让调用方管理上下文扩展性更好吧。

这里的驱动只在node_pool中使用到，可以算作一个内部使用类，暂时不需要使用context管理超时我觉得。减少接口的参数也是方便用户对driver进行自定义

[Casper-Mars]

可以给 Start 和 Stop增加 ctx ，用来控制启动停止时可能的执行超时（驱动连接重试超时等）。 但是这两个 ctx 与驱动内部每次请求控制超时的 ctx 还不是一回事，驱动内部实现应该根据传入的超时参数通过 生成新的ctx或者驱动参数 来控制超时。

讨论的不是驱动接口吗？抽象层的设计应该不需要管具体实现吧，设计抽象层考虑的不是扩展性、通用性、可读性、SLOID原则之类吗？接口定义有传入ctx，但是具体实现也是可以不用。而且让调用方管理上下文扩展性更好吧。

这里的驱动只在node_pool中使用到，可以算作一个内部使用类，暂时不需要使用context管理超时我觉得。减少接口的参数也是方便用户对driver进行自定义

确实，如果作为一个内部的接口只需要关注内聚面就可以了。不过这里定义的是一个公开的接口，用户自定义driver，上下文也是很重要的吧，例如需要ctx做协程控制。再者ctx不仅仅是超时控制，也是做客观性的一种手段，例如我司是基于ctx把redis的调用纳入到观测平台。而且减少上下文参数为什么能方便用户自定义？

[ailose]

redis驱动用scan 实现node注册很不合理 可以考虑一下 redis pub/sub 和 zset 配合

用scan实现避免不了（如果那个redis db存在大量key的问题 ）

[dxyinme]

我觉得这个可以在后续的实现里补上。减少上下文参数其实只是降低心智负担，毕竟传了不用其实也挺不舒服的看起来。

[libi]

redis驱动用scan 实现node注册很不合理 可以考虑一下 redis pub/sub 和 zset 配合

用scan实现避免不了（如果那个redis db存在大量key的问题 ）

之前的设计是通过节点心跳延续 key 的有效期来保证节点在线（节点不在线到期删除），确实不太合理。 这里可以改用更有效的方法：

1. pub/sub
2. zset 记录每个节点最后心跳时间，扫描在线节点时删除过期node

[libi]

@Casper-Mars @dxyinme
接口增加上下文 我看了下 kratos 新版本的实现： https://github.com/go-kratos/kratos/blob/520b321fe99bb2d79572539e91cbac448e3c9149/registry/registry.go#L20 https://github.com/go-kratos/kratos/blob/520b321fe99bb2d79572539e91cbac448e3c9149/transport/transport.go#L17

初步猜测设计思路为：如果该方法涉及IO导致执行时间不确定时需要添加上下文。可以从这个角度进行分析。

[Casper-Mars]

@Casper-Mars @dxyinme 接口增加上下文 我看了下 kratos 新版本的实现： https://github.com/go-kratos/kratos/blob/520b321fe99bb2d79572539e91cbac448e3c9149/registry/registry.go#L20 https://github.com/go-kratos/kratos/blob/520b321fe99bb2d79572539e91cbac448e3c9149/transport/transport.go#L17

初步猜测设计思路为：如果该方法涉及IO导致执行时间不确定时需要添加上下文。可以从这个角度进行分析。

嗯，其实是通过ctx做一个进程的生命周期管理，扩展到管理各个组件的生命周期。

[mintzhao]

有个疑问啊，既然是想让用户自己去实现driver，所以留了一个DriverV2的接口。

但是！！！

为什么还有一个 withOption(opt Option) (err error) 这样的方法？

是想让用户怎么去实现这个方法呢？我没明白。

类型无法实现 'driver.DriverV2'，因为类型有未导出的方法，并且是在不同的软件包中定义