From bd37867655c286e30b45e3369230fd758a4b9b2b Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Fri, 23 Mar 2018 14:35:33 -0700 Subject: [PATCH] introspection: Add inactive connections to output (#686) Tried to debug a stuck connection that was stuck in "startClose", but unfortunately the introspection output was missing the connection as it expects connections to be added as part of root peers, but peers only store references to active connections. Output inactive connections at the top level of the channel (which is the only thing that has references to inactive connections). --- introspection.go | 31 ++++++++++++++++++++----------- introspection_test.go | 40 +++++++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/introspection.go b/introspection.go index e75c2ed0..3dfe9f18 100644 --- a/introspection.go +++ b/introspection.go @@ -78,6 +78,10 @@ type RuntimeState struct { // Connections is the list of connection IDs in the channel Connections []uint32 ` json:"connections"` + // InactiveConnections is the connection state for connections that are not active, + // and hence are not reported as part of root peers. + InactiveConnections []ConnectionRuntimeState `json:"inactiveConnections"` + // OtherChannels is information about any other channels running in this process. OtherChannels map[string][]ChannelInfo `json:"otherChannels,omitEmpty"` @@ -206,24 +210,29 @@ func (ch *Channel) IntrospectState(opts *IntrospectionOptions) *RuntimeState { ch.mutable.RLock() numConns := len(ch.mutable.conns) + inactiveConns := make([]*Connection, 0, numConns) connIDs := make([]uint32, 0, numConns) - for id := range ch.mutable.conns { + for id, conn := range ch.mutable.conns { connIDs = append(connIDs, id) + if !conn.IsActive() { + inactiveConns = append(inactiveConns, conn) + } } ch.mutable.RUnlock() return &RuntimeState{ - ID: ch.chID, - CreatedStack: ch.createdStack, - LocalPeer: ch.PeerInfo(), - SubChannels: ch.subChannels.IntrospectState(opts), - RootPeers: ch.RootPeers().IntrospectState(opts), - Peers: ch.Peers().IntrospectList(opts), - NumConnections: numConns, - Connections: connIDs, - OtherChannels: ch.IntrospectOthers(opts), - RuntimeVersion: introspectRuntimeVersion(), + ID: ch.chID, + CreatedStack: ch.createdStack, + LocalPeer: ch.PeerInfo(), + SubChannels: ch.subChannels.IntrospectState(opts), + RootPeers: ch.RootPeers().IntrospectState(opts), + Peers: ch.Peers().IntrospectList(opts), + NumConnections: numConns, + Connections: connIDs, + InactiveConnections: getConnectionRuntimeState(inactiveConns, opts), + OtherChannels: ch.IntrospectOthers(opts), + RuntimeVersion: introspectRuntimeVersion(), } } diff --git a/introspection_test.go b/introspection_test.go index f6821c8c..77fe9a8c 100644 --- a/introspection_test.go +++ b/introspection_test.go @@ -68,16 +68,44 @@ func TestIntrospection(t *testing.T) { }) } -func TestIntrospectNumConnections(t *testing.T) { +func TestIntrospectClosedConn(t *testing.T) { // Disable the relay, since the relay does not maintain a 1:1 mapping betewen // incoming connections vs outgoing connections. opts := testutils.NewOpts().NoRelay() testutils.WithTestServer(t, opts, func(ts *testutils.TestServer) { + blockEcho := make(chan struct{}) + gotEcho := make(chan struct{}) + testutils.RegisterEcho(ts.Server(), func() { + close(gotEcho) + <-blockEcho + }) + ctx, cancel := NewContext(time.Second) defer cancel() assert.Equal(t, 0, ts.Server().IntrospectNumConnections(), "Expected no connection on new server") + // Make sure that a closed connection will reduce NumConnections. + client := ts.NewClient(nil) + require.NoError(t, client.Ping(ctx, ts.HostPort()), "Ping from new client failed") + assert.Equal(t, 1, ts.Server().IntrospectNumConnections(), "Number of connections expected to increase") + + go testutils.AssertEcho(t, client, ts.HostPort(), ts.ServiceName()) + + // The state will change to "closeStarted", but be blocked due to the blocked + // echo call. + <-gotEcho + client.Close() + + introspected := client.IntrospectState(nil) + assert.Len(t, introspected.Connections, 1, "Expected single connection due to blocked call") + assert.Len(t, introspected.InactiveConnections, 1, "Expected inactive connection due to blocked call") + + close(blockEcho) + require.True(t, testutils.WaitFor(100*time.Millisecond, func() bool { + return ts.Server().IntrospectNumConnections() == 0 + }), "Closed connection did not get removed, num connections is %v", ts.Server().IntrospectNumConnections()) + for i := 0; i < 10; i++ { client := ts.NewClient(nil) defer client.Close() @@ -86,15 +114,5 @@ func TestIntrospectNumConnections(t *testing.T) { assert.Equal(t, 1, client.IntrospectNumConnections(), "Client should have single connection") assert.Equal(t, i+1, ts.Server().IntrospectNumConnections(), "Incorrect number of server connections") } - - // Make sure that a closed connection will reduce NumConnections. - client := ts.NewClient(nil) - require.NoError(t, client.Ping(ctx, ts.HostPort()), "Ping from new client failed") - assert.Equal(t, 11, ts.Server().IntrospectNumConnections(), "Number of connections expected to increase") - - client.Close() - require.True(t, testutils.WaitFor(100*time.Millisecond, func() bool { - return ts.Server().IntrospectNumConnections() == 10 - }), "Closed connection did not get removed, num connections is %v", ts.Server().IntrospectNumConnections()) }) }