Skip to content

Fix for Y2038 problem #15934

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix for Y2038 problem #15934

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/autoscaler/aggregation/bucketing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,10 @@ func (t *TimedFloat64Buckets) WindowAverage(now time.Time) float64 {
// operations to find the index in the bucket list.
// bucketMutex needs to be held.
func (t *TimedFloat64Buckets) timeToIndex(tm time.Time) int {
// I don't think this run in 2038 :-)
// Use int64 to avoid Y2038 problem, then safely convertAdd commentMore actions
// NB: we need to divide by granularity, since it's a compressing mapping
// to buckets.
return int(tm.Unix()) / int(t.granularity.Seconds())
return int(tm.Unix() / int64(t.granularity.Seconds()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically golang when compiling apps on 64bit host int64=int so technically this isn't an issue here.

Even with the change if were compiling on a 32bit machine you'd be converting int64 => int32 without any bounds check.

I wonder if 'modulo' is the better operation here or we should just simply fail the build on 32bit systems

}

// Record adds a value with an associated time to the correct bucket.
Expand Down
Loading