Skip to content
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

Fix uint64 overflow in info/v2/conversion.go. #1333

Merged
merged 1 commit into from
Jun 16, 2016

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Jun 16, 2016

For kubernetes/kubernetes#27194.

The current implementation of InstCpuStats will sometimes cause data overflow. (See detail)

Without this change, when valueDelta is 18949414524, timeDeltaNs is 17978530662, it will return 27959484, with this change it will return 1054002403 correctly.

I also removed the dead code instCpuStats.

@dchen1107
/cc @timstclair @vishh

@k8s-bot
Copy link
Collaborator

k8s-bot commented Jun 16, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@dchen1107
Copy link
Collaborator

@k8s-bot test this

@dchen1107
Copy link
Collaborator

LGTM

@Random-Liu
Copy link
Member Author

Random-Liu commented Jun 16, 2016

@dchen1107 Just did a rebase just now. The code base is changing so fast~ :)

@k8s-bot
Copy link
Collaborator

k8s-bot commented Jun 16, 2016

Jenkins GCE e2e

Build/test passed for commit 8c362dc.

@dchen1107 dchen1107 added this to the Kubernetes v1.3 milestone Jun 16, 2016
@dchen1107
Copy link
Collaborator

LGTM

@dchen1107 dchen1107 merged commit 1250a1d into google:master Jun 16, 2016
@Random-Liu Random-Liu deleted the fix-data-overflow branch June 16, 2016 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants