raftexample: Fix recovery from snapshot

[mrkm4ntr]

• If there is a snapshot, HTTP server won't start.
  This function doesn't return.

  etcd/contrib/raftexample/kvstore.go

  Line 43 in 7cc2f8a

  s.readCommits(commitC, errorC)

• Resotring form snapshot occurs after replaying WAL.
• When taking a snapshot, the last change is not applied to the state machine yet.

[mrkm4ntr]

@gyuho Could you kindly review this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

[mrkm4ntr]

@gyuho @xiang90 Could you kindly review on this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

[MrCroxx]

Found the same problem, bro.

When I was developing my graduation project last year, I referenced this sample and I found there seemed to be something wrong here. At that time I'm a beginner in etcd or raft, and I thought that I have missed something.

When I am reviewing the sample and writing my blog, I found there IS a bug. It is very misleading for a beginner.

[ptabor]

Thank you.
It LGTM, but I think it should get reviewed by RAFT owners:

@tbg @bdarnell @tangcong

[bdarnell]

Looks fine to me, but I've never looked at raftexample or the etcd-specific snap package, so I don't think I'm very helpful as a reviewer here.

[tbg]

Unfortunately same as Ben here, I have never actually interacted with this raftexample code.

…

On Tue, Feb 2, 2021 at 7:22 AM Shintaro Murakami ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In contrib/raftexample/raft.go <#11889 (comment)>: > @@ -428,6 +415,7 @@ func (rc *raftNode) serveChannels() { // store raft entries to wal, then publish over commit channel case rd := <-rc.node.Ready(): + rc.maybeTriggerSnapshot() We have to trigger snapshot before publishEntries because writing in-memory map is async via commitC. If not do so, when taking snapshot, entries may not be applied to the map yet. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11889 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGXPZDXZYYLUDIN6NSF2WLS46K2ZANCNFSM4NANVANQ> .

[ptabor]

Please add:
// commitC is synchronous channel, so consumption of the message signals
// full application of previous messages.

[mrkm4ntr]

Addressed.