Skip to content

Commit 1bd2a09

Browse files
committed
server: always use dropDRPCHeaderListener with drpc server
Previously, the DRPC listener was conditionally created based on the DRPC setting. When DRPC was disabled, we used a noopListener that would block on Accept() until closed, effectively rejecting all DRPC connections. In #153503 we changed the DRPC server to always run regardless of the setting, but missed updating the corresponding listener logic. This commit completes that change by always using the dropDRPCHeaderListener to properly handle incoming DRPC connections. Epic: none Release note: none
1 parent 1e2b120 commit 1bd2a09

File tree

2 files changed

+4
-37
lines changed

2 files changed

+4
-37
lines changed

pkg/server/start_drpc_listener.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,23 +44,3 @@ func (ln *dropDRPCHeaderListener) Close() error {
4444
func (ln *dropDRPCHeaderListener) Addr() net.Addr {
4545
return ln.wrapped.Addr()
4646
}
47-
48-
type noopListener struct{ done chan struct{} }
49-
50-
func (l *noopListener) Accept() (net.Conn, error) {
51-
<-l.done
52-
return nil, net.ErrClosed
53-
}
54-
55-
func (l *noopListener) Close() error {
56-
if l.done == nil {
57-
return nil
58-
}
59-
close(l.done)
60-
l.done = nil
61-
return nil
62-
}
63-
64-
func (l *noopListener) Addr() net.Addr {
65-
return nil
66-
}

pkg/server/start_listen.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414

1515
"github.com/cockroachdb/cmux"
1616
"github.com/cockroachdb/cockroach/pkg/rpc"
17-
"github.com/cockroachdb/cockroach/pkg/rpc/rpcbase"
1817
"github.com/cockroachdb/cockroach/pkg/sql/pgwire"
1918
"github.com/cockroachdb/cockroach/pkg/util/log"
2019
"github.com/cockroachdb/cockroach/pkg/util/netutil"
@@ -135,22 +134,10 @@ func startListenRPCAndSQL(
135134
}
136135
}
137136

138-
// Host drpc only if it's _possible_ to turn it on (this requires a test build
139-
// or env var). If the setting _is_ on, then it was overridden in testing and
140-
// we want to host the server too.
141-
hostDRPC := rpcbase.ExperimentalDRPCEnabled.Validate(nil /* not used */, true) == nil ||
142-
rpcbase.ExperimentalDRPCEnabled.Get(&cfg.Settings.SV)
143-
144-
// If we're not hosting drpc, make a listener that never accepts anything.
145-
// We will start the dRPC server all the same; it barely consumes any
146-
// resources.
147-
var drpcL net.Listener = &noopListener{make(chan struct{})}
148-
if hostDRPC {
149-
// Throw away the header before passing the conn to the drpc server. This
150-
// would not be required explicitly if we used `drpcmigrate.ListenMux` but
151-
// cmux keeps the prefix.
152-
drpcL = &dropDRPCHeaderListener{wrapped: m.Match(drpcMatcher)}
153-
}
137+
// dropDRPCHeaderListener discards the header before passing the connection
138+
// to the drpc server. This would not be required if we used
139+
// `drpcmigrate.ListenMux`, but cmux keeps the prefix.
140+
var drpcL net.Listener = &dropDRPCHeaderListener{wrapped: m.Match(drpcMatcher)}
154141

155142
grpcL := m.Match(cmux.Any())
156143
if serverTestKnobs, ok := cfg.TestingKnobs.Server.(*TestingKnobs); ok {

0 commit comments

Comments
 (0)