From ccd7b5db215f9e4406d18909a4fbd2b7404dfdfd Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Wed, 20 Apr 2022 12:27:30 +0300 Subject: [PATCH 1/7] phase 1: runtimeexception to exception --- .../main/java/com/yahoo/oak/BasicChunk.java | 2 +- .../oak/DeletedMemoryAccessException.java | 2 +- .../main/java/com/yahoo/oak/EntryArray.java | 6 +- .../java/com/yahoo/oak/InternalOakBasics.java | 46 +++++--- .../java/com/yahoo/oak/InternalOakHash.java | 20 +++- .../java/com/yahoo/oak/InternalOakMap.java | 109 ++++++++++-------- .../main/java/com/yahoo/oak/NovaMMHeader.java | 4 +- core/src/main/java/com/yahoo/oak/OakMap.java | 25 +++- .../main/java/com/yahoo/oak/OrderedChunk.java | 57 +++++---- 9 files changed, 169 insertions(+), 102 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/BasicChunk.java b/core/src/main/java/com/yahoo/oak/BasicChunk.java index 33d3e99e..35f9556a 100644 --- a/core/src/main/java/com/yahoo/oak/BasicChunk.java +++ b/core/src/main/java/com/yahoo/oak/BasicChunk.java @@ -66,7 +66,7 @@ protected void updateBasicChild(BasicChunk child) { abstract boolean readValueFromEntryIndex(ValueBuffer value, int ei); - abstract void lookUp(ThreadContext ctx, K key); + 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..dc57a3c9 100644 --- a/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java +++ b/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java @@ -6,7 +6,7 @@ package com.yahoo.oak; -public class DeletedMemoryAccessException extends RuntimeException { +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/InternalOakBasics.java b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java index 2db21b62..408e192a 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java @@ -166,8 +166,11 @@ V replace(K key, V value, OakTransformer valueDeserializeTransformer) { for (int i = 0; i < MAX_RETRIES; i++) { BasicChunk chunk = findChunk(key, ctx); // find orderedChunk matching key - chunk.lookUp(ctx, key); - if (!ctx.isValueValid()) { + try { + chunk.lookUp(ctx, key); + } catch (DeletedMemoryAccessException e) { + continue; + } if (!ctx.isValueValid()) { return null; } @@ -184,7 +187,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) { + boolean replace(K key, V oldValue, V newValue, OakTransformer valueDeserializeTransformer) throws DeletedMemoryAccessException { ThreadContext ctx = getThreadContext(); for (int i = 0; i < MAX_RETRIES; i++) { @@ -355,7 +358,7 @@ public boolean hasNext() { return (state != null); } - protected abstract void initAfterRebalance(); + protected abstract void initAfterRebalance() throws DeletedMemoryAccessException; // the actual next() public abstract T next(); @@ -390,7 +393,7 @@ public void remove() { * The first long is the key's reference, the integer is the value's version and the second long is * the value's reference. If {@code needsValue == false}, then the value of the map entry is {@code null}. */ - void advance(boolean needsValue) { + void advance(boolean needsValue) throws DeletedMemoryAccessException { boolean validState = false; while (!validState) { @@ -471,7 +474,7 @@ 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,21 +486,28 @@ protected boolean advanceState() { BasicChunk.BasicChunkIter chunkIter = getState().getChunkIter(); updatePreviousState(); - - while (!chunkIter.hasNext()) { // skip empty chunks - chunk = getNextChunk(chunk); - if (chunk == null) { - //End of iteration - setState(null); - return false; + + for (int i = 0; i < MAX_RETRIES; i++) { + while (!chunkIter.hasNext()) { // skip empty chunks + chunk = getNextChunk(chunk); + if (chunk == null) { + //End of iteration + setState(null); + return false; + } + try { + chunkIter = getChunkIter(chunk); + } catch (DeletedMemoryAccessException e) { + continue; + } } - chunkIter = getChunkIter(chunk); - } - int nextIndex = chunkIter.next(ctx); - getState().set(chunk, chunkIter, nextIndex); + int nextIndex = chunkIter.next(ctx); + getState().set(chunk, chunkIter, nextIndex); - return true; + return true; + } + throw new RuntimeException("put failed: reached retry limit (1024)."); } } diff --git a/core/src/main/java/com/yahoo/oak/InternalOakHash.java b/core/src/main/java/com/yahoo/oak/InternalOakHash.java index a5db61df..f7c59ab0 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakHash.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakHash.java @@ -269,7 +269,11 @@ 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); - c.lookUp(ctx, key); + try { + c.lookUp(ctx, key); + } catch (DeletedMemoryAccessException e) { + continue; + } if (!ctx.isValueValid()) { return false; } @@ -407,8 +411,11 @@ 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); - c.lookUp(ctx, key); - + try { + c.lookUp(ctx, key); + } catch (DeletedMemoryAccessException e) { + continue; + } if (ctx.isValueValid()) { ValueUtils.ValueResult res = config.valueOperator.compute(ctx.value, computer); if (res == ValueUtils.ValueResult.TRUE) { @@ -441,8 +448,11 @@ 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); - c.lookUp(ctx, key); - + try { + c.lookUp(ctx, key); + } catch (DeletedMemoryAccessException e) { + continue; + } 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 diff --git a/core/src/main/java/com/yahoo/oak/InternalOakMap.java b/core/src/main/java/com/yahoo/oak/InternalOakMap.java index 0cd5dd71..dcca48fc 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) throws DeletedMemoryAccessException { Map.Entry> lowerChunkEntry = skiplist.lowerEntry(key); if (lowerChunkEntry == null) { /* we were looking for the minimal key */ @@ -913,7 +914,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 +923,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 +932,7 @@ private boolean tooHigh(OakScopedReadBuffer key) { } - private boolean inBounds(OakScopedReadBuffer key) { + private boolean inBounds(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (!isDescending) { return !tooHigh(key); } else { @@ -945,10 +946,10 @@ protected void initAfterRebalance() { K nextKey = null; while (true) { while (!getState().getChunk().readKeyFromEntryIndex(ctx.tempKey, getState().getIndex())) { - advanceState(); - if (getState() == null) { - throw new NoSuchElementException(); - } + advanceState(); + if (getState() == null) { + throw new NoSuchElementException(); + } } nextKey = null; try { @@ -975,6 +976,7 @@ protected void initAfterRebalance() { /** * Advances next to higher entry. * Return previous index + * @throws DeletedMemoryAccessException * * */ @@ -1067,43 +1069,50 @@ 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) { + continue; } } - - //Init state, not valid yet, must move forward - setPrevState(InternalOakMap.IteratorState.newInstance(null, null)); - setState(IteratorState.newInstance(nextOrderedChunk, nextChunkIter)); - advanceState(); + throw new RuntimeException("put failed: reached retry limit (1024)."); } @Override @@ -1121,7 +1130,7 @@ protected BasicChunk getNextChunk(BasicChunk current) { } } - protected BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) { + protected BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) throws DeletedMemoryAccessException { if (!isDescending) { OakScopedReadBuffer upperBoundKeyForChunk = getNextChunkMinKey((OrderedChunk) current); return ((OrderedChunk) current).ascendingIter(ctx, hi, hiInclusive, upperBoundKeyForChunk); @@ -1167,9 +1176,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; } } } diff --git a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java index bd7c4a0a..ce3e8639 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java +++ b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java @@ -47,7 +47,7 @@ private static boolean atomicallySetDeleted(long headerAddress, long offHeapMeta } - ValueUtils.ValueResult preRead(final int onHeapVersion, long headerAddress) { + ValueUtils.ValueResult preRead(final int onHeapVersion, long headerAddress) throws DeletedMemoryAccessException { long offHeapHeader = getOffHeapHeader(headerAddress); if (RC.isReferenceDeleted(offHeapHeader)) { throw new DeletedMemoryAccessException(); @@ -55,7 +55,7 @@ ValueUtils.ValueResult preRead(final int onHeapVersion, long headerAddress) { return ValueUtils.ValueResult.TRUE; } - ValueUtils.ValueResult postRead(final int onHeapVersion, long headerAddress) { + 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/OakMap.java b/core/src/main/java/com/yahoo/oak/OakMap.java index 2d08387a..dc520d3e 100644 --- a/core/src/main/java/com/yahoo/oak/OakMap.java +++ b/core/src/main/java/com/yahoo/oak/OakMap.java @@ -267,12 +267,20 @@ public Entry lowerEntry(K key) { if (this.isSubmap()) { throw new UnsupportedOperationException(); } + for (int i = 0; i < InternalOakBasics.MAX_RETRIES; i++) { - if (key == null) { - throw new NullPointerException(); + if (key == null) { + throw new NullPointerException(); + } + + try { + return internalOakMap.lowerEntry(key); + } catch(DeletedMemoryAccessException e) { + continue; + } } - return internalOakMap.lowerEntry(key); + throw new RuntimeException("replace failed: reached retry limit (1024)."); } /** @@ -290,7 +298,16 @@ public K lowerKey(K key) { throw new NullPointerException(); } - return internalOakMap.lowerEntry(key).getKey(); + for (int i = 0; i < InternalOakBasics.MAX_RETRIES; i++) { + + try { + return internalOakMap.lowerEntry(key).getKey(); + } catch(DeletedMemoryAccessException e) { + continue; + } + } + + throw new RuntimeException("replace failed: reached retry limit (1024)."); } diff --git a/core/src/main/java/com/yahoo/oak/OrderedChunk.java b/core/src/main/java/com/yahoo/oak/OrderedChunk.java index 10d97f27..7483a9b1 100644 --- a/core/src/main/java/com/yahoo/oak/OrderedChunk.java +++ b/core/src/main/java/com/yahoo/oak/OrderedChunk.java @@ -231,8 +231,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 +261,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 +305,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 +383,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 +633,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 +645,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 +698,10 @@ 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; @@ -726,14 +734,18 @@ 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); + try { + setIsEndBoundCheckNeeded(ctx, to, toInclusive, nextChunkMinKey); + } catch (DeletedMemoryAccessException e) { + throw new DeletedMemoryAccessException(); //TODO fix + } } 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,7 @@ 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 +875,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 +919,9 @@ 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 +990,11 @@ private void advance(KeyBuffer keyBuff) { return; } findNewAnchor(); - traverseLinkedList(keyBuff, false); + try { + traverseLinkedList(keyBuff, false); + } catch (DeletedMemoryAccessException e) { + continue; + } } } @@ -994,7 +1011,7 @@ public int next(ThreadContext ctx) { } @Override - protected boolean isKeyOutOfEndBound(OakScopedReadBuffer key) { + protected boolean isKeyOutOfEndBound(OakScopedReadBuffer key) throws DeletedMemoryAccessException { if (endBound == null) { return false; } From 486e578bdd157e514c0cb7f906fabb8109f21b6e Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Wed, 20 Apr 2022 15:10:40 +0300 Subject: [PATCH 2/7] fully changed runtimeexceptimeexception to exception --- .../java/com/yahoo/oak/InternalOakBasics.java | 11 +++++---- .../java/com/yahoo/oak/InternalOakHash.java | 9 +++++-- .../java/com/yahoo/oak/InternalOakMap.java | 23 +++++++++++------- .../main/java/com/yahoo/oak/NovaMMHeader.java | 5 ++-- .../java/com/yahoo/oak/NovaMemoryManager.java | 4 +++- core/src/main/java/com/yahoo/oak/OakMap.java | 4 ++-- .../main/java/com/yahoo/oak/OrderedChunk.java | 9 ++++--- core/src/main/java/com/yahoo/oak/Slice.java | 2 +- .../yahoo/oak/SyncRecycleMemoryManager.java | 4 +++- .../yahoo/oak/UnscopedValueBufferSynced.java | 14 ++++++++--- .../main/java/com/yahoo/oak/ValueUtils.java | 6 ++++- .../com/yahoo/oak/ValueUtilsSimpleTest.java | 24 +++++++++++++++---- .../java/com/yahoo/oak/ValueUtilsTest.java | 12 ++++++++-- 13 files changed, 93 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java index 408e192a..9a57c2c1 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java @@ -170,7 +170,8 @@ V replace(K key, V value, OakTransformer valueDeserializeTransformer) { chunk.lookUp(ctx, key); } catch (DeletedMemoryAccessException e) { continue; - } if (!ctx.isValueValid()) { + } + if (!ctx.isValueValid()) { return null; } @@ -187,7 +188,8 @@ 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) throws DeletedMemoryAccessException { + boolean replace(K key, V oldValue, V newValue, OakTransformer valueDeserializeTransformer) + throws DeletedMemoryAccessException { ThreadContext ctx = getThreadContext(); for (int i = 0; i < MAX_RETRIES; i++) { @@ -474,7 +476,8 @@ protected void invalidatePrevState() { protected abstract BasicChunk getNextChunk(BasicChunk current); - protected abstract BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) throws DeletedMemoryAccessException; + protected abstract BasicChunk.BasicChunkIter getChunkIter(BasicChunk current) + throws DeletedMemoryAccessException; /** * advance state to the new position @@ -496,7 +499,7 @@ protected boolean advanceState() { return false; } try { - chunkIter = getChunkIter(chunk); + chunkIter = getChunkIter(chunk); } catch (DeletedMemoryAccessException e) { continue; } diff --git a/core/src/main/java/com/yahoo/oak/InternalOakHash.java b/core/src/main/java/com/yahoo/oak/InternalOakHash.java index f7c59ab0..82b05ad0 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakHash.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakHash.java @@ -809,8 +809,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) { @@ -827,7 +828,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 dcca48fc..263d1bc2 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakMap.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakMap.java @@ -946,10 +946,10 @@ protected void initAfterRebalance() { K nextKey = null; while (true) { while (!getState().getChunk().readKeyFromEntryIndex(ctx.tempKey, getState().getIndex())) { - advanceState(); - if (getState() == null) { - throw new NoSuchElementException(); - } + advanceState(); + if (getState() == null) { + throw new NoSuchElementException(); + } } nextKey = null; try { @@ -1082,8 +1082,8 @@ protected void initState(boolean isDescending, K lowerBound, boolean lowerInclus if (nextOrderedChunk != null) { OakScopedReadBuffer upperBoundKeyForChunk = getNextChunkMinKey(nextOrderedChunk); nextChunkIter = lowerBound != null ? - nextOrderedChunk.ascendingIter(ctx, lowerBound, lowerInclusive, upperBound, upperInclusive, - upperBoundKeyForChunk) + nextOrderedChunk.ascendingIter + (ctx, lowerBound, lowerInclusive, upperBound, upperInclusive, upperBoundKeyForChunk) : nextOrderedChunk .ascendingIter(ctx, upperBound, upperInclusive, upperBoundKeyForChunk); } else { @@ -1111,6 +1111,7 @@ protected void initState(boolean isDescending, K lowerBound, boolean lowerInclus } catch (DeletedMemoryAccessException e) { continue; } + return; } throw new RuntimeException("put failed: reached retry limit (1024)."); } @@ -1310,8 +1311,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) { @@ -1328,7 +1330,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 ce3e8639..d531737d 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java +++ b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java @@ -47,10 +47,11 @@ private static boolean atomicallySetDeleted(long headerAddress, long offHeapMeta } - ValueUtils.ValueResult preRead(final int onHeapVersion, long headerAddress) throws DeletedMemoryAccessException { + 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; } diff --git a/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java b/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java index 4fab7a7a..e5780f5c 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java +++ b/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java @@ -191,6 +191,7 @@ public long getAddress() { * {@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; @@ -203,8 +204,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 dc520d3e..be65ec94 100644 --- a/core/src/main/java/com/yahoo/oak/OakMap.java +++ b/core/src/main/java/com/yahoo/oak/OakMap.java @@ -275,7 +275,7 @@ public Entry lowerEntry(K key) { try { return internalOakMap.lowerEntry(key); - } catch(DeletedMemoryAccessException e) { + } catch (DeletedMemoryAccessException e) { continue; } } @@ -302,7 +302,7 @@ public K lowerKey(K key) { try { return internalOakMap.lowerEntry(key).getKey(); - } catch(DeletedMemoryAccessException e) { + } catch (DeletedMemoryAccessException e) { continue; } } diff --git a/core/src/main/java/com/yahoo/oak/OrderedChunk.java b/core/src/main/java/com/yahoo/oak/OrderedChunk.java index 7483a9b1..514487c7 100644 --- a/core/src/main/java/com/yahoo/oak/OrderedChunk.java +++ b/core/src/main/java/com/yahoo/oak/OrderedChunk.java @@ -701,7 +701,8 @@ boolean isBoundCheckNeeded() { abstract boolean isKeyOutOfEndBound(OakScopedReadBuffer boundKey) throws DeletedMemoryAccessException; protected void setIsEndBoundCheckNeeded( - ThreadContext ctx, K to, boolean toInclusive, OakScopedReadBuffer chunkBoundaryKey) throws DeletedMemoryAccessException { + ThreadContext ctx, K to, boolean toInclusive, OakScopedReadBuffer chunkBoundaryKey) + throws DeletedMemoryAccessException { this.endBound = to; this.endBoundInclusive = toInclusive; @@ -854,7 +855,8 @@ class DescendingIter extends ChunkIter { initNext(tempKeyBuff); } - DescendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, boolean toInclusive) throws DeletedMemoryAccessException { + DescendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, boolean toInclusive) + throws DeletedMemoryAccessException { KeyBuffer tempKeyBuff = ctx.tempKey; this.from = from; @@ -921,7 +923,8 @@ private void pushToStack(boolean compareWithPrevAnchor) { * @param firstTimeInvocation * @throws DeletedMemoryAccessException */ - private void traverseLinkedList(KeyBuffer tempKeyBuff, boolean firstTimeInvocation) throws DeletedMemoryAccessException { + 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; 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/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); } From b840c752cfce0e3eeb5449e491c5cef045fb54ee Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Wed, 29 Jun 2022 11:28:27 +0300 Subject: [PATCH 3/7] addressing some of review comments --- .../main/java/com/yahoo/oak/BasicChunk.java | 3 + .../oak/DeletedMemoryAccessException.java | 4 ++ .../java/com/yahoo/oak/InternalOakBasics.java | 28 +------- .../java/com/yahoo/oak/InternalOakMap.java | 71 ++++++++++--------- .../main/java/com/yahoo/oak/NovaMMHeader.java | 7 ++ .../java/com/yahoo/oak/NovaMemoryManager.java | 1 - core/src/main/java/com/yahoo/oak/OakMap.java | 27 ++----- 7 files changed, 61 insertions(+), 80 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/BasicChunk.java b/core/src/main/java/com/yahoo/oak/BasicChunk.java index 35f9556a..79538223 100644 --- a/core/src/main/java/com/yahoo/oak/BasicChunk.java +++ b/core/src/main/java/com/yahoo/oak/BasicChunk.java @@ -66,6 +66,9 @@ protected void updateBasicChild(BasicChunk child) { abstract boolean readValueFromEntryIndex(ValueBuffer value, int ei); + /* 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 dc57a3c9..a0924dec 100644 --- a/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java +++ b/core/src/main/java/com/yahoo/oak/DeletedMemoryAccessException.java @@ -6,6 +6,10 @@ package com.yahoo.oak; +/** + * 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/InternalOakBasics.java b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java index 9a57c2c1..574fb45a 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java @@ -169,6 +169,7 @@ V replace(K key, V value, OakTransformer valueDeserializeTransformer) { try { chunk.lookUp(ctx, key); } catch (DeletedMemoryAccessException e) { + assert !(chunk instanceof HashChunk); continue; } if (!ctx.isValueValid()) { @@ -188,30 +189,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) - throws DeletedMemoryAccessException { - 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); @@ -360,7 +338,7 @@ public boolean hasNext() { return (state != null); } - protected abstract void initAfterRebalance() throws DeletedMemoryAccessException; + protected abstract void initAfterRebalance(); // the actual next() public abstract T next(); diff --git a/core/src/main/java/com/yahoo/oak/InternalOakMap.java b/core/src/main/java/com/yahoo/oak/InternalOakMap.java index 263d1bc2..4008cd1d 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakMap.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakMap.java @@ -816,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) throws DeletedMemoryAccessException { + Map.Entry lowerEntry(K key) { Map.Entry> lowerChunkEntry = skiplist.lowerEntry(key); if (lowerChunkEntry == null) { /* we were looking for the minimal key */ @@ -825,39 +825,48 @@ Map.Entry lowerEntry(K key) throws DeletedMemoryAccessException { 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 --------------*/ @@ -976,8 +985,6 @@ protected void initAfterRebalance() { /** * Advances next to higher entry. * Return previous index - * @throws DeletedMemoryAccessException - * * */ @Override diff --git a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java index d531737d..9c313edd 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMMHeader.java +++ b/core/src/main/java/com/yahoo/oak/NovaMMHeader.java @@ -47,6 +47,9 @@ 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)) { @@ -56,6 +59,10 @@ ValueUtils.ValueResult preRead(final int onHeapVersion, long headerAddress) { return ValueUtils.ValueResult.TRUE; } + /** 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); diff --git a/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java b/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java index e5780f5c..83ccb202 100644 --- a/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java +++ b/core/src/main/java/com/yahoo/oak/NovaMemoryManager.java @@ -191,7 +191,6 @@ public long getAddress() { * {@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; diff --git a/core/src/main/java/com/yahoo/oak/OakMap.java b/core/src/main/java/com/yahoo/oak/OakMap.java index be65ec94..6bb20a21 100644 --- a/core/src/main/java/com/yahoo/oak/OakMap.java +++ b/core/src/main/java/com/yahoo/oak/OakMap.java @@ -267,20 +267,13 @@ public Entry lowerEntry(K key) { if (this.isSubmap()) { throw new UnsupportedOperationException(); } - for (int i = 0; i < InternalOakBasics.MAX_RETRIES; i++) { - if (key == null) { - throw new NullPointerException(); - } - - try { - return internalOakMap.lowerEntry(key); - } catch (DeletedMemoryAccessException e) { - continue; - } + if (key == null) { + throw new NullPointerException(); } + + return internalOakMap.lowerEntry(key); - throw new RuntimeException("replace failed: reached retry limit (1024)."); } /** @@ -297,17 +290,7 @@ public K lowerKey(K key) { if (key == null) { throw new NullPointerException(); } - - for (int i = 0; i < InternalOakBasics.MAX_RETRIES; i++) { - - try { - return internalOakMap.lowerEntry(key).getKey(); - } catch (DeletedMemoryAccessException e) { - continue; - } - } - - throw new RuntimeException("replace failed: reached retry limit (1024)."); + return internalOakMap.lowerEntry(key).getKey(); } From 57047d21d5224131a4ee1ee7f570b311ff666da4 Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Sun, 18 Sep 2022 14:49:45 +0300 Subject: [PATCH 4/7] resolving issue with iterator --- .../java/com/yahoo/oak/EntryOrderedSet.java | 2 +- .../java/com/yahoo/oak/InternalOakBasics.java | 37 ++++++++++--------- .../java/com/yahoo/oak/InternalOakHash.java | 4 ++ .../java/com/yahoo/oak/InternalOakMap.java | 32 +++++++++++++--- .../main/java/com/yahoo/oak/OrderedChunk.java | 3 +- 5 files changed, 53 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java index 7489ce8b..98673c35 100644 --- a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java +++ b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java @@ -299,7 +299,7 @@ boolean isEntrySetValidAfterRebalance() { 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++) { 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 574fb45a..50abc7a0 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java @@ -338,6 +338,8 @@ public boolean hasNext() { return (state != null); } + protected abstract void initStateWithNotDeletedChunk(BasicChunk chunk); + protected abstract void initAfterRebalance(); // the actual next() @@ -468,27 +470,26 @@ protected boolean advanceState() { updatePreviousState(); - for (int i = 0; i < MAX_RETRIES; i++) { - while (!chunkIter.hasNext()) { // skip empty chunks - chunk = getNextChunk(chunk); - if (chunk == null) { - //End of iteration - setState(null); - return false; - } - try { - chunkIter = getChunkIter(chunk); - } catch (DeletedMemoryAccessException e) { - continue; - } + while (!chunkIter.hasNext()) { // skip empty chunks + chunk = getNextChunk(chunk); + if (chunk == null) { + //End of iteration + setState(null); + return false; + } + try { + chunkIter = getChunkIter(chunk); + } catch (DeletedMemoryAccessException e) { + assert chunk.state.get() == BasicChunk.State.RELEASED; + initStateWithNotDeletedChunk(chunk); + return true; } + } - int nextIndex = chunkIter.next(ctx); - getState().set(chunk, chunkIter, nextIndex); + int nextIndex = chunkIter.next(ctx); + getState().set(chunk, chunkIter, nextIndex); - return true; - } - throw new RuntimeException("put failed: reached retry limit (1024)."); + 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 82b05ad0..6ddd4b6b 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakHash.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakHash.java @@ -579,6 +579,10 @@ protected void initAfterRebalance() { initState(); } + @Override + protected void initStateWithNotDeletedChunk(BasicChunk c) { //nextKey is null here + initState(); + } /** * Advances next to higher entry. diff --git a/core/src/main/java/com/yahoo/oak/InternalOakMap.java b/core/src/main/java/com/yahoo/oak/InternalOakMap.java index 4008cd1d..8642643c 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakMap.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakMap.java @@ -981,6 +981,28 @@ protected void initAfterRebalance() { // Update the state to point to last returned key. initState(isDescending, lo, loInclusive, hi, hiInclusive); } + + @Override + protected void initStateWithNotDeletedChunk(BasicChunk chunk) { + OrderedChunk Ochunk = (OrderedChunk)chunk; + K nextKey = null; + try { + nextKey = KeyUtils.deSerializedKey(Ochunk.minKey, getKeySerializer()); + } catch (DeletedMemoryAccessException e) { + return ;//should not happen + } + 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. @@ -1120,16 +1142,16 @@ protected void initState(boolean isDescending, K lowerBound, boolean lowerInclus } return; } - throw new RuntimeException("put failed: reached retry limit (1024)."); + throw new RuntimeException("reached retry limit (1024)."); } @Override protected BasicChunk getNextChunk(BasicChunk current) { - OrderedChunk currentHashChunk = (OrderedChunk) current; + OrderedChunk currenChunk = (OrderedChunk) current; if (!isDescending) { - return currentHashChunk.next.getReference(); + return currenChunk.next.getReference(); } else { - Map.Entry> entry = skiplist.lowerEntry(currentHashChunk.minKey); + Map.Entry> entry = skiplist.lowerEntry(currenChunk.minKey); if (entry == null) { return null; } else { @@ -1172,7 +1194,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(); diff --git a/core/src/main/java/com/yahoo/oak/OrderedChunk.java b/core/src/main/java/com/yahoo/oak/OrderedChunk.java index 514487c7..9c1aeeef 100644 --- a/core/src/main/java/com/yahoo/oak/OrderedChunk.java +++ b/core/src/main/java/com/yahoo/oak/OrderedChunk.java @@ -721,7 +721,8 @@ 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; } From 7343569da4e2d20baa5b902993599c9da972f16b Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Sun, 18 Sep 2022 15:01:07 +0300 Subject: [PATCH 5/7] fixing validate stage syntax error --- core/src/main/java/com/yahoo/oak/InternalOakMap.java | 6 +++--- core/src/main/java/com/yahoo/oak/OrderedChunk.java | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/InternalOakMap.java b/core/src/main/java/com/yahoo/oak/InternalOakMap.java index 8642643c..700c867a 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakMap.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakMap.java @@ -984,12 +984,12 @@ protected void initAfterRebalance() { @Override protected void initStateWithNotDeletedChunk(BasicChunk chunk) { - OrderedChunk Ochunk = (OrderedChunk)chunk; + OrderedChunk oChunk = (OrderedChunk) chunk; K nextKey = null; try { - nextKey = KeyUtils.deSerializedKey(Ochunk.minKey, getKeySerializer()); + nextKey = KeyUtils.deSerializedKey(oChunk.minKey, getKeySerializer()); } catch (DeletedMemoryAccessException e) { - return ;//should not happen + return ; //should not happen } if (isDescending) { hiInclusive = true; diff --git a/core/src/main/java/com/yahoo/oak/OrderedChunk.java b/core/src/main/java/com/yahoo/oak/OrderedChunk.java index 9c1aeeef..81199f8d 100644 --- a/core/src/main/java/com/yahoo/oak/OrderedChunk.java +++ b/core/src/main/java/com/yahoo/oak/OrderedChunk.java @@ -721,8 +721,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? - if (!readKeyFromEntryIndex(ctx.tempKey, midIdx)) + if (!readKeyFromEntryIndex(ctx.tempKey, midIdx)) { throw new DeletedMemoryAccessException(); + } if (!isKeyOutOfEndBound(ctx.tempKey)) { isEndBoundCheckNeeded = IterEndBoundCheck.MID_END_BOUNDARY_CHECK; } From 8209e134a3fdc19c58145fb8b9bc675a800a2fd1 Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Sun, 18 Sep 2022 15:42:17 +0300 Subject: [PATCH 6/7] minor bug fix --- core/src/main/java/com/yahoo/oak/EntryOrderedSet.java | 4 +++- core/src/test/java/com/yahoo/oak/PutIfAbsentTest.java | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java index 98673c35..6b7a66f0 100644 --- a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java +++ b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java @@ -299,7 +299,9 @@ boolean isEntrySetValidAfterRebalance() { void releaseAllDeletedKeys() { KeyBuffer key = new KeyBuffer(config.keysMemoryManager.getEmptySlice()); - for (int i = 0; i < nextFreeIndex.get() - 1 ; i++) { + for (int i = 0; i < nextFreeIndex.get() - 1 && i < numOfEntries.get() ; i++) { + //the second condiiton is for case where multiple threads increment the nextfreeIndex + //which results in being bigger than the array size if (!config.valuesMemoryManager.isReferenceDeleted(getValueReference(i))) { continue; } 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; From 192a236a2a66b84f4b8d8b34ac8fb34d2bbe3ea8 Mon Sep 17 00:00:00 2001 From: Ramy Fakhoury Date: Tue, 11 Oct 2022 11:46:53 +0300 Subject: [PATCH 7/7] adressing core review, adding some comments for some of the cases (resubmitted) --- .../java/com/yahoo/oak/EntryOrderedSet.java | 6 +++-- .../java/com/yahoo/oak/InternalOakBasics.java | 14 ++++++---- .../java/com/yahoo/oak/InternalOakHash.java | 26 +++++-------------- .../java/com/yahoo/oak/InternalOakMap.java | 21 ++++++++++----- .../main/java/com/yahoo/oak/OrderedChunk.java | 9 +++---- 5 files changed, 37 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java index 6b7a66f0..59309e09 100644 --- a/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java +++ b/core/src/main/java/com/yahoo/oak/EntryOrderedSet.java @@ -297,11 +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 < nextFreeIndex.get() - 1 && i < numOfEntries.get() ; i++) { - //the second condiiton is for case where multiple threads increment the nextfreeIndex - //which results in being bigger than the array size + //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 50abc7a0..a2e2288d 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakBasics.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakBasics.java @@ -165,11 +165,14 @@ 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 + 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); + assert !(chunk instanceof HashChunk); + //Hash deals with deleted keys in an earlier stage continue; } if (!ctx.isValueValid()) { @@ -338,7 +341,8 @@ public boolean hasNext() { return (state != null); } - protected abstract void initStateWithNotDeletedChunk(BasicChunk chunk); + // for more detail on this method see implementation + protected abstract void initStateWithMinKey(BasicChunk chunk); protected abstract void initAfterRebalance(); @@ -375,7 +379,7 @@ public void remove() { * The first long is the key's reference, the integer is the value's version and the second long is * the value's reference. If {@code needsValue == false}, then the value of the map entry is {@code null}. */ - void advance(boolean needsValue) throws DeletedMemoryAccessException { + void advance(boolean needsValue) { boolean validState = false; while (!validState) { @@ -481,7 +485,7 @@ protected boolean advanceState() { chunkIter = getChunkIter(chunk); } catch (DeletedMemoryAccessException e) { assert chunk.state.get() == BasicChunk.State.RELEASED; - initStateWithNotDeletedChunk(chunk); + initStateWithMinKey(chunk); 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 6ddd4b6b..15457d43 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakHash.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakHash.java @@ -268,12 +268,8 @@ 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); - try { - c.lookUp(ctx, key); - } catch (DeletedMemoryAccessException e) { - continue; - } + HashChunk c = findChunk(key, ctx); + c.lookUp(ctx, key); if (!ctx.isValueValid()) { return false; } @@ -410,12 +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); - try { - c.lookUp(ctx, key); - } catch (DeletedMemoryAccessException e) { - continue; - } + 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) { @@ -447,12 +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); - try { - c.lookUp(ctx, key); - } catch (DeletedMemoryAccessException e) { - continue; - } + 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 @@ -580,7 +568,7 @@ protected void initAfterRebalance() { } @Override - protected void initStateWithNotDeletedChunk(BasicChunk c) { //nextKey is null here + protected void initStateWithMinKey(BasicChunk c) { //nextKey is null here initState(); } diff --git a/core/src/main/java/com/yahoo/oak/InternalOakMap.java b/core/src/main/java/com/yahoo/oak/InternalOakMap.java index 700c867a..572bd310 100644 --- a/core/src/main/java/com/yahoo/oak/InternalOakMap.java +++ b/core/src/main/java/com/yahoo/oak/InternalOakMap.java @@ -982,14 +982,16 @@ protected void initAfterRebalance() { 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 initStateWithNotDeletedChunk(BasicChunk chunk) { - OrderedChunk oChunk = (OrderedChunk) chunk; + protected void initStateWithMinKey(BasicChunk chunk) { + OrderedChunk oChunk = (OrderedChunk ) chunk; K nextKey = null; try { nextKey = KeyUtils.deSerializedKey(oChunk.minKey, getKeySerializer()); } catch (DeletedMemoryAccessException e) { - return ; //should not happen + assert e == null; //since we are not deleting minKeys (should not get here!) + return ; } if (isDescending) { hiInclusive = true; @@ -1138,7 +1140,10 @@ protected void initState(boolean isDescending, K lowerBound, boolean lowerInclus setState(IteratorState.newInstance(nextOrderedChunk, nextChunkIter)); advanceState(); } catch (DeletedMemoryAccessException e) { - continue; + 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; } @@ -1147,11 +1152,11 @@ protected void initState(boolean isDescending, K lowerBound, boolean lowerInclus @Override protected BasicChunk getNextChunk(BasicChunk current) { - OrderedChunk currenChunk = (OrderedChunk) current; + OrderedChunk currentChunk = (OrderedChunk) current; if (!isDescending) { - return currenChunk.next.getReference(); + return currentChunk.next.getReference(); } else { - Map.Entry> entry = skiplist.lowerEntry(currenChunk.minKey); + Map.Entry> entry = skiplist.lowerEntry(currentChunk.minKey); if (entry == null) { return null; } else { @@ -1160,6 +1165,8 @@ protected BasicChunk getNextChunk(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); diff --git a/core/src/main/java/com/yahoo/oak/OrderedChunk.java b/core/src/main/java/com/yahoo/oak/OrderedChunk.java index 81199f8d..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; @@ -740,11 +741,7 @@ class AscendingIter extends ChunkIter { OakScopedReadBuffer nextChunkMinKey) throws DeletedMemoryAccessException { next = entryOrderedSet.getHeadNextEntryIndex(); next = advanceNextIndexNoBound(next, ctx); - try { - setIsEndBoundCheckNeeded(ctx, to, toInclusive, nextChunkMinKey); - } catch (DeletedMemoryAccessException e) { - throw new DeletedMemoryAccessException(); //TODO fix - } + setIsEndBoundCheckNeeded(ctx, to, toInclusive, nextChunkMinKey); } AscendingIter(ThreadContext ctx, K from, boolean fromInclusive, K to, boolean toInclusive,