Smallibs/kraft

Leader, Electors and Candidate should demote to Follower when receiving youger term

sadraskol opened this issue · 2 comments

When reading the tests, I tried a case from the specification: whenever a node receives a AppendEntries call with a younger term, the node should immediately turn into a Follower, update its currentTerm and consider the node which issued the term the leader.

To prove the behavior, I wrote the following test:

@Test
fun `Leader should downgrade to follower with a RequestAppend of younger term`() {
    Transition.run {
        Leader("A".id, 1.term, listOf("A".id, "B".id, "C".id))
                .perform({ true }, RequestAppend<Unit>("B".id, 2.term, 0.index to 1.term, 0.index))
    }.let {
        assertEquals(NodeKind.Follower("A".id, 2.term, listOf("A".id, "B".id, "C".id), "B".id), it.first)
        assertEquals(listOf(), it.second)
    }
}

I think the culprit is the implementation of NodeKind.stepDown in TransitionImpl. The problem is that the method treats RequestVote and AppendEntries the same way, which should not be the case. I would advise to change the implementation to make the two RPC state transitions clearer.

That's right. In fact, this is the consequence of the differentiation between a follower without a leader from a follower with a leader. Worst each time an elector receives a message with a younger term it "becomes" again an elector. As a consequence, when a candidate has been elected it shall never send election messages. Therefor an elector stays an elector until a new election is initiated.

The stepDown can be revisited but this is not clear enough:

    private fun <Command> NodeKind.stepDown(action: Action<Command>): TransitionResult<Command> {
        val elector = this.becomeElector(action.term)
        
        return when (action) {
            is RequestAppend<Command> ->
                elector.becomeFollower(action.leader)
            else ->
                elector
        } to when (this) {
            is Leader ->
                listOf(ArmTimeout(Election))
            else ->
                listOf()
        }
    }

I tried to find a more elegant way to implement it, and didn't succeed. Your suggestion will do its job.