-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Try reconnecting Mongo on EOF #3269
Conversation
plugins/database/mongodb/mongodb.go
Outdated
if err != nil { | ||
switch err { | ||
case nil: | ||
case io.EOF || strings.Contains(err.Error(), "EOF"): |
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.
Did you mean to compare err
with io.EOF
? I wonder if this will compile since io.EOF
does not return a bool.
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.
io.EOF
is the following:
var EOF = errors.New("EOF")
As such I am indeed trying to compare it directly, but in case it's simply another actual error type that contains EOF in its string, I'm also doing the string check.
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.
Since its a error object and not a bool, I suspect if this will compile.
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.
Should it be a comma instead of a ||
?
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.
So you're totally right about that, which is really amusing because the original reporter swore that he'd tried this branch and it didn't fix his problem. I'm suspicious.
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.
Looks good other then that one comment by Vishal
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.
LGTM!
* oss/master: Plugin Version Update (#3275) Lazy-load plugin mounts (#3255) changelog++ changelog++ Add pki/root/sign-self-issued. (#3274) Travis, be happier please changelog++ Change auth helper interface to api.Secret. (#3263) changelog++ Try reconnecting Mongo on EOF (#3269) Don't append a trailing slash to the request path if it doesn't actually help find something (#3271) changelog++ Use TypeDurationSecond for TTL values in PKI. (#3270) changelog++ changelog++ Use net.SplitHostPort on Consul address (#3268) Normalize plugin_name option for mount and enable-auth (#3202) Updating Okta lib for credential backend (#3245) Explicitly mention that aws/aws-ec2 were unified under aws.
This was an attempt to fix #2973. It appears not to work, for reasons that are not at all apparent (this checks for the error string containing "EOF" which is the exact error string printed in the logs), but it also doesn't seem like a bad thing to include anyways.