LogBufferUnblocker::unblock activeTermCount/expectedTermCount

[goglusid]

For the following function, if the following situation happens then the blockedoffset is used with the wrong term id.

IF

blockedPosition is within TermId = 0 (the second half because otherwise the pub=lmt doesn't cross two terms)
activeTermCount is now at 1

Then

index = 1
termId = 1

However given that blockedOffset refers to term 0, working with termBuffers[index] doesn't make sense.

It seems like we should use the expectedTermCount to compute the index instead.

LogBufferUnblocker::unblock()
{
final int termLength = termBuffers[0].capacity();
final int positionBitsToShift = Integer.numberOfTrailingZeros(termLength);
final int activeTermCount = activeTermCount(logMetaDataBuffer);
final int expectedTermCount = (int)(blockedPosition >> positionBitsToShift);
final int index = indexByTermCount(activeTermCount);
final long rawTail = rawTailVolatile(logMetaDataBuffer, index);
final int termId = termId(rawTail);

...

    final int tailOffset = termOffset(rawTail, termLength);
    final int blockedOffset = computeTermOffsetFromPosition(blockedPosition, positionBitsToShift);

}

[tmontgomery]

You are talking about the Java code and not the C++ code, correct... the use of "::" had me confused.

If I follow your logic, there are some tests for this case. Specifically, doesn't shouldUnblockWhenPositionHasNonCommittedMessageAndTailPastEndOfTerm work how you mention? Or at least the spirit of it.

If I don't understand the test case, it should be straight forward to construct it in LogBufferUnblockTest.java.

[mjpt777]

I think this is an issue in the Java code. I've been testing and about to push a fix. It has been a typo when I realised a missing case and my solution is not right.

[mjpt777]

It would be good to review if what I pushed makes sense.

[mjpt777]

That last push has an issue. I'm fixing.

[tmontgomery]

Let me know when you are happy and I will review and reflect on the C side.

[goglusid]

The reported issue is applicable to Java and C.

For the unblock section of the code I think we should use expectedTermCount.

For the rotateLog sections, I think we should still use the activTermCount though.

[goglusid]

Sorry I replied too rapidly I will review the latest changes later and post my feedback

[tmontgomery]

@goglusid I'm jetlagged badly, so it takes very little to confuse me. ;)

[goglusid]

The latest changes resolved the issue. ;) Thanks