Skip to content
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

Merged
merged 1 commit into from
Sep 1, 2015
Merged

*: support wal dir #3413

merged 1 commit into from
Sep 1, 2015

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Aug 31, 2015

Having a dedicated disk to store wal files can improve the throughput and stabilize the cluster.
Add --wal-flag to support this use case.

@xiang90
Copy link
Contributor Author

xiang90 commented Aug 31, 2015

/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.

@xiang90 xiang90 added this to the v2.2.0 milestone Aug 31, 2015
@@ -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 {
Copy link
Contributor

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).

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@chancez
Copy link
Contributor

chancez commented Sep 1, 2015

LGTM, I had a small code nitpick, but the docs added seem to be just fine.

@adohe-zz
Copy link

adohe-zz commented Sep 1, 2015

LGTM.

err = os.MkdirAll(cfg.MemberDir(), privateDirMode)
if err != nil && err != os.ErrExist {
return nil, err
}
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@robszumski
Copy link
Contributor

LGTM

@xiang90
Copy link
Contributor Author

xiang90 commented Sep 1, 2015

@yichengq PTAL

@yichengq
Copy link
Contributor

yichengq commented Sep 1, 2015

LGTM.

xiang90 added a commit that referenced this pull request Sep 1, 2015
@xiang90 xiang90 merged commit a941188 into etcd-io:master Sep 1, 2015
@xiang90 xiang90 deleted the snapshot_dir branch September 1, 2015 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants