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

etcdserver: commit before sending snapshot #5862

Merged
merged 1 commit into from
Jul 3, 2016
Merged

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Jul 3, 2016

Fix #5857.

Reproduce:

  1. write some v2 keys to force a snapshot
  2. remove a member
  3. add the member back
  4. kill the added member immediately before a new snapshot is triggered or consistent index is ever updated.

Then the newly added member will not be able to restart anymore.

This pr fixes it.

@xiang90
Copy link
Contributor Author

xiang90 commented Jul 3, 2016

/cc @binwiederhier @heyitsanthony

@heyitsanthony
Copy link
Contributor

lgtm
is it necessary to do a full Commit() to persist down to disk or is it only so that saveIndex is triggered so Snapshot() has the most recent index?

@binwiederhier
Copy link

Can confirm that it worked for me.

@gyuho
Copy link
Contributor

gyuho commented Jul 3, 2016

lgtm.

@xiang90
Copy link
Contributor Author

xiang90 commented Jul 3, 2016

is it necessary to do a full Commit() to persist down to disk or is it only so that saveIndex is triggered so Snapshot() has the most recent index?

Both. We need to send a snapshot contains the full synced data.

But in this case, the commit() is just for the metadata since when we call db.Snapshot(), it will do the db.sync() for us, however without updating the in-memory metadata like consistentIndex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Cannot restart after shutdown / Ctrl+C (etcdmain: database file does not match with snapshot)
4 participants