Skip to content

Commit

Permalink
[dbnode] Fix integration code under race with TChannel channel option…
Browse files Browse the repository at this point in the history
…s mutation issue (#2958)
  • Loading branch information
robskillington authored Nov 30, 2020
1 parent 9036832 commit 060d253
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 10 deletions.
11 changes: 9 additions & 2 deletions src/dbnode/client/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 8 additions & 6 deletions src/dbnode/network/server/tchannelthrift/cluster/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
10 changes: 8 additions & 2 deletions src/dbnode/network/server/tchannelthrift/node/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 060d253

Please sign in to comment.