Skip to content

Commit

Permalink
TreeNodeDynamicSize impl part 10 - Zero pad empty space
Browse files Browse the repository at this point in the history
Previously, space that was left empty after removing key pointers and children
was not padded with zeroes. If this empty space was later partly used
by data space (the actual keys) and then reclaimed we would have
broken unused child pointers left as a result. This is not problem
until you want to reuse this space for child pointers again. But when
you do, you read the (broken) child pointer that is their before writing
what you want to write. Because this garbage data is interpreted as a
broken child pointer we can not overwrite it.
See visual below.

We fix this problem by always padding space left empty with zeroes
so that it can be reused for children later.

[HEADER] [KEYS_AND_CHILDREN] [EMPTY_SPACE] [DATA_SPACE]

[HEADER] [C0,K0*,C1,K1*,C2,K2*,C3] [EMPTY      ] [SOME_DATA]
- Removing K2*C3 without padding
[HEADER] [C0,K0*,C1,K1*,C2] [K2*,C3  EMPTY     ] [SOME_DATA]
- Data grows, breaking the unused child pointer
[HEADER] [C0,K0*,C1,K1*,C2] [K2*,C] [SOME_DATA                    ]
- Defragment shrinks data space
[HEADER] [C0,K0*,C1,K1*,C2] [K2*,C    EMPTY   ] [SOME_DATA   ]
- Insert a new K* and C, need to read space where C3 used to be.
  This is now garbage data and can not be read. FAIL
                                                        v
[HEADER] [C0,K0*,C1,K1*,C2] [K2*,C    EMPTY   ] [SOME_DATA   ]
  • Loading branch information
