Merge pull request #33376 from luxas/fix_arm_atomics_2

Automatic merge from submit-queue

Move HighWaterMark to the top of the struct in order to fix arm, second time

ref: #33117

Sorry for not fixing everyone at once, but I seriously wasn't prepared for that quick LGTM 😄, so here's the other half.

@lavalamp 

> lgtm, but seriously, this is terrible, we probably have this bug all over. And what if someone embeds the etcdWatcher struct in something else not at the top? We need the compiler to enforce things like this, it just can't be done manually. Can you file or link a golang issue for this?

I totally agree! There isn't currently a way of programmatically detecting this unfortunately.
I guess @davecheney or @minux can explain better to you why it's so hard.

This is noted in https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/multi-platform.md as a corner case indeed.

@pwittrock This should be cherrypicked toghether with #33117
pull/6/head
Kubernetes Submit Queue 2016-09-23 12:05:09 -07:00 committed by GitHub
commit 331eb83585
3 changed files with 14 additions and 6 deletions

View File

@ -219,6 +219,8 @@ type Expectations interface {
// ControlleeExpectations track controllee creates/deletes. // ControlleeExpectations track controllee creates/deletes.
type ControlleeExpectations struct { type ControlleeExpectations struct {
// Important: Since these two int64 fields are using sync/atomic, they have to be at the top of the struct due to a bug on 32-bit platforms
// See: https://golang.org/pkg/sync/atomic/ for more information
add int64 add int64
del int64 del int64
key string key string

View File

@ -46,14 +46,17 @@ type Tunneler interface {
} }
type SSHTunneler struct { type SSHTunneler struct {
// Important: Since these two int64 fields are using sync/atomic, they have to be at the top of the struct due to a bug on 32-bit platforms
// See: https://golang.org/pkg/sync/atomic/ for more information
lastSync int64 // Seconds since Epoch
lastSSHKeySync int64 // Seconds since Epoch
SSHUser string SSHUser string
SSHKeyfile string SSHKeyfile string
InstallSSHKey InstallSSHKey InstallSSHKey InstallSSHKey
HealthCheckURL *url.URL HealthCheckURL *url.URL
tunnels *ssh.SSHTunnelList tunnels *ssh.SSHTunnelList
lastSync int64 // Seconds since Epoch
lastSSHKeySync int64 // Seconds since Epoch
lastSyncMetric prometheus.GaugeFunc lastSyncMetric prometheus.GaugeFunc
clock clock.Clock clock clock.Clock

View File

@ -130,6 +130,13 @@ func (i *indexedWatchers) terminateAll(objectType reflect.Type) {
// Cacher implements storage.Interface (although most of the calls are just // Cacher implements storage.Interface (although most of the calls are just
// delegated to the underlying storage). // delegated to the underlying storage).
type Cacher struct { type Cacher struct {
// HighWaterMarks for performance debugging.
// Important: Since HighWaterMark is using sync/atomic, it has to be at the top of the struct due to a bug on 32-bit platforms
// See: https://golang.org/pkg/sync/atomic/ for more information
incomingHWM HighWaterMark
// Incoming events that should be dispatched to watchers.
incoming chan watchCacheEvent
sync.RWMutex sync.RWMutex
// Before accessing the cacher's cache, wait for the ready to be ok. // Before accessing the cacher's cache, wait for the ready to be ok.
@ -164,10 +171,6 @@ type Cacher struct {
watcherIdx int watcherIdx int
watchers indexedWatchers watchers indexedWatchers
// Incoming events that should be dispatched to watchers.
incoming chan watchCacheEvent
incomingHWM HighWaterMark
// Handling graceful termination. // Handling graceful termination.
stopLock sync.RWMutex stopLock sync.RWMutex
stopped bool stopped bool