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

[TA4284] fix(rpc): close connection at the client side #168

Merged
merged 7 commits into from
Dec 18, 2018

Conversation

utkarshmani1997
Copy link
Contributor

@utkarshmani1997 utkarshmani1997 commented Dec 6, 2018

Controller initiates a new connection upon replica registration,
this might create some problem due to concurrent goroutines, as some
of the goroutine may not have been exited. To ensure graceful close
of the connection, i have added CloseRead and CloseWrite in both
replica and controller.

Earlier we were not logging error in replica in such scenario, an error will be logged after this fix.

Signed-off-by: Utkarsh Mani Tripathi utkarsh.tripathi@mayadata.io

@utkarshmani1997 utkarshmani1997 force-pushed the US3967-shutdown-fd branch 3 times, most recently from 421e3f1 to 63c31a3 Compare December 7, 2018 04:10
rpc/client.go Outdated Show resolved Hide resolved
@vishnuitta
Copy link
Contributor

Please try code as below to make sure GO routines are exited:

diff --git a/rpc/client.go b/rpc/client.go
index 6fc1a75..6fb8ce8 100644
--- a/rpc/client.go
+++ b/rpc/client.go
@@ -70,6 +70,8 @@ func NewClient(conn net.Conn, closeChan chan struct{}) *Client {
                messages:  map[uint32]*Message{},
                closeChan: closeChan,
        }
+       c.write_exited = false
+       c.read_exited = false
        go c.loop()
        go c.write()
        go c.read()
@@ -221,6 +223,17 @@ func (c *Client) Close() {
 
 func (c *Client) loop() {
        defer close(c.send)
+       defer func() {
+               c.wire.CloseRead()
+               c.wire.CloseWrite()
+               for {
+                       if (c.read_exited == true) && (c.write_exited == true) {
+                               break
+                       }
+                       time.Sleep()
+               }
+               c.wire.Close()
+       }
 
        for {
                select {
@@ -317,6 +330,7 @@ func (c *Client) write() {
                }
        }
        logrus.Infof("Exiting rpc writer, RemoteAddr:%v", c.peerAddr)
+       c.write_exited = true
 }
 
 func (c *Client) read() {
@@ -330,4 +344,5 @@ func (c *Client) read() {
                c.responses <- msg
        }
        logrus.Infof("Exiting rpc reader, RemoteAddr:%v", c.peerAddr)
+       c.read_exited = true
 }

Controller initiates a new connection upon replica registration,
this might create some problem due to concurrent goroutines, as some
of the goroutine may not have been exited. To ensure graceful close
of the connection, i have added CloseRead and CloseWrite in both
replica and controller.

Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
@utkarshmani1997 utkarshmani1997 force-pushed the US3967-shutdown-fd branch 3 times, most recently from a9c0c71 to ded3d91 Compare December 11, 2018 13:11
Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
rpc/server.go Outdated Show resolved Hide resolved
Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
rpc/server.go Outdated Show resolved Hide resolved
rpc/server.go Outdated Show resolved Hide resolved
rpc/server.go Outdated Show resolved Hide resolved
rpc/wire.go Outdated Show resolved Hide resolved
rpc/wire.go Outdated Show resolved Hide resolved
Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
Copy link
Contributor

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes are good

@vishnuitta vishnuitta merged commit 2cd5049 into openebs-archive:master Dec 18, 2018
utkarshmani1997 added a commit to utkarshmani1997/jiva that referenced this pull request Jan 29, 2019
…or pingTimeout (openebs-archive#168)

Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
utkarshmani1997 added a commit to utkarshmani1997/jiva that referenced this pull request Jan 29, 2019
…or pingTimeout (openebs-archive#168)

Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
vishnuitta pushed a commit that referenced this pull request Jan 29, 2019
* [TA4284] fix(rpc): close connection at the client & server on errors or pingTimeout (#168)

Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>

* [TA4759] fix(replica): handle case of disk-detach

- Issue Ref: openebs/openebs#1387
- Log and handle the case of disk-detach cases (input/output error)
  in case it's `EIO` error fatal the replica.

Note:
- What happens to running replica ?
  Replica will be fataled.
- What will happen upon replica restart after fatal?
  Replica will always be fataled until disk is attached
- What will happen to controller ?
  It will remove the fataled replica, and will continue to serve
  IO's if quorum is met.

Signed-off-by: Utkarsh Mani Tripathi <utkarsh.tripathi@mayadata.io>
@utkarshmani1997 utkarshmani1997 deleted the US3967-shutdown-fd branch July 16, 2019 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants