Skip to content

Commit

Permalink
More retry-resilient StringSchemaKey compare
Browse files Browse the repository at this point in the history
As well as deduplication of some common native schema key initialization
code reseting flags and entityId.
  • Loading branch information
tinwelint committed Mar 26, 2018
1 parent f88d6e8 commit e5415d9
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 32 deletions.
Expand Up @@ -30,7 +30,7 @@
* {@link ValueMerger} which will merely detect conflict, not change any value if conflict, i.e. if the
* key already exists. After this merge has been used in a call to {@link Writer#merge(Object, Object, ValueMerger)}
* {@link #checkConflict(Value[])} can be called to check whether or not that call conflicted with
* an existing key. A call to {@link #checkConflict(Value[])} will also clear the conflict flag.
* an existing key. A call to {@link #checkConflict(Value[])} will also initialize the conflict flag.
*
* @param <VALUE> type of values being merged.
*/
Expand Down
Expand Up @@ -30,7 +30,7 @@
*/
abstract class NativeSchemaKey<SELF extends NativeSchemaKey> extends ValueWriter.Adapter<RuntimeException>
{
static final boolean DEFAULT_COMPARE_ID = true;
private static final boolean DEFAULT_COMPARE_ID = true;

private long entityId;
private boolean compareId = DEFAULT_COMPARE_ID;
Expand Down Expand Up @@ -64,12 +64,23 @@ void setEntityId( long entityId )

final void from( long entityId, Value... values )
{
compareId = DEFAULT_COMPARE_ID;
this.entityId = entityId;
initialize( entityId );
// copy value state and store in this key instance
assertValidValue( values ).writeTo( this );
}

/**
* Initializes this key with entity id and resets other flags to default values.
* Doesn't touch value data.
*
* @param entityId entity id to set for this key.
*/
void initialize( long entityId )
{
this.compareId = DEFAULT_COMPARE_ID;
this.entityId = entityId;
}

private Value assertValidValue( Value... values )
{
if ( values.length > 1 )
Expand All @@ -94,17 +105,15 @@ String propertiesAsString()

final void initAsLowest()
{
compareId = DEFAULT_COMPARE_ID;
entityId = Long.MIN_VALUE;
initialize( Long.MIN_VALUE );
initValueAsLowest();
}

abstract void initValueAsLowest();

final void initAsHighest()
{
compareId = DEFAULT_COMPARE_ID;
entityId = Long.MAX_VALUE;
initialize( Long.MAX_VALUE );
initValueAsHighest();
}

Expand Down
Expand Up @@ -81,8 +81,7 @@ void initValueAsHighest()
public void fromDerivedValue( long entityId, long derivedValue )
{
rawValueBits = derivedValue;
setEntityId( entityId );
setCompareId( DEFAULT_COMPARE_ID );
initialize( entityId );
}

/**
Expand Down
Expand Up @@ -27,7 +27,6 @@
import org.neo4j.values.storable.Values;

import static java.lang.String.format;
import static org.neo4j.values.storable.UTF8StringValue.codePointByteArrayCompare;

/**
* Includes value and entity id (to be able to handle non-unique values). A value can be any {@link String},
Expand All @@ -39,8 +38,6 @@ class StringSchemaKey extends NativeSchemaKey<StringSchemaKey>

private boolean ignoreLength;

// TODO something better or?
// TODO this is UTF-8 bytes for now
byte[] bytes;

int size()
Expand All @@ -59,6 +56,13 @@ protected Value assertCorrectType( Value value )
return value;
}

@Override
void initialize( long entityId )
{
super.initialize( entityId );
ignoreLength = false;
}

@Override
public Value asValue()
{
Expand All @@ -80,15 +84,15 @@ void initValueAsHighest()
void initAsPrefixLow( String prefix )
{
writeString( prefix );
setEntityId( Long.MIN_VALUE );
setCompareId( DEFAULT_COMPARE_ID );
initialize( Long.MIN_VALUE );
// Don't set ignoreLength = true here since the "low" a.k.a. left side of the range should care about length.
// This will make the prefix lower than those that matches the prefix (their length is >= that of the prefix)
}

void initAsPrefixHigh( String prefix )
{
writeString( prefix );
setEntityId( Long.MAX_VALUE );
setCompareId( DEFAULT_COMPARE_ID );
initialize( Long.MAX_VALUE );
ignoreLength = true;
}

Expand All @@ -107,7 +111,6 @@ private boolean isHighest()
@Override
int compareValueTo( StringSchemaKey other )
{
// TODO cover all cases of bytes == null and special tie breaker and document
if ( bytes != other.bytes )
{
if ( bytes == null )
Expand All @@ -124,20 +127,10 @@ int compareValueTo( StringSchemaKey other )
return 0;
}

try
{
// TODO change to not throw
return codePointByteArrayCompare( bytes, other.bytes, ignoreLength || other.ignoreLength );
}
catch ( Exception e )
{
// We can not throw here because we will visit this method inside a pageCursor.shouldRetry() block.
// Just return a comparison that at least will be commutative.
return byteArrayCompare( bytes, other.bytes );
}
return byteArrayCompare( bytes, other.bytes, ignoreLength | other.ignoreLength );
}

private static int byteArrayCompare( byte[] a, byte[] b )
private static int byteArrayCompare( byte[] a, byte[] b, boolean ignoreLength )
{
assert a != null && b != null : "Null arrays not supported.";

Expand All @@ -149,14 +142,14 @@ private static int byteArrayCompare( byte[] a, byte[] b )
int length = Math.min( a.length, b.length );
for ( int i = 0; i < length; i++ )
{
int compare = Byte.compare( a[i], b[i] );
int compare = Short.compare( (short) (a[i] & 0xFF), (short) (b[i] & 0xFF) );
if ( compare != 0 )
{
return compare;
}
}

return Integer.compare( a.length, b.length );
return ignoreLength ? 0 : Integer.compare( a.length, b.length );
}

@Override
Expand Down

0 comments on commit e5415d9

Please sign in to comment.