Ensure one committed entry is always accessible in the log

[kuujo]

This PR fixes #60 to ensure that at least one committed entry is always retained in the log and made accessible during AppendRequest handling.

[kuujo]

I think the first build failed due to the Catalyst bug recently fixed. I released a new Catalyst version, so I'll update the dependency in another PR.

[madjam]

👍
Nice. I like this much better.

[kuujo]

Agree. Much nicer.

But I think there could still be an issue here. commitIndex can be greater than lastIndex and in that case, this logic will hide lastIndex if it's clean()ed. I'm actually not sure if this could happen in practice. State machines should clean entries deterministically, and AppendRequest doesn't send null entries but always ends with a non-null entry. So, it shouldn't really be possible for a log with commitIndex > lastIndex to have a non-null entry at the end since the only way for an entry at the end of a log to be null is for the local server state machine to clean() it, and the local server state machine can't clean an entry before the leader's state machine simply as a property of the order of commitment. But maybe it wouldn't hurt to add a index == lastIndex() condition just in case.

[madjam]

I agree. This is a case that should not arise. It may make sense to add an assertion to catch it if/when it occurs.

[kuujo]

Ahh yes assertion good idea

On Nov 20, 2015, at 9:30 AM, Madan Jampani notifications@github.com wrote:

I agree. This is a case that should not arise. It may make sense to add an assertion to catch it if/when it occurs.

—
Reply to this email directly or view it on GitHub.

[kuujo]

Hmm... The assertion's kind of a pain because some code does read the log in that state while appending entries. I'm just gonna leave it how it is for now.

On a related note, the fact that it writes entries sequentially and entries received in AppendRequest can be received with gaps does not impact the assertion that the last entry will always be visible. If a failure occurs while handling an AppendRequest, the recovery process will recover the log up to the last written non-null entry and will exclude skipped/null entries, so that's not an issue.