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: ensure hardstate is persisten before applying committed entries #16675

Closed
wants to merge 1 commit into from

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Sep 30, 2023

// If the hardstate isn't empty, we must sync the hardstate to WAL file before
// applying the committed entries. Otherwise, etcd may have already responded
// to the client, but the hardstate isn't persisted to the WAL file. When etcd
// starts again, it loads the stale hardstate, and accordingly a linearizable
// request may get the stale readIndex (actually committed Index), eventually
// it won't wait for the latest applied data before responding to the client.

The solution is similar to #14400. Need to evaluate the impact on performance.

@serathius
Copy link
Member

Hmm, it's an interesting that etcd didn't require flushing move of commit index.

Etcd starts from sending a publish request that requires a quorum, it doesn't even wait for the request to be processed, just for a proposal. So, to avoid going back in time, etcd just requires getting a quorum. This is enough for a the member to get the commit index from leader and maybe apply the entries from before the crash.

I see two problems:

  • --wait-cluster-ready-timeout totally circumvents this mechanism allowing etcd to start serving data before it's ready.
  • There is no mechanism that prevents serving before commit index was applied, waiting for quorum might not be enough.

@serathius
Copy link
Member

serathius commented Oct 1, 2023

Also "ensure hardstate is persisted before applying committed entries" might be stricter then required. We just need to "ensure hardstate is persisted before responsing to user".

@ahrtr
Copy link
Member Author

ahrtr commented Oct 5, 2023

Won't continue to work on this.

#16666 (comment)

@ahrtr ahrtr closed this Oct 5, 2023
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.

2 participants