-
Notifications
You must be signed in to change notification settings - Fork 9.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
embed: fix blocking Close before gRPC server start #7848
Conversation
can we add a test for it? |
@@ -83,6 +83,10 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { | |||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come we don't close grpcServerC at here?
embed/etcd.go
Outdated
// errored before starting gRPC server for serveCtx.grpcServerC | ||
for _, sctx := range e.sctxs { | ||
close(sctx.grpcServerC) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a helper function to close sctxs? I see that this code is called twice already.
embed/etcd.go
Outdated
@@ -83,6 +83,10 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) { | |||
return | |||
} | |||
if e.sctxs, err = startClientListeners(cfg); err != nil { | |||
// errored before starting gRPC server for serveCtx.grpcServerC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just put these in the defer
?
defer func() {
if e == nil || err == nil {
return
}
if !serving {
for _, sctx := range e.sctxs { close(sctx.grpcServerC)
}
e.Close()
e = nil
}()
c791d13
to
9cdee03
Compare
PTLA. |
lgtm thanks |
lgtm |
If 'StartEtcd' returns before starting gRPC server (e.g. mismatch snapshot, misconfiguration), receiving from grpcServerC blocks forever. This patch just closes the channel to not block on grpcServerC, and proceeds to next stop operations in Close. This was masking the issues in etcd-io#7834 Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #7848 +/- ##
=========================================
+ Coverage 75.54% 76% +0.46%
=========================================
Files 332 332
Lines 26177 26183 +6
=========================================
+ Hits 19776 19901 +125
+ Misses 4970 4857 -113
+ Partials 1431 1425 -6
Continue to review full report at Codecov.
|
If 'StartEtcd' returns before starting gRPC server
(e.g. mismatch snapshot, misconfiguration),
receiving from grpcServerC blocks forever. This patch
just closes the channel to not block on grpcServerC,
and proceeds to next stop operations in Close.
This was masking the issues in #7834