Skip to content

Commit

Permalink
Explicitly reserve key bit for future use
Browse files Browse the repository at this point in the history 
One of the bits in the key/value header is now explicitly reserved
for future use. This reduces max key size to 4905, but in reality
this limit is even lower anyway.
  • Loading branch information
@tinwelint
tinwelint committed Mar 7, 2018
1 parent 9a3c928 commit 6d9fbd2
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 5 deletions.
22 changes: 19 additions & 3 deletions community/index/src/main/java/org/neo4j/index/internal/gbptree/DynamicSizeUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.neo4j.io.pagecache.PageCursor;

import static java.lang.String.format;
import static org.neo4j.index.internal.gbptree.PageCursorUtil.getUnsignedShort;
import static org.neo4j.index.internal.gbptree.PageCursorUtil.putUnsignedShort;

Expand Down Expand Up @@ -60,16 +61,19 @@
* [0,0,1,k,k,k,k,k][1,v,v,v,v,v,v,v][v,v,v,v,v,v,v,v]
*
* Two byte key, no value
* [0,1,0,k,k,k,k,k][k,k,k,k,k,k,k,k]
* [0,1,0,k,k,k,k,k][0,k,k,k,k,k,k,k]
*
* Two byte key, one byte value
* [0,1,1,k,k,k,k,k][k,k,k,k,k,k,k,k][0,v,v,v,v,v,v,v]
* [0,1,1,k,k,k,k,k][0,k,k,k,k,k,k,k][0,v,v,v,v,v,v,v]
*
* Two byte key, two byte value
* [0,1,1,k,k,k,k,k][k,k,k,k,k,k,k,k][1,v,v,v,v,v,v,v][v,v,v,v,v,v,v,v]
* [0,1,1,k,k,k,k,k][0,k,k,k,k,k,k,k][1,v,v,v,v,v,v,v][v,v,v,v,v,v,v,v]
* </pre>
* This key/value size format is used, both for leaves and internal nodes even though internal nodes can never have values.
*
* The most significant key bit in the second byte (shown as 0) is not needed for the discrete key sizes for our 8k page size
* and is to be considered reserved for future use.
*
* Relative layout of key and key_value
* KeyOffset points to the exact offset where key entry or key_value entry
* can be read.
Expand All @@ -89,7 +93,9 @@ class DynamicSizeUtil
private static final int FLAG_FIRST_BYTE_TOMBSTONE = 0x80;
private static final long FLAG_READ_TOMBSTONE = 0x80000000_00000000L;
static final int MASK_ONE_BYTE_KEY_SIZE = 0x1F;
static final int MASK_TWO_BYTE_KEY_SIZE = 0xFFF;
static final int MASK_ONE_BYTE_VALUE_SIZE = 0x7F;
static final int MASK_TWO_BYTE_VALUE_SIZE = 0x7FFF;
private static final int FLAG_HAS_VALUE_SIZE = 0x20;
private static final int FLAG_ADDITIONAL_KEY_SIZE = 0x40;
private static final int FLAG_ADDITIONAL_VALUE_SIZE = 0x80;
Expand Down Expand Up @@ -122,6 +128,11 @@ static void putKeyValueSize( PageCursor cursor, int keySize, int valueSize )
if ( hasAdditionalKeySize )
{
firstByte |= FLAG_ADDITIONAL_KEY_SIZE;
if ( keySize > MASK_TWO_BYTE_KEY_SIZE )
{
throw new IllegalArgumentException(
format( "Max supported key size is %d, but tried to store key of size %d", MASK_TWO_BYTE_KEY_SIZE, keySize ) );
}
}
if ( hasValueSize )
{
Expand All @@ -144,6 +155,11 @@ static void putKeyValueSize( PageCursor cursor, int keySize, int valueSize )
byte firstByte = (byte) (valueSize & MASK_ONE_BYTE_VALUE_SIZE); // Least significant 7 bits
if ( needsAdditionalValueSize )
{
if ( valueSize > MASK_TWO_BYTE_VALUE_SIZE )
{
throw new IllegalArgumentException(
format( "Max supported value size is %d, but tried to store value of size %d", MASK_TWO_BYTE_VALUE_SIZE, valueSize ) );
}
firstByte |= FLAG_ADDITIONAL_VALUE_SIZE;
}
cursor.putByte( firstByte );
Expand Down
48 changes: 46 additions & 2 deletions community/index/src/test/java/org/neo4j/index/internal/gbptree/DynamicSizeUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,17 @@

import static org.junit.Assert.assertEquals;

import static org.junit.Assert.fail;
import static org.neo4j.index.internal.gbptree.DynamicSizeUtil.extractKeySize;
import static org.neo4j.index.internal.gbptree.DynamicSizeUtil.extractValueSize;
import static org.neo4j.index.internal.gbptree.DynamicSizeUtil.putKeyValueSize;
import static org.neo4j.index.internal.gbptree.DynamicSizeUtil.readKeyValueSize;

public class DynamicSizeUtilTest
{
private static final int KEY_ONE_BYTE_MAX = 0x1F;
private static final int KEY_TWO_BYTE_MIN = KEY_ONE_BYTE_MAX + 1;
private static final int KEY_TWO_BYTE_MAX = 0x1FFF;
private static final int KEY_TWO_BYTE_MAX = 0xFFF;
private static final int VAL_ONE_BYTE_MIN = 1;
private static final int VAL_ONE_BYTE_MAX = 0x7F;
private static final int VAL_TWO_BYTE_MIN = VAL_ONE_BYTE_MAX + 1;
Expand Down Expand Up @@ -92,6 +94,48 @@ public void shouldPutAndGetKeySize() throws Exception
shouldPutAndGetKeySize( KEY_TWO_BYTE_MAX, 2 );
}

@Test
public void shouldPreventWritingKeyLargerThanMaxPossible() throws Exception
{
// given
int keySize = 0xFFF;

// when
try
{
putKeyValueSize( cursor, keySize + 1, 0 );
fail( "Expected failure" );
}
catch ( IllegalArgumentException e )
{
// then good
}

// whereas when size is one less than that
shouldPutAndGetKeyValueSize( keySize, 0, 2 );
}

@Test
public void shouldPreventWritingValueLargerThanMaxPossible() throws Exception
{
// given
int valueSize = 0x7FFF;

// when
try
{
putKeyValueSize( cursor, 1, valueSize + 1 );
fail( "Expected failure" );
}
catch ( IllegalArgumentException e )
{
// then good
}

// whereas when size is one less than that
shouldPutAndGetKeyValueSize( 1, valueSize, 3 );
}

private void shouldPutAndGetKeySize( int keySize, int expectedBytes )
{
int size = putAndGetKey( keySize );
Expand All @@ -118,7 +162,7 @@ private void shouldPutAndGetKeyValueSize( int keySize, int valueSize, int expect
private int putAndGetKeyValue( int keySize, int valueSize )
{
int offsetBefore = cursor.getOffset();
DynamicSizeUtil.putKeyValueSize( cursor, keySize, valueSize );
putKeyValueSize( cursor, keySize, valueSize );
int offsetAfter = cursor.getOffset();
cursor.setOffset( offsetBefore );
long readKeyValueSize = readKeyValueSize( cursor );
Expand Down

0 comments on commit 6d9fbd2

Please sign in to comment.