Repository navigation
Description
Summary
A node that writes a NewView record and then a ProposedRecord or Commit
before restarting restores two different view numbers. PersistedState.Restore
sets View.Number from the proposal record. Consensus.setViewAndSeq keeps the
view from the application metadata because LoadNewViewIfApplicable returns
nil. The controller and the view then disagree about the current view.
Impact
Controller.startView computes the leader and starts the LeaderMonitor with
the older view, while the view runs with the proposal's view. The view changer
also starts from the older view. The node can compute the wrong leader and send
messages for a view that does not match the view restored from its WAL. This can
delay or block progress after a restart. It is a liveness problem.
Root cause
The built-in WAL drops the NewView record before the restart. View.Save
writes the pre-prepare with truncateTo=true, and
WriteAheadLogFile.ReadAll clears all earlier entries when it reads that flag.
After the restart, PersistedState.Entries contains only the proposal pair:
ProposedRecord(view 301, seq 201)
LoadNewViewIfApplicable has no NewView to find, so it returns nil.
Consensus.setViewAndSeq keeps newView from the application metadata, and
startComponents starts the controller and the view changer in that older view.
Restore still reads the proposal record and sets View.Number to the
proposal's view (301). The controller uses the metadata view (300), while the
view uses 301.
The earlier explanation, "LoadNewViewIfApplicable only reads the last entry",
was incomplete: scanning the WAL for a NewView cannot help, because the record
is already gone.
Reproduction
A real restart after a view change and a proposal leaves this WAL:
ProposedRecord(seq 201, view 301)
The NewView(view 301, seq 200) record was truncated when the pre-prepare was
saved.
Then:
LoadNewViewIfApplicable()returns nil.Restore(view)setsview.Number = 301.setViewAndSeq(metadata view 300, seq 200)returns view 300.- The controller starts at view 300, while the view runs at view 301.
The integration test TestRestartAfterNewViewAndProposalRestoresStartView
reproduces this with four nodes: node 4 accepts the pre-prepare in view 1 but
not enough prepares or commits, then restarts. After the restart
GetLeaderID() returns 1 instead of 2.
Expected behavior
When a NewView record survives, LoadNewViewIfApplicable should scan the WAL
backwards to the latest view transition, return the last NewView, skip
proposal and commit records, and stop at a later ViewChange so
LoadViewChangeIfApplicable keeps precedence.
When no NewView survives, Consensus.setViewAndSeq should use the view of
the trailing ProposedRecord or Commit. If that view is newer than the
metadata view, the controller should start there and reset decisions in view to
0. The sequence should stay from the metadata.
Fix
PR: #750
It adds PersistedState.LoadLastProposalView and uses it as a fallback in
Consensus.setViewAndSeq. Tests cover a NewView followed by a proposal, by a
commit, and by a proposal plus a later ViewChange, and the trailing proposal
and commit fallback. The integration test
TestRestartAfterNewViewAndProposalRestoresStartView covers the real WAL path.
This is separate from #749: #749 is about the in-flight proposal when the WAL
ends with a view record. This issue is about the start view after a restart.
Activity