Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: gRPC gateway not support cancel-request #12333

Closed
Yiyiyimu opened this issue Sep 25, 2020 · 6 comments
Closed

bug: gRPC gateway not support cancel-request #12333

Yiyiyimu opened this issue Sep 25, 2020 · 6 comments

Comments

@Yiyiyimu
Copy link

Hi community,

I'm developing etcd client for lua/openresty, and since it does not support grpc, we're using grpc gateway to use RESTful API. I know it would be hard to reproduce the problem, since it's in another language, but I try to make myself clear.

  1. For example, if I would like to create watch request, I would do
res, err = http_cli:request({
        method  = "POST",
        path    = /v3/watch,
        body    = {"create_request":{"watch_id":100,"key":"L3Rlc3Q="}}
    })
  1. And here is the problem, when I set body to create_request or progress_request, there would be normal responce.
    But when I set body to cancel_request, the request would always return timeout, the same when I set body to foo_request. So I guess cancel_request might not be supported in gRPC gateway.
    Also I tried to find other etcd client in non-grpc-supported language, but it seems none of them support this feature that cancel the watch. I would be also grateful if there are some reference to implement this feature.
@Yiyiyimu
Copy link
Author

I tried to directly close the http connection that watch inside, it does close watch where no more change would be get, but the connection keeps there so when we do netstat -an | grep 2379 we could find 1000+ of connection, while it should be ~100

@ptabor
Copy link
Contributor

ptabor commented Sep 25, 2020

I think I've seen it in integration tests as well.

I suspect it might be related to this comment:

// Currently, client contexts are overwritten with "valCtx" that never closes.

Proxy cannot close underlying watch request to etcd-server without closing the whole underlying connection, so it also does not support it on its surface.
In PR #12319 I decouple ctx of Watch server from context of underlying connection, but its only beginning of the story.

@Yiyiyimu
Copy link
Author

It's so great you're working on it! Hope this problem could get fixed soon.

@ptabor
Copy link
Contributor

ptabor commented Sep 25, 2020

I must have been not precise: I find it important, but I don't claim ownership of this problem (yet).

@Yiyiyimu
Copy link
Author

I must have been not precise: I find it important, but I don't claim ownership of this problem (yet).

No problem I mean it would be better if it could get fixed, but we'll also try to fix it on our side since it is confirmed the etcd native way is not supported for now so we need to find our own way to deal with it. Anyway thank you for your explanation!

@Yiyiyimu
Copy link
Author

fixed on our side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants