-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
*: support wal dir #3413
*: support wal dir #3413
Conversation
/cc @kelseyhightower This fixes #2442. @philips @adohe Can you take a look to see if this matches your expectation? @robszumski @ecnahc515 It would be great if you guys can check the documentation changes. |
@@ -105,7 +108,12 @@ func (c *ServerConfig) verifyLocalMember(strict bool) error { | |||
|
|||
func (c *ServerConfig) MemberDir() string { return path.Join(c.DataDir, "member") } | |||
|
|||
func (c *ServerConfig) WALDir() string { return path.Join(c.MemberDir(), "wal") } | |||
func (c *ServerConfig) WALDir() string { |
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.
Couldn't this logic be moved into the initialization of the ServerConfig
? IE: You can do this logic in startEtcd()
and then just assign to config.WALDir
(and then remove this method).
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.
We could. But this is how MemberDir ans SnapshotDir works no2.(If I remember correctly, we have some legacy issue put all these here but we do not have to do it any more now ) We can clean them up all together in a future pull request.
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.
Sounds good.
LGTM, I had a small code nitpick, but the docs added seem to be just fine. |
LGTM. |
err = os.MkdirAll(cfg.MemberDir(), privateDirMode) | ||
if err != nil && err != os.ErrExist { | ||
return nil, err | ||
} |
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.
why do we need to make member dir in this commit?
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.
We used to use mkall to create member dir when creating member/wal. It might not be enough after we support separate wal dir.
LGTM |
e58f5b9
to
d94e712
Compare
@yichengq PTAL |
LGTM. |
Having a dedicated disk to store wal files can improve the throughput and stabilize the cluster.
Add
--wal-flag
to support this use case.