diff --git a/core/src/main/java/com/yahoo/oak/BasicChunk.java b/core/src/main/java/com/yahoo/oak/BasicChunk.java index 33d3e99e..79538223 100644 --- a/core/src/main/java/com/yahoo/oak/BasicChunk.java +++ b/core/src/main/java/com/yahoo/oak/BasicChunk.java @@ -66,7 +66,10 @@ protected void updateBasicChild(BasicChunk child) { abstract boolean readValueFromEntryIndex(ValueBuffer value, int ei); - abstract void lookUp(ThreadContext ctx, K key); + /* the lookUp method throws this exception for OrderedChunks only while trying to access a released chunk. + * the lookUp method is not allowed to throw the exception in the case of HashChunk. + */ + abstract void lookUp(ThreadContext ctx, K key) throws DeletedMemoryAccessException; /*-------------- Publishing related methods and getters ---------------*/ /** diff --git a/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java b/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java index 2f94ce1d..a0924dec 100644 --- a/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java +++ b/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java @@ -6,7 +6,11 @@ package com.yahoo.oak; -public class DeletedMemoryAccessException extends RuntimeException { +/** + * This Exception is thrown when using the Nova MemoryManager, such exception indicates that we are trying + * to access some off heap memory location that was deleted concurrently to the access. + */ +public class DeletedMemoryAccessException extends Exception { public DeletedMemoryAccessException() { super(); } diff --git a/core/src/main/java/com/yahoo/oak/EntryArray.java b/core/src/main/java/com/yahoo/oak/EntryArray.java index a33e5351..f549edfb 100644 --- a/core/src/main/java/com/yahoo/oak/EntryArray.java +++ b/core/src/main/java/com/yahoo/oak/EntryArray.java @@ -182,9 +182,9 @@ int getNextNonZeroIndex(int currentIndex) { int nextIndex = currentIndex; while (!isNotZero && isIndexInBound(++nextIndex)) { - isNotZero = getEntryFieldLong(nextIndex, KEY_REF_OFFSET) != 0 || - getEntryFieldLong(nextIndex, VALUE_REF_OFFSET) != 0 || - getEntryFieldLong(nextIndex, OPT_FIELD_OFFSET) != 0; + isNotZero = array.getEntryFieldLong(nextIndex, KEY_REF_OFFSET) != 0 || + array.getEntryFieldLong(nextIndex, VALUE_REF_OFFSET) != 0 || + array.getEntryFieldLong(nextIndex, OPT_FIELD_OFFSET) != 0; } if (isIndexInBound(nextIndex)) { return nextIndex; diff --git a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java index 7489ce8b..59309e09 100644 --- a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java +++ b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java @@ -297,9 +297,13 @@ boolean isEntrySetValidAfterRebalance() { return true; } + //This method is called by only one thread on a release chunk. void releaseAllDeletedKeys() { KeyBuffer key = new KeyBuffer(config.keysMemoryManager.getEmptySlice()); - for (int i = 0; i < numOfEntries.get() ; i++) { + for (int i = 0; i < nextFreeIndex.get() - 1 && i < numOfEntries.get() ; i++) { + //the second condition is for case where multiple threads increment the nextfreeIndex + //which results in being bigger than the array size, this happens when multiple threads try + // to increase the nextFreeIndex if (!config.valuesMemoryManager.isReferenceDeleted(getValueReference(i))) { continue; } diff --git a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java index 2db21b62..a2e2288d 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java @@ -165,8 +165,16 @@ V replace(K key, V value, OakTransformer valueDeserializeTransformer) { ThreadContext ctx = getThreadContext(); for (int i = 0; i < MAX_RETRIES; i++) { - BasicChunk chunk = findChunk(key, ctx); // find orderedChunk matching key - chunk.lookUp(ctx, key); + BasicChunk chunk = findChunk(key, ctx); // find Chunk matching key + try { + chunk.lookUp(ctx, key); + //the look up method might encounter a chunk which is released, while using Nova as a memory manager + //as a result we might access an already deleted key, thus the need to catch the exception + } catch (DeletedMemoryAccessException e) { + assert !(chunk instanceof HashChunk); + //Hash deals with deleted keys in an earlier stage + continue; + } if (!ctx.isValueValid()) { return null; } @@ -184,29 +192,7 @@ V replace(K key, V value, OakTransformer valueDeserializeTransformer) { throw new RuntimeException("replace failed: reached retry limit (1024)."); } - boolean replace(K key, V oldValue, V newValue, OakTransformer valueDeserializeTransformer) { - ThreadContext ctx = getThreadContext(); - - for (int i = 0; i < MAX_RETRIES; i++) { - // find chunk matching key, puts this key hash into ctx.operationKeyHash - BasicChunk c = findChunk(key, ctx); // find orderedChunk matching key - c.lookUp(ctx, key); - if (!ctx.isValueValid()) { - return false; - } - - ValueUtils.ValueResult res = getValueOperator().compareExchange(c, ctx, oldValue, newValue, - valueDeserializeTransformer, getValueSerializer()); - if (res == ValueUtils.ValueResult.RETRY) { - // it might be that this chunk is proceeding with rebalance -> help - helpRebalanceIfInProgress(c); - continue; - } - return res == ValueUtils.ValueResult.TRUE; - } - - throw new RuntimeException("replace failed: reached retry limit (1024)."); - } + abstract boolean replace(K key, V oldValue, V newValue, OakTransformer valueDeserializeTransformer); abstract boolean putIfAbsentComputeIfPresent(K key, V value, Consumer computer); @@ -355,6 +341,9 @@ public boolean hasNext() { return (state != null); } + // for more detail on this method see implementation + protected abstract void initStateWithMinKey(BasicChunk chunk); + protected abstract void initAfterRebalance(); // the actual next() @@ -471,7 +460,8 @@ protected void invalidatePrevState() { protected abstract BasicChunk getNextChunk(BasicChunk current); - protected abstract BasicChunk.BasicChunkIter getChunkIter(BasicChunk current); + protected abstract BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) + throws DeletedMemoryAccessException; /** * advance state to the new position @@ -483,7 +473,7 @@ protected boolean advanceState() { BasicChunk.BasicChunkIter chunkIter = getState().getChunkIter(); updatePreviousState(); - + while (!chunkIter.hasNext()) { // skip empty chunks chunk = getNextChunk(chunk); if (chunk == null) { @@ -491,13 +481,19 @@ protected boolean advanceState() { setState(null); return false; } - chunkIter = getChunkIter(chunk); + try { + chunkIter = getChunkIter(chunk); + } catch (DeletedMemoryAccessException e) { + assert chunk.state.get() == BasicChunk.State.RELEASED; + initStateWithMinKey(chunk); + return true; + } } int nextIndex = chunkIter.next(ctx); getState().set(chunk, chunkIter, nextIndex); - return true; + return true; } } diff --git a/core/src/main/java/com/yahoo/oak/InternalOakHash.java b/core/src/main/java/com/yahoo/oak/InternalOakHash.java index a5db61df..15457d43 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakHash.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakHash.java @@ -268,7 +268,7 @@ boolean replace(K key, V oldValue, V newValue, OakTransformer valueDeserializ for (int i = 0; i < MAX_RETRIES; i++) { // find chunk matching key, puts this key hash into ctx.operationKeyHash - BasicChunk c = findChunk(key, ctx); + HashChunk c = findChunk(key, ctx); c.lookUp(ctx, key); if (!ctx.isValueValid()) { return false; @@ -406,9 +406,8 @@ boolean computeIfPresent(K key, Consumer computer) { for (int i = 0; i < MAX_RETRIES; i++) { // find chunk matching key, puts this key hash into ctx.operationKeyHash - BasicChunk c = findChunk(key, ctx); + HashChunk c = findChunk(key, ctx); c.lookUp(ctx, key); - if (ctx.isValueValid()) { ValueUtils.ValueResult res = config.valueOperator.compute(ctx.value, computer); if (res == ValueUtils.ValueResult.TRUE) { @@ -440,9 +439,8 @@ Result remove(K key, V oldValue, OakTransformer transformer) { for (int i = 0; i < MAX_RETRIES; i++) { // find chunk matching key, puts this key hash into ctx.operationKeyHash - BasicChunk c = findChunk(key, ctx); + HashChunk c = findChunk(key, ctx); c.lookUp(ctx, key); - if (!ctx.isKeyValid()) { // There is no such key. If we did logical deletion and someone else did the physical deletion, // then the old value is saved in v. Otherwise v is (correctly) null @@ -569,6 +567,10 @@ protected void initAfterRebalance() { initState(); } + @Override + protected void initStateWithMinKey(BasicChunk c) { //nextKey is null here + initState(); + } /** * Advances next to higher entry. @@ -799,8 +801,9 @@ class EntryTransformIterator extends HashIter { } public T next() { + ValueUtils.ValueResult res; advance(true); - ValueUtils.ValueResult res = ctx.value.s.preRead(); + res = ctx.value.s.preRead(); if (res == ValueUtils.ValueResult.FALSE) { return next(); } else if (res == ValueUtils.ValueResult.RETRY) { @@ -817,7 +820,11 @@ public T next() { new AbstractMap.SimpleEntry<>(ctx.key, ctx.value); T transformation = transformer.apply(entry); - ctx.value.s.postRead(); + try { + ctx.value.s.postRead(); + } catch (DeletedMemoryAccessException e) { + return next(); + } return transformation; } } diff --git a/core/src/main/java/com/yahoo/oak/InternalOakMap.java b/core/src/main/java/com/yahoo/oak/InternalOakMap.java index 0cd5dd71..572bd310 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakMap.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakMap.java @@ -264,7 +264,8 @@ private void updateIndexAndNormalize( // returns false when restart is needed // (if rebalance happened or another valid entry with same key was found) - private boolean allocateAndLinkEntry(OrderedChunk c, ThreadContext ctx, K key, boolean isPutIfAbsent) { + private boolean allocateAndLinkEntry(OrderedChunk c, ThreadContext ctx, K key, boolean isPutIfAbsent) + throws DeletedMemoryAccessException { // There was no such key found, going to allocate a new key. // EntryOrderedSet allocates the entry (holding the key) and ctx is going to be updated // to be used by EntryOrderedSet's subsequent requests to write value @@ -815,7 +816,7 @@ boolean replace(K key, V oldValue, V newValue, OakTransformer valueDeserializ throw new RuntimeException("replace failed: reached retry limit (1024)."); } - Map.Entry lowerEntry(K key) { + Map.Entry lowerEntry(K key) { Map.Entry> lowerChunkEntry = skiplist.lowerEntry(key); if (lowerChunkEntry == null) { /* we were looking for the minimal key */ @@ -824,39 +825,48 @@ Map.Entry lowerEntry(K key) { ThreadContext ctx = getThreadContext(); - OrderedChunk c = lowerChunkEntry.getValue(); - /* Iterate orderedChunk to find prev(key), no upper limit */ - OrderedChunk.AscendingIter chunkIter = c.ascendingIter(ctx, null, false, null); - int prevIndex = chunkIter.next(ctx); + for (int i = 0; i < InternalOakBasics.MAX_RETRIES; i++) { + try { - while (chunkIter.hasNext()) { - int nextIndex = chunkIter.next(ctx); - if (c.compareKeyAndEntryIndex(ctx.tempKey, key, nextIndex) <= 0) { - break; + OrderedChunk c = lowerChunkEntry.getValue(); + /* Iterate orderedChunk to find prev(key), no upper limit */ + OrderedChunk.AscendingIter chunkIter = c.ascendingIter(ctx, null, false, null); + int prevIndex = chunkIter.next(ctx); + + while (chunkIter.hasNext()) { + int nextIndex = chunkIter.next(ctx); + if (c.compareKeyAndEntryIndex(ctx.tempKey, key, nextIndex) <= 0) { + break; + } + prevIndex = nextIndex; + } + + /* Edge case: we're looking for the lowest key in the map and it's still greater than minkey + (in which case prevKey == key) */ + if (c.compareKeyAndEntryIndex(ctx.tempKey, key, prevIndex) == 0) { + return new AbstractMap.SimpleImmutableEntry<>(null, null); + } + // ctx.tempKey was updated with prevIndex key as a side effect of compareKeyAndEntryIndex() + K keyDeserialized = getKeySerializer().deserialize(ctx.tempKey); + + // get value associated with this (prev) key + boolean isAllocated = c.readValueFromEntryIndex(ctx.value, prevIndex); + if (!isAllocated) { // value reference was invalid, try again + return lowerEntry(key); + } + + Result valueDeserialized = config.valueOperator.transform(ctx.result, ctx.value, + getValueSerializer()::deserialize); + if (valueDeserialized.operationResult != ValueUtils.ValueResult.TRUE) { + return lowerEntry(key); + } + return new AbstractMap.SimpleImmutableEntry<>(keyDeserialized, (V) valueDeserialized.value); + } catch (DeletedMemoryAccessException e) { + continue; } - prevIndex = nextIndex; - } - - /* Edge case: we're looking for the lowest key in the map and it's still greater than minkey - (in which case prevKey == key) */ - if (c.compareKeyAndEntryIndex(ctx.tempKey, key, prevIndex) == 0) { - return new AbstractMap.SimpleImmutableEntry<>(null, null); } - // ctx.tempKey was updated with prevIndex key as a side effect of compareKeyAndEntryIndex() - K keyDeserialized = getKeySerializer().deserialize(ctx.tempKey); - // get value associated with this (prev) key - boolean isAllocated = c.readValueFromEntryIndex(ctx.value, prevIndex); - if (!isAllocated) { // value reference was invalid, try again - return lowerEntry(key); - } - - Result valueDeserialized = config.valueOperator.transform(ctx.result, ctx.value, - getValueSerializer()::deserialize); - if (valueDeserialized.operationResult != ValueUtils.ValueResult.TRUE) { - return lowerEntry(key); - } - return new AbstractMap.SimpleImmutableEntry<>(keyDeserialized, (V) valueDeserialized.value); + throw new RuntimeException("replace failed: reached retry limit (1024)."); } /*-------------- Iterators --------------*/ @@ -913,7 +923,7 @@ abstract class OrderedIter extends BasicIter { initState(isDescending, lo, loInclusive, hi, hiInclusive); } - private boolean tooLow(OakScopedReadBuffer key) { + private boolean tooLow(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (lo == null) { return false; } @@ -922,7 +932,7 @@ private boolean tooLow(OakScopedReadBuffer key) { return c > 0 || (c == 0 && !loInclusive); } - private boolean tooHigh(OakScopedReadBuffer key) { + private boolean tooHigh(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (hi == null) { return false; } @@ -931,7 +941,7 @@ private boolean tooHigh(OakScopedReadBuffer key) { } - private boolean inBounds(OakScopedReadBuffer key) { + private boolean inBounds(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (!isDescending) { return !tooHigh(key); } else { @@ -971,12 +981,35 @@ protected void initAfterRebalance() { // Update the state to point to last returned key. initState(isDescending, lo, loInclusive, hi, hiInclusive); } + + //The same as initState, but always works on a released chunk minKey which is never deleted. + @Override + protected void initStateWithMinKey(BasicChunk chunk) { + OrderedChunk oChunk = (OrderedChunk ) chunk; + K nextKey = null; + try { + nextKey = KeyUtils.deSerializedKey(oChunk.minKey, getKeySerializer()); + } catch (DeletedMemoryAccessException e) { + assert e == null; //since we are not deleting minKeys (should not get here!) + return ; + } + if (isDescending) { + hiInclusive = true; + hi = nextKey; + } else { + loInclusive = true; + lo = nextKey; + } + + // Update the state to point to last returned key. + initState(isDescending, lo, loInclusive, hi, hiInclusive); + } + /** * Advances next to higher entry. * Return previous index * - * */ @Override void advance(boolean needsValue) { @@ -1067,52 +1100,63 @@ protected void initState(boolean isDescending, K lowerBound, boolean lowerInclus OrderedChunk.ChunkIter nextChunkIter; OrderedChunk nextOrderedChunk; - - if (!isDescending) { - if (lowerBound != null) { - nextOrderedChunk = skiplist.floorEntry(lowerBound).getValue(); - } else { - nextOrderedChunk = skiplist.firstEntry().getValue(); - // need to iterate from the beginning of the orderedChunk till the end - } - if (nextOrderedChunk != null) { - OakScopedReadBuffer upperBoundKeyForChunk = getNextChunkMinKey(nextOrderedChunk); - nextChunkIter = lowerBound != null ? - nextOrderedChunk.ascendingIter(ctx, lowerBound, lowerInclusive, upperBound, upperInclusive, - upperBoundKeyForChunk) - : nextOrderedChunk - .ascendingIter(ctx, upperBound, upperInclusive, upperBoundKeyForChunk); - } else { - setState(null); - return; - } - } else { - nextOrderedChunk = upperBound != null ? skiplist.floorEntry(upperBound).getValue() - : skiplist.lastEntry().getValue(); - if (nextOrderedChunk != null) { - nextChunkIter = upperBound != null ? - nextOrderedChunk - .descendingIter(ctx, upperBound, upperInclusive, lowerBound, lowerInclusive) - : nextOrderedChunk.descendingIter(ctx, lowerBound, lowerInclusive); - } else { - setState(null); - return; + + for (int i = 0; i < MAX_RETRIES; i++) { + try { + if (!isDescending) { + if (lowerBound != null) { + nextOrderedChunk = skiplist.floorEntry(lowerBound).getValue(); + } else { + nextOrderedChunk = skiplist.firstEntry().getValue(); + // need to iterate from the beginning of the orderedChunk till the end + } + if (nextOrderedChunk != null) { + OakScopedReadBuffer upperBoundKeyForChunk = getNextChunkMinKey(nextOrderedChunk); + nextChunkIter = lowerBound != null ? + nextOrderedChunk.ascendingIter + (ctx, lowerBound, lowerInclusive, upperBound, upperInclusive, upperBoundKeyForChunk) + : nextOrderedChunk + .ascendingIter(ctx, upperBound, upperInclusive, upperBoundKeyForChunk); + } else { + setState(null); + return; + } + } else { + nextOrderedChunk = upperBound != null ? skiplist.floorEntry(upperBound).getValue() + : skiplist.lastEntry().getValue(); + if (nextOrderedChunk != null) { + nextChunkIter = upperBound != null ? + nextOrderedChunk + .descendingIter(ctx, upperBound, upperInclusive, lowerBound, lowerInclusive) + : nextOrderedChunk.descendingIter(ctx, lowerBound, lowerInclusive); + } else { + setState(null); + return; + } + } + + //Init state, not valid yet, must move forward + setPrevState(InternalOakMap.IteratorState.newInstance(null, null)); + setState(IteratorState.newInstance(nextOrderedChunk, nextChunkIter)); + advanceState(); + } catch (DeletedMemoryAccessException e) { + assert e == null; + //The exception here is needed as the ascendingIter/descendingIter signature throws the + // exception in case that the key is deleted, this could not happen here since we are dealing with + // minKey which we never delete even when using memory manager that deleted keys } + return; } - - //Init state, not valid yet, must move forward - setPrevState(InternalOakMap.IteratorState.newInstance(null, null)); - setState(IteratorState.newInstance(nextOrderedChunk, nextChunkIter)); - advanceState(); + throw new RuntimeException("reached retry limit (1024)."); } @Override protected BasicChunk getNextChunk(BasicChunk current) { - OrderedChunk currentHashChunk = (OrderedChunk) current; + OrderedChunk currentChunk = (OrderedChunk) current; if (!isDescending) { - return currentHashChunk.next.getReference(); + return currentChunk.next.getReference(); } else { - Map.Entry> entry = skiplist.lowerEntry(currentHashChunk.minKey); + Map.Entry> entry = skiplist.lowerEntry(currentChunk.minKey); if (entry == null) { return null; } else { @@ -1121,7 +1165,9 @@ protected BasicChunk getNextChunk(BasicChunk current) { } } - protected BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) { + //This method throws DeletedMemoryAccessException when checking if some deleted key, + // is in the boundary of some released chunk. + protected BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) throws DeletedMemoryAccessException { if (!isDescending) { OakScopedReadBuffer upperBoundKeyForChunk = getNextChunkMinKey((OrderedChunk) current); return ((OrderedChunk) current).ascendingIter(ctx, hi, hiInclusive, upperBoundKeyForChunk); @@ -1155,7 +1201,7 @@ private OakScopedReadBuffer getNextChunkMinKey(OrderedChunk c) { @Override protected boolean advanceState() { while (true) { - boolean valueToReturn = super.advanceState(); + boolean valueToReturn = super.advanceState(); //TODO changes names to continue or advnace if (valueToReturn) { BasicChunk chunk = getState().getChunk(); @@ -1167,9 +1213,13 @@ protected boolean advanceState() { if (!chunk.readKeyFromEntryIndex(ctx.tempKey, nextIndex)) { continue; } - if (!inBounds(ctx.tempKey)) { - setState(null); - valueToReturn = false; + try { + if (!inBounds(ctx.tempKey)) { + setState(null); + valueToReturn = false; + } + } catch (DeletedMemoryAccessException e) { + continue; } } } @@ -1297,8 +1347,9 @@ class EntryTransformIterator extends OrderedIter { } public T next() { + ValueUtils.ValueResult res; advance(true); - ValueUtils.ValueResult res = ctx.value.s.preRead(); + res = ctx.value.s.preRead(); if (res == ValueUtils.ValueResult.FALSE) { return next(); } else if (res == ValueUtils.ValueResult.RETRY) { @@ -1315,7 +1366,12 @@ public T next() { new AbstractMap.SimpleEntry<>(ctx.key, ctx.value); T transformation = transformer.apply(entry); - ctx.value.s.postRead(); + try { + ctx.value.s.postRead(); + } catch (DeletedMemoryAccessException e) { + return next(); + } + return transformation; } } diff --git a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java index bd7c4a0a..9c313edd 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java +++ b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java @@ -47,15 +47,23 @@ private static boolean atomicallySetDeleted(long headerAddress, long offHeapMeta } + /** this method needs to be called before accessing off-heap memory for reading when using the Nova memory manager + @return {@link ValueUtils.ValueResult} TRUE in case of the underlying memory is still valid to access, else FALSE. + */ ValueUtils.ValueResult preRead(final int onHeapVersion, long headerAddress) { long offHeapHeader = getOffHeapHeader(headerAddress); if (RC.isReferenceDeleted(offHeapHeader)) { - throw new DeletedMemoryAccessException(); + return ValueUtils.ValueResult.FALSE; + //throw new DeletedMemoryAccessException(); } return ValueUtils.ValueResult.TRUE; } - ValueUtils.ValueResult postRead(final int onHeapVersion, long headerAddress) { + /** this method needs to be called after accessing off-heap memory for reading using the Nova memory manager + @return {@link ValueUtils.ValueResult} TRUE in case of the underlying memory is still valid to access. + @throws DeletedMemoryAccessException in case of the underlying memory is deleted. + */ + ValueUtils.ValueResult postRead(final int onHeapVersion, long headerAddress) throws DeletedMemoryAccessException { DirectUtils.UNSAFE.loadFence(); long offHeapHeader = getOffHeapHeader(headerAddress); int offHeapVersion = RC.getFirst(offHeapHeader); diff --git a/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java b/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java index 4fab7a7a..83ccb202 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java +++ b/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java @@ -203,8 +203,9 @@ public ValueUtils.ValueResult preRead() { * @return {@code TRUE} if the read lock was released successfully * {@code FALSE} if the value is marked as deleted * {@code RETRY} if the value was moved, or the version of the off-heap value does not match {@code version}. + * @throws DeletedMemoryAccessException */ - public ValueUtils.ValueResult postRead() { + public ValueUtils.ValueResult postRead() throws DeletedMemoryAccessException { assert version != ReferenceCodecSyncRecycle.INVALID_VERSION; return HEADER.postRead(version, getMetadataAddress()); } diff --git a/core/src/main/java/com/yahoo/oak/OakMap.java b/core/src/main/java/com/yahoo/oak/OakMap.java index 2d08387a..6bb20a21 100644 --- a/core/src/main/java/com/yahoo/oak/OakMap.java +++ b/core/src/main/java/com/yahoo/oak/OakMap.java @@ -271,8 +271,9 @@ public Entry lowerEntry(K key) { if (key == null) { throw new NullPointerException(); } - + return internalOakMap.lowerEntry(key); + } /** @@ -289,7 +290,6 @@ public K lowerKey(K key) { if (key == null) { throw new NullPointerException(); } - return internalOakMap.lowerEntry(key).getKey(); } diff --git a/core/src/main/java/com/yahoo/oak/OrderedChunk.java b/core/src/main/java/com/yahoo/oak/OrderedChunk.java index 10d97f27..30245ee9 100644 --- a/core/src/main/java/com/yahoo/oak/OrderedChunk.java +++ b/core/src/main/java/com/yahoo/oak/OrderedChunk.java @@ -26,7 +26,8 @@ class OrderedChunk extends BasicChunk { public static final int ORDERED_CHUNK_MAX_ITEMS_DEFAULT = 4096; /*-------------- Members --------------*/ - KeyBuffer minKey; // minimal key that can be put in this chunk + KeyBuffer minKey; // minimal key that can be put in this chunk. + //These keys are never released even when using Nova memory manager AtomicMarkableReference> next; private final EntryOrderedSet entryOrderedSet; @@ -231,8 +232,9 @@ private int getLastSortedEntryIndex(int sortedCount) { * @param key the key to compare * @param ei the entry index to compare with * @return the comparison result + * @throws DeletedMemoryAccessException */ - int compareKeyAndEntryIndex(KeyBuffer tempKeyBuff, K key, int ei) { + int compareKeyAndEntryIndex(KeyBuffer tempKeyBuff, K key, int ei) throws DeletedMemoryAccessException { boolean isAllocated = entryOrderedSet.readKey(tempKeyBuff, ei); if (!isAllocated) { throw new DeletedMemoryAccessException(); @@ -260,8 +262,9 @@ int compareKeyAndEntryIndex(KeyBuffer tempKeyBuff, K key, int ei) { * This means that there is an entry with that key, but there is no value attached to this key. * Such entry can be reused after finishing the deletion process, if needed. * @param key the key to look up + * @throws DeletedMemoryAccessException */ - void lookUp(ThreadContext ctx, K key) { + void lookUp(ThreadContext ctx, K key) throws DeletedMemoryAccessException { // binary search sorted part of key array to quickly find node to start search at // it finds previous-to-key int curr = binaryFind(ctx.tempKey, key); @@ -303,8 +306,9 @@ void lookUp(ThreadContext ctx, K key) { * (1) the given key is less or equal than the smallest key in the chunk OR * (2) entries are unsorted so there is a need to start from the beginning of the linked list * NONE_NEXT is going to be returned + * @throws DeletedMemoryAccessException */ - private int binaryFind(KeyBuffer tempKey, K key) { + private int binaryFind(KeyBuffer tempKey, K key) throws DeletedMemoryAccessException { int sortedCount = this.sortedCount.get(); // if there are no sorted keys, return NONE_NEXT to indicate that a regular linear search is needed if (sortedCount == 0) { @@ -380,8 +384,9 @@ boolean finalizeDeletion(ThreadContext ctx) { * @param key the key to link * @return The previous entry index if the key was already added by another thread. * Otherwise, if successful, it will return the current entry index. + * @throws DeletedMemoryAccessException */ - int linkEntry(ThreadContext ctx, K key) { + int linkEntry(ThreadContext ctx, K key) throws DeletedMemoryAccessException { int prev; int curr; int cmp; @@ -629,9 +634,10 @@ OrderedChunk markAndGetNext() { * Ascending iterator from the beginning of the chunk. The end boundary is given by parameter * key "to" might not be in this chunk. Parameter nextChunkMinKey - is the minimal key of the * next chunk, all current and future keys of this chunk are less than nextChunkMinKey + * @throws DeletedMemoryAccessException */ AscendingIter ascendingIter(ThreadContext ctx, K to, boolean toInclusive, - OakScopedReadBuffer nextChunkMinKey) { + OakScopedReadBuffer nextChunkMinKey) throws DeletedMemoryAccessException { return new AscendingIter(ctx, to, toInclusive, nextChunkMinKey); } @@ -640,26 +646,29 @@ AscendingIter ascendingIter(ThreadContext ctx, K to, boolean toInclusive, * The end boundary is given by parameter key "to", but it might not be in this chunk. * Parameter nextChunkMinKey - is the minimal key of the next chunk, * all current and future keys of this chunk are less than nextChunkMinKey + * @throws DeletedMemoryAccessException */ AscendingIter ascendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, - boolean toInclusive, OakScopedReadBuffer nextChunkMinKey) { + boolean toInclusive, OakScopedReadBuffer nextChunkMinKey) throws DeletedMemoryAccessException { return new AscendingIter(ctx, from, fromInclusive, to, toInclusive, nextChunkMinKey); } /** * Descending iterator from the end of the chunk. * The lower bound given by parameter key "to" might not be in this chunk + * @throws DeletedMemoryAccessException */ - DescendingIter descendingIter(ThreadContext ctx, K to, boolean toInclusive) { + DescendingIter descendingIter(ThreadContext ctx, K to, boolean toInclusive) throws DeletedMemoryAccessException { return new DescendingIter(ctx, to, toInclusive); } /** * Descending iterator from key "from" given as parameter. * The lower bound given by parameter key "to" might not be in this chunk + * @throws DeletedMemoryAccessException */ DescendingIter descendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, - boolean toInclusive) { + boolean toInclusive) throws DeletedMemoryAccessException { return new DescendingIter(ctx, from, fromInclusive, to, toInclusive); } @@ -690,10 +699,11 @@ boolean isBoundCheckNeeded() { ** meaning that scan is near to its end. ** For descending scan it is the low key, for ascending scan it is the high. **/ - abstract boolean isKeyOutOfEndBound(OakScopedReadBuffer boundKey); + abstract boolean isKeyOutOfEndBound(OakScopedReadBuffer boundKey) throws DeletedMemoryAccessException; protected void setIsEndBoundCheckNeeded( - ThreadContext ctx, K to, boolean toInclusive, OakScopedReadBuffer chunkBoundaryKey) { + ThreadContext ctx, K to, boolean toInclusive, OakScopedReadBuffer chunkBoundaryKey) + throws DeletedMemoryAccessException { this.endBound = to; this.endBoundInclusive = toInclusive; @@ -712,7 +722,9 @@ protected void setIsEndBoundCheckNeeded( // it is caught when traversing to the start bound and midIdx is set to -1 // is the key in the middle index already above the upper limit to stop on? - readKeyFromEntryIndex(ctx.tempKey, midIdx); + if (!readKeyFromEntryIndex(ctx.tempKey, midIdx)) { + throw new DeletedMemoryAccessException(); + } if (!isKeyOutOfEndBound(ctx.tempKey)) { isEndBoundCheckNeeded = IterEndBoundCheck.MID_END_BOUNDARY_CHECK; } @@ -726,14 +738,14 @@ protected void setIsEndBoundCheckNeeded( class AscendingIter extends ChunkIter { AscendingIter(ThreadContext ctx, K to, boolean toInclusive, - OakScopedReadBuffer nextChunkMinKey) { + OakScopedReadBuffer nextChunkMinKey) throws DeletedMemoryAccessException { next = entryOrderedSet.getHeadNextEntryIndex(); next = advanceNextIndexNoBound(next, ctx); setIsEndBoundCheckNeeded(ctx, to, toInclusive, nextChunkMinKey); } AscendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, boolean toInclusive, - OakScopedReadBuffer nextChunkMinKey) { + OakScopedReadBuffer nextChunkMinKey) throws DeletedMemoryAccessException { KeyBuffer tempKeyBuff = ctx.tempKey; next = binaryFind(tempKeyBuff, from); @@ -808,7 +820,7 @@ private int advanceNextIndexNoBound(final int entryIndex, ThreadContext ctx) { } @Override - protected boolean isKeyOutOfEndBound(OakScopedReadBuffer key) { + protected boolean isKeyOutOfEndBound(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (endBound == null) { return false; } @@ -830,7 +842,7 @@ class DescendingIter extends ChunkIter { private boolean fromInclusive; private final int skipEntriesForBiggerStack = Math.max(1, getMaxItems() / 10); // 1 is the lowest possible value - DescendingIter(ThreadContext ctx, K to, boolean toInclusive) { + DescendingIter(ThreadContext ctx, K to, boolean toInclusive) throws DeletedMemoryAccessException { KeyBuffer tempKeyBuff = ctx.tempKey; setIsEndBoundCheckNeeded(ctx, to, toInclusive, minKey); from = null; @@ -842,7 +854,8 @@ class DescendingIter extends ChunkIter { initNext(tempKeyBuff); } - DescendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, boolean toInclusive) { + DescendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, boolean toInclusive) + throws DeletedMemoryAccessException { KeyBuffer tempKeyBuff = ctx.tempKey; this.from = from; @@ -863,7 +876,7 @@ class DescendingIter extends ChunkIter { setIsEndBoundCheckNeeded(ctx, to, toInclusive, minKey); } - private void initNext(KeyBuffer keyBuff) { + private void initNext(KeyBuffer keyBuff) throws DeletedMemoryAccessException { traverseLinkedList(keyBuff, true); advance(keyBuff); } @@ -907,8 +920,10 @@ private void pushToStack(boolean compareWithPrevAnchor) { * fill the stack * * @param firstTimeInvocation + * @throws DeletedMemoryAccessException */ - private void traverseLinkedList(KeyBuffer tempKeyBuff, boolean firstTimeInvocation) { + private void traverseLinkedList(KeyBuffer tempKeyBuff, boolean firstTimeInvocation) + throws DeletedMemoryAccessException { assert stack.size() == 1; // anchor is in the stack if (prevAnchor == entryOrderedSet.getNextEntryIndex(anchor)) { next = NONE_NEXT; // there is no next; @@ -977,7 +992,11 @@ private void advance(KeyBuffer keyBuff) { return; } findNewAnchor(); - traverseLinkedList(keyBuff, false); + try { + traverseLinkedList(keyBuff, false); + } catch (DeletedMemoryAccessException e) { + continue; + } } } @@ -994,7 +1013,7 @@ public int next(ThreadContext ctx) { } @Override - protected boolean isKeyOutOfEndBound(OakScopedReadBuffer key) { + protected boolean isKeyOutOfEndBound(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (endBound == null) { return false; } diff --git a/core/src/main/java/com/yahoo/oak/Slice.java b/core/src/main/java/com/yahoo/oak/Slice.java index 3d8e7f33..20024343 100644 --- a/core/src/main/java/com/yahoo/oak/Slice.java +++ b/core/src/main/java/com/yahoo/oak/Slice.java @@ -118,7 +118,7 @@ interface Slice { * {@code FALSE} if the value is marked as deleted * {@code RETRY} if the value was moved, or the version of the off-heap value does not match {@code version}. */ - ValueUtils.ValueResult postRead(); + ValueUtils.ValueResult postRead() throws DeletedMemoryAccessException; /** * Acquires a write lock diff --git a/core/src/main/java/com/yahoo/oak/SyncRecycleMemoryManager.java b/core/src/main/java/com/yahoo/oak/SyncRecycleMemoryManager.java index b2439e08..0eb3e974 100644 --- a/core/src/main/java/com/yahoo/oak/SyncRecycleMemoryManager.java +++ b/core/src/main/java/com/yahoo/oak/SyncRecycleMemoryManager.java @@ -391,6 +391,7 @@ public String toString() { * {@code FALSE} if the header/off-heap-cut is marked as deleted * {@code RETRY} if the header/off-heap-cut was moved, or the version of the off-heap header * does not match {@code version}. + * @throws DeletedMemoryAccessException */ public ValueUtils.ValueResult preRead() { assert version != ReferenceCodecSyncRecycle.INVALID_VERSION; @@ -403,8 +404,9 @@ public ValueUtils.ValueResult preRead() { * @return {@code TRUE} if the read lock was released successfully * {@code FALSE} if the value is marked as deleted * {@code RETRY} if the value was moved, or the version of the off-heap value does not match {@code version}. + * @throws DeletedMemoryAccessException */ - public ValueUtils.ValueResult postRead() { + public ValueUtils.ValueResult postRead() throws DeletedMemoryAccessException { assert version != ReferenceCodecSyncRecycle.INVALID_VERSION; return HEADER.unlockRead(version, getMetadataAddress()); } diff --git a/core/src/main/java/com/yahoo/oak/UnscopedValueBufferSynced.java b/core/src/main/java/com/yahoo/oak/UnscopedValueBufferSynced.java index 6f2c4c84..7e00e4c1 100644 --- a/core/src/main/java/com/yahoo/oak/UnscopedValueBufferSynced.java +++ b/core/src/main/java/com/yahoo/oak/UnscopedValueBufferSynced.java @@ -44,7 +44,11 @@ public T transform(OakTransformer transformer) { try { return transformer.apply(internalScopedReadBuffer); } finally { - end(); + try { + end(); + } catch (DeletedMemoryAccessException e) { + throw new RuntimeException(); + } } } @@ -56,7 +60,11 @@ protected R safeAccessToScopedBuffer(Getter getter, int index) { try { return getter.get(internalScopedReadBuffer, index); } finally { - end(); + try { + end(); + } catch (DeletedMemoryAccessException e) { + throw new RuntimeException(); + } } } @@ -78,7 +86,7 @@ private void start() { throw new RuntimeException("Op failed: reached retry limit (1024)."); } - private void end() { + private void end() throws DeletedMemoryAccessException { internalScopedReadBuffer.s.postRead(); } diff --git a/core/src/main/java/com/yahoo/oak/ValueUtils.java b/core/src/main/java/com/yahoo/oak/ValueUtils.java index 4b3426b0..d05db7d0 100644 --- a/core/src/main/java/com/yahoo/oak/ValueUtils.java +++ b/core/src/main/java/com/yahoo/oak/ValueUtils.java @@ -38,7 +38,11 @@ Result transform(Result result, ValueBuffer value, OakTransformer transfo T transformation = transformer.apply(value); return result.withValue(transformation); } finally { - value.s.postRead(); + try { + value.s.postRead(); + } catch (DeletedMemoryAccessException e) { + return result.withFlag(false); + } } } diff --git a/core/src/test/java/com/yahoo/oak/PutIfAbsentTest.java b/core/src/test/java/com/yahoo/oak/PutIfAbsentTest.java index 66003ffa..76bc1804 100644 --- a/core/src/test/java/com/yahoo/oak/PutIfAbsentTest.java +++ b/core/src/test/java/com/yahoo/oak/PutIfAbsentTest.java @@ -25,8 +25,8 @@ @RunWith(Parameterized.class) public class PutIfAbsentTest { - private static final int NUM_THREADS = 31; - private static final long TIME_LIMIT_IN_SECONDS = 200; // was 25, changed for jacoco + private static final int NUM_THREADS = 8; + private static final long TIME_LIMIT_IN_SECONDS = 250; // was 25, changed for jacoco private static final int NUM_KEYS = 100000; private ConcurrentZCMap oak; diff --git a/core/src/test/java/com/yahoo/oak/ValueUtilsSimpleTest.java b/core/src/test/java/com/yahoo/oak/ValueUtilsSimpleTest.java index f490ecf9..9fda9b36 100644 --- a/core/src/test/java/com/yahoo/oak/ValueUtilsSimpleTest.java +++ b/core/src/test/java/com/yahoo/oak/ValueUtilsSimpleTest.java @@ -81,7 +81,11 @@ public void testCannotWriteLockReadLocked() throws InterruptedException { } Assert.assertEquals(ValueUtils.ValueResult.TRUE, s.preRead()); flag.incrementAndGet(); - s.postRead(); + try { + s.postRead(); + } catch (DeletedMemoryAccessException e) { + e.printStackTrace(); + } }); Assert.assertEquals(ValueUtils.ValueResult.TRUE, s.preRead()); writer.start(); @@ -93,7 +97,11 @@ public void testCannotWriteLockReadLocked() throws InterruptedException { } Thread.sleep(2000); flag.incrementAndGet(); - s.postRead(); + try { + s.postRead(); + } catch (DeletedMemoryAccessException e) { + e.printStackTrace(); + } reader.join(); writer.join(); } @@ -119,7 +127,11 @@ public void testCannotDeletedReadLocked() throws InterruptedException { } Assert.assertEquals(ValueUtils.ValueResult.TRUE, s.preRead()); flag.incrementAndGet(); - s.postRead(); + try { + s.postRead(); + } catch (DeletedMemoryAccessException e) { + e.printStackTrace(); + } }); Assert.assertEquals(ValueUtils.ValueResult.TRUE, s.preRead()); deleter.start(); @@ -131,7 +143,11 @@ public void testCannotDeletedReadLocked() throws InterruptedException { } Thread.sleep(2000); flag.incrementAndGet(); - s.postRead(); + try { + s.postRead(); + } catch (DeletedMemoryAccessException e) { + e.printStackTrace(); + } reader.join(); deleter.join(); } diff --git a/core/src/test/java/com/yahoo/oak/ValueUtilsTest.java b/core/src/test/java/com/yahoo/oak/ValueUtilsTest.java index 0c4b9afd..87988708 100644 --- a/core/src/test/java/com/yahoo/oak/ValueUtilsTest.java +++ b/core/src/test/java/com/yahoo/oak/ValueUtilsTest.java @@ -279,7 +279,11 @@ public int calculateHash(Integer object) { int a = getInt(0); int b = getInt(4); int c = getInt(8); - s.getSlice().postRead(); + try { + s.getSlice().postRead(); + } catch (DeletedMemoryAccessException e) { + e.printStackTrace(); + } putter.join(); Assert.assertNotEquals(randomValues[0], a); Assert.assertNotEquals(randomValues[1], b); @@ -413,7 +417,11 @@ public void cannotComputeReadLockedTest() throws InterruptedException { for (int i = 0; i < 3; i++) { results[i] = getInt(i * 4); } - s.getSlice().postRead(); + try { + s.getSlice().postRead(); + } catch (DeletedMemoryAccessException e) { + e.printStackTrace(); + } computer.join(); Assert.assertArrayEquals(randomValues, results); }