{"action":"created","issue":{"url":"https://api.github.com/repos/go-redis/redis/issues/933","repository_url":"https://api.github.com/repos/go-redis/redis","labels_url":"https://api.github.com/repos/go-redis/redis/issues/933/labels{/name}","comments_url":"https://api.github.com/repos/go-redis/redis/issues/933/comments","events_url":"https://api.github.com/repos/go-redis/redis/issues/933/events","html_url":"https://github.com/go-redis/redis/issues/933","id":390243399,"node_id":"MDU6SXNzdWUzOTAyNDMzOTk=","number":933,"title":"How to gracefully close connections blocked on reading from Redis streams?","user":{"login":"RussellLuo","id":5109942,"node_id":"MDQ6VXNlcjUxMDk5NDI=","avatar_url":"https://avatars1.githubusercontent.com/u/5109942?v=4","gravatar_id":"","url":"https://api.github.com/users/RussellLuo","html_url":"https://github.com/RussellLuo","followers_url":"https://api.github.com/users/RussellLuo/followers","following_url":"https://api.github.com/users/RussellLuo/following{/other_user}","gists_url":"https://api.github.com/users/RussellLuo/gists{/gist_id}","starred_url":"https://api.github.com/users/RussellLuo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RussellLuo/subscriptions","organizations_url":"https://api.github.com/users/RussellLuo/orgs","repos_url":"https://api.github.com/users/RussellLuo/repos","events_url":"https://api.github.com/users/RussellLuo/events{/privacy}","received_events_url":"https://api.github.com/users/RussellLuo/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-12-12T14:12:04Z","updated_at":"2018-12-13T16:07:04Z","closed_at":null,"author_association":"NONE","body":"## Question\r\n\r\nPer the documentation of [XREAD](https://redis.io/commands/xread#blocking-for-data):\r\n\r\n> In order to avoid polling at a fixed or adaptive interval the command is able to block if it could not return any data, according to the specified streams and IDs, and automatically unblock once one of the requested keys accept data.\r\n\r\nIn other words, it's recommended to use the blocking mode while reading from Redis streams.\r\n\r\nWhen using go-redis, we may write the following code:\r\n\r\n```go\r\npackage main\r\n\r\nimport (\r\n\t\"log\"\r\n\t\"os\"\r\n\t\"os/signal\"\r\n\t\"sync\"\r\n\r\n\t\"github.com/go-redis/redis\"\r\n)\r\n\r\nfunc main() {\r\n\tclient := redis.NewClient(&redis.Options{\r\n\t\tAddr: \"localhost:6379\",\r\n\t\t// Only need 1 connection for reading.\r\n\t\tPoolSize: 1,\r\n\t})\r\n\r\n\tvar waitGroup sync.WaitGroup\r\n\twaitGroup.Add(1)\r\n\tgo func() {\r\n\t\tdefer waitGroup.Done()\r\n\r\n\t\tlastID := \"0-0\"\r\n\r\n\t\tfor {\r\n\t\t\tstreams, err := client.XRead(&redis.XReadArgs{\r\n\t\t\t\tStreams: []string{\"mystream\", lastID},\r\n\t\t\t\tBlock:   0, // Wait for new messages without a timeout.\r\n\t\t\t}).Result()\r\n\t\t\tif err != nil {\r\n\t\t\t\tlog.Printf(\"err: %+v\\n\", err)\r\n\t\t\t\treturn\r\n\t\t\t}\r\n\r\n\t\t\tlog.Printf(\"received streams: %+v\\n\", streams)\r\n\r\n\t\t\tmessages := streams[0].Messages\r\n\t\t\tlastID = messages[len(messages)-1].ID\r\n\t\t}\r\n\t}()\r\n\r\n\tc := make(chan os.Signal, 1)\r\n\tsignal.Notify(c, os.Interrupt)\r\n\r\n\tselect {\r\n\tcase <-c:\r\n\t\t// Question: how to gracefully close the blocking connection here?\r\n\t\tclient.Close()\r\n\r\n\t\twaitGroup.Wait()\r\n\t}\r\n}\r\n```\r\n\r\nThen there is a question: **how to gracefully close the blocking connection?**\r\n\r\n## The problem of client.Close()\r\n\r\nIn the example code, If we use `client.Close()` to close the connection, it will cause an error log:\r\n\r\n```\r\nerr: read tcp 127.0.0.1:55520->127.0.0.1:6379: use of closed network connection\r\n```\r\n\r\nWhat happened behind `client.Close()` is as follows:\r\n\r\n1. `client.Close()` will [close the connection pool](https://github.com/go-redis/redis/blob/11cf9400d58a23fe233f2544ed4cd55b7a373f89/redis.go#L244)\r\n2. then the only 1 connection in the pool will be actually [closed](https://github.com/go-redis/redis/blob/11cf9400d58a23fe233f2544ed4cd55b7a373f89/internal/pool/pool.go#L333)\r\n3. then `client.XRead()`, which is blocking on [cn.WithReader()](https://github.com/go-redis/redis/blob/11cf9400d58a23fe233f2544ed4cd55b7a373f89/redis.go#L204), will be unblocked and returns an error \"use of closed network connection\"\r\n4. then `c.releaseConn()` will be called, and err is judged as [a bad one](https://github.com/go-redis/redis/blob/11cf9400d58a23fe233f2544ed4cd55b7a373f89/redis.go#L104), which will finally **closes the connection again**\r\n\r\nThe real problem is:\r\n\r\n- when the connection is closed at step 2, the file descriptor is also released\r\n- when the connection is closed again at step 4, the original file descriptor may have been used by another valid connection, which will be closed mistakenly.\r\n\r\nThere was [a discussion about the same issue](https://stackoverflow.com/a/27790293/963379) on StackOverflow.\r\n\r\n## Possible solution\r\n\r\nUse [TCPConn.CloseRead](https://golang.org/pkg/net/#TCPConn.CloseRead) or [UnixConn.CloseRead](https://golang.org/pkg/net/#UnixConn.CloseRead), which are the Golang's equivalents of Unix's `shutdown()`.\r\n\r\nThen we need to add a function`CloseRead()` into [redis.Client](https://github.com/go-redis/redis/blob/11cf9400d58a23fe233f2544ed4cd55b7a373f89/redis.go#L388).\r\n\r\nAny other thoughts?"},"comment":{"url":"https://api.github.com/repos/go-redis/redis/issues/comments/447023884","html_url":"https://github.com/go-redis/redis/issues/933#issuecomment-447023884","issue_url":"https://api.github.com/repos/go-redis/redis/issues/933","id":447023884,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NzAyMzg4NA==","user":{"login":"RussellLuo","id":5109942,"node_id":"MDQ6VXNlcjUxMDk5NDI=","avatar_url":"https://avatars1.githubusercontent.com/u/5109942?v=4","gravatar_id":"","url":"https://api.github.com/users/RussellLuo","html_url":"https://github.com/RussellLuo","followers_url":"https://api.github.com/users/RussellLuo/followers","following_url":"https://api.github.com/users/RussellLuo/following{/other_user}","gists_url":"https://api.github.com/users/RussellLuo/gists{/gist_id}","starred_url":"https://api.github.com/users/RussellLuo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RussellLuo/subscriptions","organizations_url":"https://api.github.com/users/RussellLuo/orgs","repos_url":"https://api.github.com/users/RussellLuo/repos","events_url":"https://api.github.com/users/RussellLuo/events{/privacy}","received_events_url":"https://api.github.com/users/RussellLuo/received_events","type":"User","site_admin":false},"created_at":"2018-12-13T16:07:04Z","updated_at":"2018-12-13T16:07:04Z","author_association":"NONE","body":"> Do you suggest we use CloseRead when closing the connection pool?\r\n\r\nIn the example scenario I mentioned above, if we can avoid closing the connection twice, I think it's ok to use the existing client.Close().\r\n\r\nThere is another scenario, in theory, where a connection may be shared by two processes (e.g. both are blocked on reading from the same connection). Then client.CloseRead() can unblock both processes, while client.Close() can only unblock the process where it is called within.\r\n\r\nThat being said, I have little experience with the latter scenario in Golang, so I don't know if it's a practical problem.\r\n\r\n> The alternative is to add some guard against closing the connection twice which should be trivial and conceptually simpler.\r\n\r\nYeah, for the example scenario, I think the alternative solution is good. Maybe we can check the error string \"use of closed network connection\" (which is an internal error [ErrNetClosing](https://github.com/golang/go/blob/571d93e977862f91bb153f0b98937ca655505fcd/src/internal/poll/fd.go#L18)) before closing the connection?"}}
{"actor":{"display_login":"RussellLuo"}}