k3s-io / kine

Run Kubernetes on MySQL, Postgres, sqlite, dqlite, not etcd.
Apache License 2.0
1.54k stars 231 forks source link

Question: the reason for executing cancel multiple times in watch handle function #198

Closed wangyanphp closed 1 year ago

wangyanphp commented 1 year ago

i noticed that will execute cancel function multiple times in

  1. https://github.com/k3s-io/kine/blob/master/pkg/server/watch.go#L91 and
  2. https://github.com/k3s-io/kine/blob/master/pkg/server/watch.go#L95

that will cause sending canceled watch response to etcd client multiple times:https://github.com/k3s-io/kine/blob/master/pkg/server/watch.go#L135 image

when etcd client received multiple canceled response, it will panic because of close a closed channel https://github.com/etcd-io/etcd/blob/main/client/v3/watch.go#L637 image

so my question is why kine doesn't send canceled response only once?

brandond commented 1 year ago

Is this a theoretical problem, or something you've actually run into? How are you able to reproduce this? The only reason the Cancel on line 91 would ever be called is if for some reason the watch event cannot be sent to the client. If that fails, it is highly unlikely that the client would still be connected to see the the second cancel, which I suspect is why we never see this when kine is used as a Kubernetes datastore.

wangyanphp commented 1 year ago

it's a theoretical problem. because kine could do it easily that send canceled response only once for watch id, but kine doesn't do it. So I'm curious if there's some details or design here that I didn't notice

brandond commented 1 year ago

I think that while it looks like it could happen just from reading through the code, in reality it does not because the client disconnects after receiving the first cancel. Feel free to reopen this if you find a way to actually reproduce an issue.