-
Notifications
You must be signed in to change notification settings - Fork 720
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
*: upgrade etcd to master #1101
Conversation
/run-all-tests |
server/join.go
Outdated
initialCluster := "" | ||
if wal.Exist(path.Join(cfg.DataDir, "member")) { | ||
if wal.Exist(path.Join(cfg.DataDir, "member/wal")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, it breaks the compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it's compatibility. but better to keep the previous logic. address it.
@@ -42,6 +42,8 @@ func newTestSingleConfig() *embed.Config { | |||
cfg.Name = "test_etcd" | |||
cfg.Dir, _ = ioutil.TempDir("/tmp", "test_etcd") | |||
cfg.WalDir = "" | |||
cfg.Logger = "zap" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we use zap in production?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
etcd plan to use zap
replace the capnslog
logging. and we only enable it in the test.
server/server.go
Outdated
@@ -285,6 +285,16 @@ func (s *Server) Run() error { | |||
return nil | |||
} | |||
|
|||
// SetUpTestMode for testing | |||
func (s *Server) SetUpTestMode() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do it in server.CreateServer
? It looks verbose to call it in every tests...
LGTM. PTAL @siddontang @Connor1996 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
Changes:
embed.Config.InitialElectionTickAdvance
to enable/disable initial election tick fast-forward.embed.NewConfig()
would return*embed.Config
withInitialElectionTickAdvance
as true bydefault.
--pre-vote
flag to enable to run an additional Raft election phasewill enbale later
embed.CompactorModePeriodic
forcompactor.ModePeriodic
.compatible
embed.CompactorModeRevision
forcompactor.ModeRevision
.compatible
embed.Config.CorsInfo
in*cors.CORSInfo
type toembed.Config.CORS
inmap[string]struct{}
type.not used
embed.Config.SetupLogging
.Now logger is set up automatically based onembed.Config.Logger
,embed.Config.LogOutputs
,embed.Config.Debug
fields.will let test meet the panic problem, fixed.
embed.Config.Logger
to support structured loggerzap
in server-side.only use
zap
in test.embed.Config.SnapCount
field toembed.Config.SnapshotCount
, to be consistent with the flag nameetcd --snapshot-count
.not used
embed.Config.LogOutput
toembed.Config.LogOutputs
to support multiple log outputs.compatible
embed.Config.LogOutputs
type fromstring
to[]string
to support multiple log outputs.compatible
raft.Config.CheckQuorum
when starting withForceNewCluster
.If new members are added to standalone node with
CheckQuorum == false
and the standalone node is the leader, this may reduce cluster availabilities, since the leader won't check quorum activities.