Skip to content

Commit

Permalink
etcdserver, embed, integration: don't use pointer for ServerConfig
Browse files Browse the repository at this point in the history 
ServerConfig is owned by etdcserver and unshared, so don't pass or store by
pointer. Also removes duplicated field 'snapCount'.
  • Loading branch information
@heyitsanthony
heyitsanthony committed May 10, 2017
1 parent 066062a commit d433067
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 40 deletions.
2 changes: 1 addition & 1 deletion embed/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
}
}

srvcfg := &etcdserver.ServerConfig{
srvcfg := etcdserver.ServerConfig{
Name: cfg.Name,
ClientURLs: cfg.ACUrls,
PeerURLs: cfg.APUrls,
Expand Down
6 changes: 3 additions & 3 deletions etcdserver/raft.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func advanceTicksForElection(n raft.Node, electionTicks int) {
}
}

func startNode(cfg *ServerConfig, cl *membership.RaftCluster, ids []types.ID) (id types.ID, n raft.Node, s *raft.MemoryStorage, w *wal.WAL) {
func startNode(cfg ServerConfig, cl *membership.RaftCluster, ids []types.ID) (id types.ID, n raft.Node, s *raft.MemoryStorage, w *wal.WAL) {
var err error
member := cl.MemberByName(cfg.Name)
metadata := pbutil.MustMarshal(
Expand Down Expand Up @@ -419,7 +419,7 @@ func startNode(cfg *ServerConfig, cl *membership.RaftCluster, ids []types.ID) (i
return
}

func restartNode(cfg *ServerConfig, snapshot *raftpb.Snapshot) (types.ID, *membership.RaftCluster, raft.Node, *raft.MemoryStorage, *wal.WAL) {
func restartNode(cfg ServerConfig, snapshot *raftpb.Snapshot) (types.ID, *membership.RaftCluster, raft.Node, *raft.MemoryStorage, *wal.WAL) {
var walsnap walpb.Snapshot
if snapshot != nil {
walsnap.Index, walsnap.Term = snapshot.Metadata.Index, snapshot.Metadata.Term
Expand Down Expand Up @@ -453,7 +453,7 @@ func restartNode(cfg *ServerConfig, snapshot *raftpb.Snapshot) (types.ID, *membe
return id, cl, n, s, w
}

func restartAsStandaloneNode(cfg *ServerConfig, snapshot *raftpb.Snapshot) (types.ID, *membership.RaftCluster, raft.Node, *raft.MemoryStorage, *wal.WAL) {
func restartAsStandaloneNode(cfg ServerConfig, snapshot *raftpb.Snapshot) (types.ID, *membership.RaftCluster, raft.Node, *raft.MemoryStorage, *wal.WAL) {
var walsnap walpb.Snapshot
if snapshot != nil {
walsnap.Index, walsnap.Term = snapshot.Metadata.Index, snapshot.Metadata.Term
Expand Down
23 changes: 10 additions & 13 deletions etcdserver/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,10 @@ type EtcdServer struct {
// 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
r raftNode // uses 64-bit atomics; keep 64-bit aligned.

readych chan struct{}
r raftNode

snapCount uint64
Cfg ServerConfig

w wait.Wait

Expand Down Expand Up @@ -249,7 +247,7 @@ type EtcdServer struct {

// NewServer creates a new EtcdServer from the supplied configuration. The
// configuration is considered static for the lifetime of the EtcdServer.
func NewServer(cfg *ServerConfig) (srv *EtcdServer, err error) {
func NewServer(cfg ServerConfig) (srv *EtcdServer, err error) {
st := store.New(StoreClusterPrefix, StoreKeysPrefix)

var (
Expand Down Expand Up @@ -408,11 +406,10 @@ func NewServer(cfg *ServerConfig) (srv *EtcdServer, err error) {

heartbeat := time.Duration(cfg.TickMs) * time.Millisecond
srv = &EtcdServer{
readych: make(chan struct{}),
Cfg: cfg,
snapCount: cfg.SnapCount,
errorc: make(chan error, 1),
store: st,
readych: make(chan struct{}),
Cfg: cfg,
errorc: make(chan error, 1),
store: st,
r: *newRaftNode(
raftNodeConfig{
isIDRemoved: func(id uint64) bool { return cl.IsIDRemoved(types.ID(id)) },
Expand Down Expand Up @@ -522,9 +519,9 @@ func (s *EtcdServer) Start() {
// modify a server's fields after it has been sent to Start.
// This function is just used for testing.
func (s *EtcdServer) start() {
if s.snapCount == 0 {
if s.Cfg.SnapCount == 0 {
plog.Infof("set snapshot count to default %d", DefaultSnapCount)
s.snapCount = DefaultSnapCount
s.Cfg.SnapCount = DefaultSnapCount
}
s.w = wait.New()
s.applyWait = wait.NewTimeList()
Expand Down Expand Up @@ -909,7 +906,7 @@ func (s *EtcdServer) applyEntries(ep *etcdProgress, apply *apply) {
}

func (s *EtcdServer) triggerSnapshot(ep *etcdProgress) {
if ep.appliedi-ep.snapi <= s.snapCount {
if ep.appliedi-ep.snapi <= s.Cfg.SnapCount {
return
}

Expand Down
34 changes: 12 additions & 22 deletions etcdserver/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func TestApplyRepeat(t *testing.T) {
})
s := &EtcdServer{
r: *r,
Cfg: &ServerConfig{},
store: st,
cluster: cl,
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand Down Expand Up @@ -527,7 +526,6 @@ func TestApplyConfChangeError(t *testing.T) {
srv := &EtcdServer{
r: *newRaftNode(raftNodeConfig{Node: n}),
cluster: cl,
Cfg: &ServerConfig{},
}
_, err := srv.applyConfChange(tt.cc, nil)
if err != tt.werr {
Expand Down Expand Up @@ -682,7 +680,7 @@ func TestDoProposal(t *testing.T) {
transport: rafthttp.NewNopTransporter(),
})
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *r,
store: st,
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand Down Expand Up @@ -710,7 +708,7 @@ func TestDoProposal(t *testing.T) {
func TestDoProposalCancelled(t *testing.T) {
wt := mockwait.NewRecorder()
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *newRaftNode(raftNodeConfig{Node: newNodeNop()}),
w: wt,
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand All @@ -732,7 +730,7 @@ func TestDoProposalCancelled(t *testing.T) {

func TestDoProposalTimeout(t *testing.T) {
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *newRaftNode(raftNodeConfig{Node: newNodeNop()}),
w: mockwait.NewNop(),
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand All @@ -748,7 +746,7 @@ func TestDoProposalTimeout(t *testing.T) {

func TestDoProposalStopped(t *testing.T) {
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *newRaftNode(raftNodeConfig{Node: newNodeNop()}),
w: mockwait.NewNop(),
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand Down Expand Up @@ -852,7 +850,7 @@ func TestSyncTrigger(t *testing.T) {
})

srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *r,
store: mockstore.NewNop(),
SyncTicker: tk,
Expand Down Expand Up @@ -910,7 +908,6 @@ func TestSnapshot(t *testing.T) {
storage: p,
})
srv := &EtcdServer{
Cfg: &ServerConfig{},
r: *r,
store: st,
}
Expand Down Expand Up @@ -979,9 +976,7 @@ func TestSnapshotOrdering(t *testing.T) {
raftStorage: rs,
})
s := &EtcdServer{
Cfg: &ServerConfig{
DataDir: testdir,
},
Cfg: ServerConfig{DataDir: testdir},
r: *r,
store: st,
cluster: cl,
Expand Down Expand Up @@ -1051,8 +1046,7 @@ func TestTriggerSnap(t *testing.T) {
transport: rafthttp.NewNopTransporter(),
})
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
snapCount: uint64(snapc),
Cfg: ServerConfig{TickMs: 1, SnapCount: uint64(snapc)},
r: *r,
store: st,
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand Down Expand Up @@ -1116,9 +1110,7 @@ func TestConcurrentApplyAndSnapshotV3(t *testing.T) {
raftStorage: rs,
})
s := &EtcdServer{
Cfg: &ServerConfig{
DataDir: testdir,
},
Cfg: ServerConfig{DataDir: testdir},
r: *r,
store: st,
cluster: cl,
Expand Down Expand Up @@ -1202,7 +1194,6 @@ func TestAddMember(t *testing.T) {
})
s := &EtcdServer{
r: *r,
Cfg: &ServerConfig{},
store: st,
cluster: cl,
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand Down Expand Up @@ -1244,7 +1235,6 @@ func TestRemoveMember(t *testing.T) {
})
s := &EtcdServer{
r: *r,
Cfg: &ServerConfig{},
store: st,
cluster: cl,
reqIDGen: idutil.NewGenerator(0, time.Time{}),
Expand Down Expand Up @@ -1319,7 +1309,7 @@ func TestPublish(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
srv := &EtcdServer{
readych: make(chan struct{}),
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
id: 1,
r: *newRaftNode(raftNodeConfig{Node: n}),
attributes: membership.Attributes{Name: "node1", ClientURLs: []string{"http://a", "http://b"}},
Expand Down Expand Up @@ -1369,7 +1359,7 @@ func TestPublishStopped(t *testing.T) {
transport: rafthttp.NewNopTransporter(),
})
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *r,
cluster: &membership.RaftCluster{},
w: mockwait.NewNop(),
Expand All @@ -1391,7 +1381,7 @@ func TestPublishRetry(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
n := newNodeRecorderStream()
srv := &EtcdServer{
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *newRaftNode(raftNodeConfig{Node: n}),
w: mockwait.NewNop(),
stopping: make(chan struct{}),
Expand Down Expand Up @@ -1432,7 +1422,7 @@ func TestUpdateVersion(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
srv := &EtcdServer{
id: 1,
Cfg: &ServerConfig{TickMs: 1},
Cfg: ServerConfig{TickMs: 1},
r: *newRaftNode(raftNodeConfig{Node: n}),
attributes: membership.Attributes{Name: "node1", ClientURLs: []string{"http://node1.com"}},
cluster: &membership.RaftCluster{},
Expand Down
2 changes: 1 addition & 1 deletion integration/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ func (m *member) Clone(t *testing.T) *member {
func (m *member) Launch() error {
plog.Printf("launching %s (%s)", m.Name, m.grpcAddr)
var err error
if m.s, err = etcdserver.NewServer(&m.ServerConfig); err != nil {
if m.s, err = etcdserver.NewServer(m.ServerConfig); err != nil {
return fmt.Errorf("failed to initialize the etcd server: %v", err)
}
m.s.SyncTicker = time.NewTicker(500 * time.Millisecond)
Expand Down

0 comments on commit d433067

Please sign in to comment.