-
Notifications
You must be signed in to change notification settings - Fork 492
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
Handle if collStats returns an int64 for size. #9159
Conversation
Add some testing for how we handle the bson.M result and allow it to be int or int64 and generate an error rather than panic if it isn't.
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
// 2billion megabytes is 2 petabytes, which is outside our range anyway. | ||
if asint64 > math.MaxInt32 { | ||
return math.MaxInt32, nil | ||
} |
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.
Is there ever a case that asint64 < 0
so that we also handle the min case or math.MinInt32
case?
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.
Well, if mongo returns a negative number of megabytes for the size of a collection we certainly have issues. I'm ok handling that case, just in a belt-and-suspenders type of defensive programming. But I'd probably cap the threshold at 0. Cause a negative size would do really weird things.
Treat them as errors rather than just passing them through.
|
#9169 ## Description of change Merge 2.3 into 2.4, bringing in: prdesc Merge pull request #9159 from jameinel/2.3-collection-size-1790626 ## QA steps See individual patches. ## Documentation changes None. ## Bug reference prdesc https://bugs.launchpad.net/juju/+bug/1790626
#9299 ## Description of change Merge 2.4 into develop, picking up these PRs: #9287 container networking, manually provisioned machines #9296 more robust cmr controller connectivity #9251 Fix the exporting of the series of the application when exporting a bundle #9293 bundle deploy panic #9285 tomb.Go shouldn't be called multiple times from a non-tomb managed goroutine #9263 Add back the state of each unit in a relation to goal state #9235 handle maas partitions in a storage constraint #9191 Add a default retry inside 'juju status' #9177 latest ec2 instance types #9180 fix space check before upgrade #9159 mongo collection size ## QA steps bootstrap smoke test
Description of change
In production, we ran into panic() because Mongo was returning an int64 for a size field instead of an int. We don't have full understanding of how it was happening, but this should at least cover the issue.
Add some testing for how we handle the bson.M result and allow it to be
int or int64 and generate an error rather than panic if it isn't.
QA steps
Tests added to cover the new code path which was factored out so that it could be directly testable. We don't have exact reproduction for how to get Mongo to behave in this way. One possibility is to just create a logs table that is very very large (>2GB?) and see if it panics without this change.
Documentation changes
None.
Bug reference
https://bugs.launchpad.net/juju/+bug/1790626