burqen committed Jan 16, 2018
1 parent bada419 commit def4e5f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
Expand Up @@ -433,8 +433,9 @@ private void assertKeyOrder( PageCursor cursor, KeyRange<KEY> range, int keyCoun
node.keyAt( cursor, readKey, pos, type );
if ( !range.inRange( readKey ) )
{
cursor.setCursorException( "Expected range for this node is " + range + " but found " + readKey +
" in position " + pos + ", with key count " + keyCount );
cursor.setCursorException(
format( "Expected range for this node is %n%s%n but found %s in position %d, with keyCount %d on page %d",
range, readKey, pos, keyCount, cursor.getCurrentPageId() ) );
}
if ( !first )
{
Expand Down
Expand Up @@ -412,7 +412,7 @@ private void doInsertInInternal( PageCursor cursor, StructurePropagation<KEY> st
{
// Overflow
// We will overwrite rightKey in structurePropagation, so copy it over to a place holder
layout.copyKey( structurePropagation.rightKey, newKeyPlaceHolder );
layout.copyKey( primKey, newKeyPlaceHolder );
splitInternal( cursor, structurePropagation, newKeyPlaceHolder, rightChild, keyCount,
stableGeneration, unstableGeneration );
return;
Expand Down
Expand Up @@ -353,7 +353,7 @@ abstract void moveKeyValuesFromLeftToRight( PageCursor leftCursor, int leftKeyCo

// Useful for debugging
@SuppressWarnings( "unused" )
void printNode( PageCursor cursor, boolean includeValue, long stableGeneration, long unstableGeneration )
void printNode( PageCursor cursor, boolean includeValue, boolean includeAllocSpace, long stableGeneration, long unstableGeneration )
{ // default no-op
}
}
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.index.internal.gbptree;

import java.util.Arrays;
import java.util.StringJoiner;

import org.neo4j.collection.primitive.PrimitiveIntStack;
Expand Down Expand Up @@ -197,6 +198,9 @@ void removeKeyAndRightChildAt( PageCursor cursor, int keyPos, int keyCount )

// Remove for offsetArray
removeSlotAt( cursor, keyPos, keyCount, keyPosOffsetInternal( 0 ), keyChildSize() );

// Zero pad empty area
zeroPad( cursor, keyPosOffsetInternal( keyCount - 1 ), bytesKeyOffset() + childSize() );
}

@Override
Expand All @@ -218,6 +222,9 @@ void removeKeyAndLeftChildAt( PageCursor cursor, int keyPos, int keyCount )

// Move last child
cursor.copyTo( childOffset( keyCount ), cursor, childOffset( keyCount - 1 ), childSize() );

// Zero pad empty area
zeroPad( cursor, keyPosOffsetInternal( keyCount - 1 ), bytesKeyOffset() + childSize() );
}

@Override
Expand Down Expand Up @@ -466,9 +473,8 @@ For each dead space of size X (can be multiple consecutive dead keys)
int prevAllocOffset = getAllocOffset( cursor );
setAllocOffset( cursor, aliveRangeOffset );

// Pad reclaimed area with zeroes
cursor.setOffset( prevAllocOffset );
cursor.putBytes( aliveRangeOffset - prevAllocOffset, (byte) 0 );
// Zero pad reclaimed area
zeroPad( cursor, prevAllocOffset, aliveRangeOffset - prevAllocOffset );

// Update offset array
int keyCount = keyCount( cursor );
Expand Down Expand Up @@ -763,6 +769,7 @@ private void moveKeysAndChildren( PageCursor fromCursor, int fromPos, PageCursor
int targetOffset = includeLeftMostChild ? childOffset( 0 ) : childOffset( 1 );
fromCursor.copyTo( childFromOffset, toCursor, targetOffset, lengthInBytes );

// Move actual keys and update pointers
int toAllocOffset = getAllocOffset( toCursor );
for ( int i = 0; i < count; i++, toPos++ )
{
Expand All @@ -772,6 +779,15 @@ private void moveKeysAndChildren( PageCursor fromCursor, int fromPos, PageCursor
putKeyOffset( toCursor, toAllocOffset );
}
setAllocOffset( toCursor, toAllocOffset );

// Zero pad empty area
zeroPad( fromCursor, childFromOffset, lengthInBytes );
}

private void zeroPad( PageCursor fromCursor, int childFromOffset, int lengthInBytes )
{
fromCursor.setOffset( childFromOffset );
fromCursor.putBytes( lengthInBytes, (byte) 0 );
}

private int transferRawKey( PageCursor fromCursor, int fromPos, PageCursor toCursor, int toAllocOffset )
Expand Down Expand Up @@ -1032,7 +1048,8 @@ public String toString()
return "TreeNodeDynamicSize[pageSize:" + pageSize + ", keyValueSizeCap:" + keyValueSizeCap + "]";
}

private String asString( PageCursor cursor, boolean includeValue, long stableGeneration, long unstableGeneration )
private String asString( PageCursor cursor, boolean includeValue, boolean includeAllocSpace,
long stableGeneration, long unstableGeneration )
{
int currentOffset = cursor.getOffset();
// [header] <- dont care
Expand All @@ -1049,6 +1066,13 @@ private String asString( PageCursor cursor, boolean includeValue, long stableGen
// OFFSET ARRAY
String offsetArray = readOffsetArray( cursor, stableGeneration, unstableGeneration, type );

// ALLOC SPACE
String allocSpace = "";
if ( includeAllocSpace )
{
allocSpace = readAllocSpace( cursor, allocOffset, type );
}

// KEYS
KEY readKey = layout.newKey();
VALUE readValue = layout.newValue();
Expand Down Expand Up @@ -1092,14 +1116,32 @@ private String asString( PageCursor cursor, boolean includeValue, long stableGen
}

cursor.setOffset( currentOffset );
return additionalHeader + offsetArray + " " + keys;
return additionalHeader + offsetArray + " " + allocSpace + " " + keys;
}

@SuppressWarnings( "unused" )
@Override
void printNode( PageCursor cursor, boolean includeValue, long stableGeneration, long unstableGeneration )
void printNode( PageCursor cursor, boolean includeValue, boolean includeAllocSpace, long stableGeneration, long unstableGeneration )
{
System.out.println( asString( cursor, includeValue, stableGeneration, unstableGeneration ) );
System.out.println( asString( cursor, includeValue, includeAllocSpace, stableGeneration, unstableGeneration ) );
}

private String readAllocSpace( PageCursor cursor, int allocOffset, Type type )
{
int keyCount = keyCount( cursor );
int endOfOffsetArray = type == INTERNAL ? keyPosOffsetInternal( keyCount ) : keyPosOffsetLeaf( keyCount );
cursor.setOffset( endOfOffsetArray );
int bytesToRead = allocOffset - endOfOffsetArray;
byte[] allocSpace = new byte[bytesToRead];
cursor.getBytes( allocSpace );
for ( byte b : allocSpace )
{
if ( b != 0 )
{
return "v" + endOfOffsetArray + ">" + bytesToRead + "|" + Arrays.toString( allocSpace );
}
}
return "v" + endOfOffsetArray + ">" + bytesToRead + "|[0...]";
}

private String readOffsetArray( PageCursor cursor, long stableGeneration, long unstableGeneration, Type type )
Expand Down

0 comments on commit def4e5f

Please sign in to comment.