From df94f58462204ee843bcd0ea469990a4f904a9aa Mon Sep 17 00:00:00 2001 From: Jared Hulbert Date: Fri, 8 Jul 2016 11:05:41 -0700 Subject: [PATCH 1/3] raft: atomic access alignment The relevant structures are properly aligned, however, there is no comment highlighting the need to keep it aligned as is present elsewhere in the codebase. Adding note to keep alignment, in line with similar comments in the codebase. --- raft/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/raft/node.go b/raft/node.go index 0be7d0954..3f5f146f4 100644 --- a/raft/node.go +++ b/raft/node.go @@ -38,7 +38,7 @@ var ( // SoftState provides state that is useful for logging and debugging. // The state is volatile and does not need to be persisted to the WAL. type SoftState struct { - Lead uint64 + Lead uint64 // must use atomic operations to access; keep 64-bit aligned. RaftState StateType } From 90889ebc0f179f8ae506ed1925fc061edae18017 Mon Sep 17 00:00:00 2001 From: Jared Hulbert Date: Fri, 8 Jul 2016 11:13:53 -0700 Subject: [PATCH 2/3] raftpb: atomic access alignment The Entry struct has misaligned fields that are accessed atomically. The misalignment is caused by the EntryType enum which the Protocol Buffers spec forces to be a 32bit int. Moving the order of the fields without renumbering them in the .proto file seems to align the go structure without changing the wire format. --- raft/raftpb/raft.pb.go | 2 +- raft/raftpb/raft.proto | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/raft/raftpb/raft.pb.go b/raft/raftpb/raft.pb.go index cddc975b4..63a8d4cb9 100644 --- a/raft/raftpb/raft.pb.go +++ b/raft/raftpb/raft.pb.go @@ -189,9 +189,9 @@ func (x *ConfChangeType) UnmarshalJSON(data []byte) error { func (ConfChangeType) EnumDescriptor() ([]byte, []int) { return fileDescriptorRaft, []int{2} } type Entry struct { - Type EntryType `protobuf:"varint,1,opt,name=Type,json=type,enum=raftpb.EntryType" json:"Type"` Term uint64 `protobuf:"varint,2,opt,name=Term,json=term" json:"Term"` Index uint64 `protobuf:"varint,3,opt,name=Index,json=index" json:"Index"` + Type EntryType `protobuf:"varint,1,opt,name=Type,json=type,enum=raftpb.EntryType" json:"Type"` Data []byte `protobuf:"bytes,4,opt,name=Data,json=data" json:"Data,omitempty"` XXX_unrecognized []byte `json:"-"` } diff --git a/raft/raftpb/raft.proto b/raft/raftpb/raft.proto index 1948fc1e4..ae072963b 100644 --- a/raft/raftpb/raft.proto +++ b/raft/raftpb/raft.proto @@ -15,9 +15,9 @@ enum EntryType { } message Entry { + optional uint64 Term = 2 [(gogoproto.nullable) = false]; // must be 64-bit aligned for atomic operations + optional uint64 Index = 3 [(gogoproto.nullable) = false]; // must be 64-bit aligned for atomic operations optional EntryType Type = 1 [(gogoproto.nullable) = false]; - optional uint64 Term = 2 [(gogoproto.nullable) = false]; - optional uint64 Index = 3 [(gogoproto.nullable) = false]; optional bytes Data = 4; } From f78d4713ea195c494b427b93be1d6fc40ac169fb Mon Sep 17 00:00:00 2001 From: Jared Hulbert Date: Fri, 8 Jul 2016 11:20:47 -0700 Subject: [PATCH 3/3] etcdserver: atomic access alignment Most fields accessed with sync/atomic functions are 64bit aligned, but a couple are not. This makes comments out of date and therefore misleading. Affected fields reordered, comments scrubbed and updated. --- etcdserver/server.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/etcdserver/server.go b/etcdserver/server.go index d53584352..9f06a178d 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -153,13 +153,13 @@ type Server interface { // EtcdServer is the production implementation of the Server interface type EtcdServer struct { - // r and inflightSnapshots must be the first elements to keep 64-bit alignment for atomic - // access to fields - - // count the number of inflight snapshots. - // MUST use atomic operation to access this field. - inflightSnapshots int64 - Cfg *ServerConfig + // inflightSnapshots holds count the number of snapshots currently inflight. + inflightSnapshots int64 // must use atomic operations to access; keep 64-bit aligned. + appliedIndex uint64 // must use atomic operations to access; keep 64-bit aligned. + // consistIndex used to hold the offset of current executing entry + // It is initialized to 0 before executing any entry. + consistIndex consistentIndex // must use atomic operations to access; keep 64-bit aligned. + Cfg *ServerConfig readych chan struct{} r raftNode @@ -194,10 +194,6 @@ type EtcdServer struct { // compactor is used to auto-compact the KV. compactor *compactor.Periodic - // consistent index used to hold the offset of current executing entry - // It is initialized to 0 before executing any entry. - consistIndex consistentIndex - // peerRt used to send requests (version, lease) to peers. peerRt http.RoundTripper reqIDGen *idutil.Generator @@ -211,8 +207,6 @@ type EtcdServer struct { // wg is used to wait for the go routines that depends on the server state // to exit when stopping the server. wg sync.WaitGroup - - appliedIndex uint64 } // NewServer creates a new EtcdServer from the supplied configuration. The