persistit: keep JournalManager consistent when rollover fails near the block end#267
Open
vharseko wants to merge 1 commit into
Open
Conversation
…e block end An IOException thrown inside JournalManager.rollover() - e.g. an InterruptedIOException when the rolling thread is interrupted during flush, truncate or force - after writeJournalEnd() had already consumed the JE reserve left _currentAddress closer than JE.OVERHEAD to the block end. The retried rollover then wrote the JE record past the buffer limit (JournalRecord putters bypass the ByteBuffer bounds checks) and advance() updated _currentAddress before the position update threw, tearing the invariant _writeBufferAddress + _writeBuffer.position() == _currentAddress. Every subsequent journal write then failed, seen as the AssertionError storm in TransactionTest2.transactionsWithInterrupts. - writeJournalEnd(): skip the JE record when it no longer fits before the block end so a retried rollover completes and heals the state; recovery treats the missing JE as a dirty shutdown. - advance(): update the buffer position, which validates against the limit and may throw, before _currentAddress so a failure cannot tear the bookkeeping. - JournalRecord: fail fast when a record write would exceed the buffer limit instead of silently writing past it into the backing array. - IOFailureTest: deterministically reproduce the abandoned-rollover geometry with an injected truncate failure. - TransactionTest2: catch Throwable in workers so internal Errors are reported and counted instead of silently killing the thread. Fixes OpenIdentityPlatform#265
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #265.
Under sustained
Thread.interrupt()stress theJournalManagerwrite-buffer bookkeeping could be torn around a journal rollover, after which every journal writer tripped the invariant assert (writeBufferAddress=2,999,999,998 position=0 currentAddress=3,000,000,038). The failure is two-phase:rollover()is not exception-safe:writeJournalEnd()advances_currentAddressbyJE.OVERHEAD(40 bytes) to within 40 bytes of the block end, and only then does the I/O (flush/truncate/force). AnInterruptedIOExceptionthere (surfaced byMediatedFileChannelon an interrupted thread) abandons the rollover before_currentAddressjumps to the next block boundary. The invariant still holds at this point, but the reserveprepareWriteBuffermaintains for the JE record is gone: fewer thanJE.OVERHEADbytes remain to the block end, and the write-buffer limit is clamped to that remainder.JE.put*helpers write into the backing array directly, bypassing theByteBufferlimit, so nothing fails untiladvance(JE.OVERHEAD)updates_currentAddressfirst and then throws from_writeBuffer.position(...)(newPosition > limit), leaving_currentAddressexactly 40 bytes ahead of_writeBufferAddress + position- the observed values. Every subsequent journal write then fails the assert (or hits the sameIllegalArgumentExceptionwith assertions disabled).Changes
JournalManager.writeJournalEnd(): skip the JE record when it no longer fits before the block end, so a retried rollover completes and heals the poisoned state; recovery treats the missing JE as a dirty shutdown.JournalManager.advance(): update the buffer position (which validates against the limit and may throw) before_currentAddress, so a failure cannot tear the invariant.JournalRecord: fail fast (IndexOutOfBoundsException) when a record write would exceed the buffer limit instead of silently writing past it into the backing array.IOFailureTest#testRolloverRecoversAfterFailureNearBlockEnd: deterministic reproduction - positions_currentAddressexactlyJE.OVERHEAD + 2bytes before the block end, aborts the rollover with an injectedtruncate()failure, verifies the poisoned geometry, then verifies the retried rollover heals and journal writes keep working.TransactionTest2: workers catchThrowableso an internalErroris reported and counted instead of silently killing the thread (the cascade that pollutedtransactionsConcurrentWithPersistitClosein the same CI run).Testing
IllegalArgumentException: newPosition > limit: (40 > 2)(and the same error fromtearDown, showing the state never heals) and passes with the fix.persistit/coresuite: 566 tests, 0 failures, 0 errors, 5 pre-existing skips.