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

contrib/raftexample: adding new members after snapshot taken fails #13741

Closed
dperny opened this issue Feb 24, 2022 · 5 comments · Fixed by #13760
Closed

contrib/raftexample: adding new members after snapshot taken fails #13741

dperny opened this issue Feb 24, 2022 · 5 comments · Fixed by #13760
Labels

Comments

@dperny
Copy link

dperny commented Feb 24, 2022

What happened?

Using the contrib/raftexample code, adding a new node to the cluster if there is a Snapshot outstanding results in:

2 attempted to restore snapshot but it is not in the ConfState {[1] [] [] [] false []}; should never happen

What did you expect to happen?

Node should be successfully added to cluster. Snapshot should be restored and logs replicated past it.

How can we reproduce it (as minimally and precisely as possible)?

  1. First, alter the example such that the snapshot threshold is very low. In raftexample/raft.go, find the line var defaultSnapshotCount uint64 = 10000 and set the number to 3.
  2. Build and run the raftexample binary as listed in README.md
  3. Put a few entries into the cluster to trigger creation of a snapshot.
  4. Attempt to add a new member to the cluster through the commands listed in README.md.
  5. Observe the failure.

Anything else we need to know?

I am a maintainer of Docker Swarmkit, working on upgrading many old dependencies. Upgrading our etcd/raft dependency past 3.3.x does not work.

I have tracked down our problem to the error message listed in the first second. Attempting to add a new member fails after a snapshot has been taken, because when attempting to restore the snapshot, we are not listed in the snapshot's ConfState, and so will not restore the snapshot.

From where I am, with my current understanding, the problem looks like this:

  1. Node 2 need to join the cluster.
  2. To join the cluster, Node 2 needs to replicate the log.
  3. To replicate the log, Node 2 must first start from the snapshot.
  4. Node 2 cannot start from the snapshot, because the ConfState in the snapshot metadata does not include Node 2, as the snapshot comes before the log entry adding Node 2 to the ConfState.

I understand that this is not a bug in itself -- the error lies somewhere in Swarmkit's use of the raft library. There's something we must do before or in the course of restoring the received snapshot. I just don't know what it is.

My first instinct is to rely on the raft example to determine what I am doing wrong. However, with the example broken in an identical way, I'm left trying to plow through walls of the main etcd server code to locate the analogous sections and figure out how etcd handles this problem of snapshots during cluster joins.

Fixing this issue with contrib/raftexample allows me (and others, potentially) to understand what the issue is in our usage of etcd/raft, how to correctly join a new member with snapshot, and how to ultimately fix this issue in the Swarmkit code base.

This issue was previously raised in #12473, which was closed for being stale, and a fix was attempted in #13578, which was more-or-less deemed incorrect. I apologize if opening a new issue is incorrect.

Etcd version (please run commands below)

N/A, but exact version of the codebase being used is v3.5.2

Etcd configuration (command line flags or environment variables)

N/A

Etcd debug information (please run commands blow, feel free to obfuscate the IP address or FQDN in the output)

No response

Relevant log output

No response

@thaJeztah
Copy link

@spzala ptal 🤗

@ahrtr
Copy link
Member

ahrtr commented Mar 7, 2022

Thanks for raising this issue. I just fixed it in pull/13760. Please see the explanation in the PR. Please also kindly let me know whether it works for you.

@dperny
Copy link
Author

dperny commented Mar 7, 2022

Thank you very much! Late last week I'd finally gotten far enough into the etcd code to determine this was likely the solution, but I was not yet certain. This clears it up.

@thaJeztah
Copy link

Thanks @ahrtr !

@spzala
Copy link
Member

spzala commented Mar 7, 2022

Thanks @ahrtr I will be reviewing the changes today.

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

Successfully merging a pull request may close this issue.

4 participants