From fa87aa69e7b3bc3b701818a6cdff9b04733832a2 Mon Sep 17 00:00:00 2001
From: Rob Skillington <rob@chronosphere.io>
Date: Mon, 30 Nov 2020 03:05:34 -0500
Subject: [PATCH] [dbnode] Fix integration code under race with TChannel
 channel options mutation issue

---
 src/dbnode/client/options.go                       | 11 +++++++++--
 .../server/tchannelthrift/cluster/server.go        | 14 ++++++++------
 .../network/server/tchannelthrift/node/server.go   | 10 ++++++++--
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/dbnode/client/options.go b/src/dbnode/client/options.go
index c8e350b199..9ec1613fd8 100644
--- a/src/dbnode/client/options.go
+++ b/src/dbnode/client/options.go
@@ -318,9 +318,16 @@ func NewOptionsForAsyncClusters(opts Options, topoInits []topology.Initializer,
 }
 
 func defaultNewConnectionFn(
-	channelName string, address string, opts Options,
+	channelName string, address string, clientOpts Options,
 ) (xresource.SimpleCloser, rpc.TChanNode, error) {
-	channel, err := tchannel.NewChannel(channelName, opts.ChannelOptions())
+	// NB(r): Keep ref to a local channel options since it's actually modified
+	// by TChannel itself to set defaults.
+	var opts *tchannel.ChannelOptions
+	if chanOpts := clientOpts.ChannelOptions(); chanOpts != nil {
+		immutableOpts := *chanOpts
+		opts = &immutableOpts
+	}
+	channel, err := tchannel.NewChannel(channelName, opts)
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/src/dbnode/network/server/tchannelthrift/cluster/server.go b/src/dbnode/network/server/tchannelthrift/cluster/server.go
index 5b05af00e8..746fe9aea0 100644
--- a/src/dbnode/network/server/tchannelthrift/cluster/server.go
+++ b/src/dbnode/network/server/tchannelthrift/cluster/server.go
@@ -50,11 +50,6 @@ func NewServer(
 	contextPool context.Pool,
 	opts *tchannel.ChannelOptions,
 ) ns.NetworkService {
-	// Make the opts immutable on the way in
-	if opts != nil {
-		immutableOpts := *opts
-		opts = &immutableOpts
-	}
 	return &server{
 		address:     address,
 		client:      client,
@@ -64,7 +59,14 @@ func NewServer(
 }
 
 func (s *server) ListenAndServe() (ns.Close, error) {
-	channel, err := tchannel.NewChannel(ChannelName, s.opts)
+	// NB(r): Keep ref to a local channel options since it's actually modified
+	// by TChannel itself to set defaults.
+	var opts *tchannel.ChannelOptions
+	if s.opts != nil {
+		immutableOpts := *s.opts
+		opts = &immutableOpts
+	}
+	channel, err := tchannel.NewChannel(ChannelName, opts)
 	if err != nil {
 		return nil, err
 	}
diff --git a/src/dbnode/network/server/tchannelthrift/node/server.go b/src/dbnode/network/server/tchannelthrift/node/server.go
index dfe44af0a2..91f560fa92 100644
--- a/src/dbnode/network/server/tchannelthrift/node/server.go
+++ b/src/dbnode/network/server/tchannelthrift/node/server.go
@@ -52,8 +52,14 @@ func NewServer(
 }
 
 func (s *server) ListenAndServe() (ns.Close, error) {
-	chanOpts := s.opts.ChannelOptions()
-	channel, err := tchannel.NewChannel(channel.ChannelName, chanOpts)
+	// NB(r): Keep ref to a local channel options since it's actually modified
+	// by TChannel itself to set defaults.
+	var opts *tchannel.ChannelOptions
+	if chanOpts := s.opts.ChannelOptions(); chanOpts != nil {
+		immutableOpts := *chanOpts
+		opts = &immutableOpts
+	}
+	channel, err := tchannel.NewChannel(channel.ChannelName, opts)
 	if err != nil {
 		return nil, err
 	}