mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Jean-Noel Rouvignac
18.56.2014 60f8d8d4575206697f47c040d4272dee27251bab
refs
author Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Friday, April 18, 2014 15:56 +0200
committer Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Friday, April 18, 2014 15:56 +0200
commit60f8d8d4575206697f47c040d4272dee27251bab
tree 7d33de30b9f87bcb424d3153a027a522ccfbadc5 tree | zip | gz
parent 9737dd7d653611c9d9ee38640685a68d43abbca4 view | diff
OPENDJ-1430 Some changes are missing from the external changelog changeNumber

The bug was due to a very complex interaction between various components. Here is a scenario and explanation:
1- the change number indexer has no more records to proceed, because all cursors are exhausted, so it calls wait().
2- a new change Upd1 comes in for an exhausted cursor, medium consistency cannot move.
3- a new change Upd2 comes in for a cursor that is not already opened, medium consistency can move, so wake up the change number indexer.
3- on wake up, the change number indexer calls next(), advancing the CompositeDBCursor, which recycles the exhausted cursor, then calls next() on it, making it lose its change. CompositeDBCursor currentRecord == Upd1.
4- on the next iteration of the loop in run(), a new cursor is created, triggering the creation of a new CompositeDBCursor => Upd1 is lost. CompositeDBCursor currentRecord == Upd2.

The problem comes from two parts:
- CompositeDBCursor consumes next change from a cursor (which completely forget about this change) and stores it itself
- ChangeNumberIndexer manages recycling/creating cursors on its own and recreates CompositeDBCursor when a new cursor is created.

The fix required:
- Preventing CompositeDBCursor from consuming changes from underlying cursors until it can forget about this same change itself.
- Ensuring only ChangeNumberIndexer handle recycling the cursors it owns instead of having both CompositeDBCursor and ChangeNumberIndexer trying to do it. It is also more performant to let ChangeNumberIndexer manage its cursors.



CompositeDBCursor.java:
Added recycleExhaustedCursors field to tell the composite whether it can recycle the cursors itself or not (recycling the cursors is currently needed for persistent searches on the changelog, maybe will we be able to remove it in the future, that would simplify the code a lot).
Modified the ctor to pass in value of recycleExhaustedCursors.
Removed currentRecord and currentData fields, replaced by reading the record and field on the first entry in the cursors SortedMap.
Added state field to ensure the first call to next() does not consume the first change in the cursors SortedMap.

ChangeNumberIndexer.java:
ChangeNumberIndexer now manages alone the cursors recycling and creation and recreates the CompositeDBCursor when needed.
In run(), removed the now unneeded call to next() after the wait.
Added recycleExhaustedCursors().

JEChangelogDB.java:
Consequence of the change to CompositeDBCursor. Kept old recycling behaviour.


ChangeNumberIndexerTest.java:
Added emptyDBTwoDSsDoesNotLoseChanges() to cover the case being fixed by current commit.
Renamed test methods dropping the "Initial" when it was not adding much to the test comprehension.

CompositeDBCursorTest.java:
In newUpdateMsg(), added toString() implementation to help debug.
Removed recycleTwoElementCursorsTODOJNR().
In recycleTwoElementCursors(), changed the tests a bit to match the changes to CompositeDBCursor.
5 files modified
259 ■■■■ changed files
opends/src/server/org/opends/server/replication/server/changelog/je/ChangeNumberIndexer.java 40 ●●●● diff | view | raw | blame | history
opends/src/server/org/opends/server/replication/server/changelog/je/CompositeDBCursor.java 92 ●●●● diff | view | raw | blame | history
opends/src/server/org/opends/server/replication/server/changelog/je/JEChangelogDB.java 4 ●●● diff | view | raw | blame | history
opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/ChangeNumberIndexerTest.java 83 ●●●● diff | view | raw | blame | history
opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/CompositeDBCursorTest.java 40 ●●●●● diff | view | raw | blame | history