Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-41585

[N1FTY] Close old gRPC ClientConns on refresh

    XMLWordPrintable

    Details

    • Triage:
      Untriaged
    • Story Points:
      1
    • Is this a Regression?:
      Unknown

      Description

      As recommended here, it is advised to call close on gRPC.ClientConn whenever the application intends to not use it anymore ..

      https://godoc.org/google.golang.org/grpc#Dial

      To ensure resources are not leaked due to the stream returned, one of the following actions must be performed:
       
      1. Call Close on the ClientConn.
      2. Cancel the context provided.
      3. Call RecvMsg until a non-nil error is returned. A protobuf-generated client-streaming RPC, for instance, might use the helper function CloseAndRecv (note that CloseSend does not Recv, therefore is not guaranteed to release all resources).
      4. Receive a non-nil, non-io.EOF error from Header or SendMsg. 

      On Refresh(..), N1FTY sets up a new set of ClientConns' to FTS servers but does not close the dereferenced ClientConns. This will need fixing.

      I used this command to track the number of open gRPC connections ..

      netstat -an | grep ESTABLISHED | grep -w 9201 | wc -l 

      With some code instrumenting by forcing indexer refreshes continuously - I'm able to see that the gRPC connections count (available from the above command) grow over time. The fix for this issue ensures that the number remains constant despite continuous refresh.

        Attachments

          Issue Links

          For Gerrit Dashboard: MB-41585
          # Subject Branch Project Status CR V

            Activity

            Show
            abhinav Abhinav Dangeti added a comment - http://review.couchbase.org/c/n1fty/+/136656
            Hide
            build-team Couchbase Build Team added a comment -

            Build couchbase-server-7.0.0-3180 contains n1fty commit 0f25d7a with commit message:
            MB-41585: Tear down old gRPC ClientConns on indexer.Refresh

            Show
            build-team Couchbase Build Team added a comment - Build couchbase-server-7.0.0-3180 contains n1fty commit 0f25d7a with commit message: MB-41585 : Tear down old gRPC ClientConns on indexer.Refresh
            Hide
            build-team Couchbase Build Team added a comment -

            Build couchbase-server-7.0.0-3187 contains n1fty commit 03d00b0 with commit message:
            MB-41585: [BP] Tear down old gRPC ClientConns on indexer.Refresh

            Show
            build-team Couchbase Build Team added a comment - Build couchbase-server-7.0.0-3187 contains n1fty commit 03d00b0 with commit message: MB-41585 : [BP] Tear down old gRPC ClientConns on indexer.Refresh
            Hide
            build-team Couchbase Build Team added a comment -

            Build couchbase-server-6.6.1-9072 contains n1fty commit 03d00b0 with commit message:
            MB-41585: [BP] Tear down old gRPC ClientConns on indexer.Refresh

            Show
            build-team Couchbase Build Team added a comment - Build couchbase-server-6.6.1-9072 contains n1fty commit 03d00b0 with commit message: MB-41585 : [BP] Tear down old gRPC ClientConns on indexer.Refresh
            Hide
            wayne Wayne Siu added a comment -

            Build couchbase-server-6.5.1-6305 contains n1fty commit b15b7a4 with commit message:
            MB-41603: [BP] Tear down old gRPC ClientConns on indexer.Refresh

            Show
            wayne Wayne Siu added a comment - Build couchbase-server-6.5.1-6305 contains n1fty commit b15b7a4 with commit message: MB-41603: [BP] Tear down old gRPC ClientConns on indexer.Refresh
            Hide
            girish.benakappa Girish Benakappa added a comment -

            Verified in 6.5.1-6306, 6.6.1 build 9081, 7.0.0 build 3208 based on testcases: https://docs.google.com/spreadsheets/d/1xXThDbJXwC8ibUTqlzTsRcUrl8DOrmYKUKCs7Vast_Q/edit#gid=0

            Show
            girish.benakappa Girish Benakappa added a comment - Verified in 6.5.1-6306, 6.6.1 build 9081, 7.0.0 build 3208 based on testcases: https://docs.google.com/spreadsheets/d/1xXThDbJXwC8ibUTqlzTsRcUrl8DOrmYKUKCs7Vast_Q/edit#gid=0

              People

              Assignee:
              abhinav Abhinav Dangeti
              Reporter:
              abhinav Abhinav Dangeti
              Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes

                    PagerDuty