From e47f50a87923e7752ffb866612f10997b64da6d4 Mon Sep 17 00:00:00 2001 From: dominictootell Date: Sat, 15 Aug 2015 15:38:27 +0100 Subject: [PATCH 1/3] implement array based ring, and fix concurrency issue in DummyListenableFuture, and allow enough time in Overflow test for reconnect --- .../net/spy/memcached/ArrayBasedCeilRing.java | 153 ++++++++ .../memcached/ArrayBasedCeilRingIterator.java | 61 +++ .../net/spy/memcached/KetamaNodeLocator.java | 60 +-- .../spy/memcached/ArrayBasedCeilRingTest.java | 359 ++++++++++++++++++ .../net/spy/memcached/QueueOverflowTest.java | 2 +- .../internal/DummyListenableFuture.java | 4 +- 6 files changed, 590 insertions(+), 49 deletions(-) create mode 100644 src/main/java/net/spy/memcached/ArrayBasedCeilRing.java create mode 100644 src/main/java/net/spy/memcached/ArrayBasedCeilRingIterator.java create mode 100644 src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java diff --git a/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java b/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java new file mode 100644 index 000000000..ad1622ea3 --- /dev/null +++ b/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java @@ -0,0 +1,153 @@ +package net.spy.memcached; + + + +import java.util.*; + +/** + * Uses a sorted array of values to represent the consistent hash ring of values that are associated with + * memcached nodes are at that index. + * + * The method {@link #findCeilIndex(long)} finds the index in the array (the memcached node in the ring) at which the + * first value is greater than or equal to the given long is. If the long is greater than the maximum value + * in the array then the memcached node at the first position in the array is returned (The first item in the ring) + */ +public class ArrayBasedCeilRing { + + private final long[] sortedNodePositions; + private final MemcachedNode[] sortedNodes; + private final Collection allNodes; + private final int lastIndexPosition; + + /** + * Is Passed ({@see #nodes}) a map of longs to the associated memcached nodes. + * This is used to create the sorted array ring and associated + * memcached nodes at those array indexes. + * + * The {@see #allNodes} is a list of unique memcached nodes that make up the current cluster + * + * @param nodes A map of longs to memcached nodes that should populate the consistent hash ring. + * @param allNodes The unique list of memcached nodes that are represented in the ring. + */ + public ArrayBasedCeilRing(Map nodes, Collection allNodes) { + long[] sortedNodePositions = new long[nodes.size()]; + MemcachedNode[] sortedNodes = new MemcachedNode[nodes.size()]; + this.allNodes = new ArrayList(allNodes); + + Long[] sortedObjectNodePositions = nodes.keySet().toArray(new Long[nodes.size()]); + Arrays.sort(sortedObjectNodePositions); + + int i = 0; + for(Long position : sortedObjectNodePositions) { + sortedNodePositions[i] = position; + sortedNodes[i++] = nodes.get(position); + } + + this.sortedNodePositions = sortedNodePositions; + this.sortedNodes = sortedNodes; + this.lastIndexPosition = nodes.size()-1; + } + + /** + * Returns the max value in the ring that a memcached node is located at. + */ + public long getMaxPosition() { + return sortedNodePositions[lastIndexPosition]; + } + + /** + * Returns as a map, the memcached nodes associated to their positions + * @return + */ + public Map asMap() { + Map map = new TreeMap(); + for(int i=0;i getAllNodes() { + return allNodes; + } + + public ArrayBasedCeilRing roClone() { + Map nodes = new HashMap(sortedNodePositions.length,1.0f); + for(int i=0;i allNodes = new ArrayList(this.allNodes.size()); + for(MemcachedNode node : this.allNodes) { + allNodes.add(new MemcachedNodeROImpl(node)); + } + + return new ArrayBasedCeilRing(nodes,allNodes); + } + + public MemcachedNode findClosestNode(long key) { + return sortedNodes[findCeilIndex(key)]; + } + + /** + * Find the index in the array at which the first value greater than or equal + * to the given hashVal is. If hashVal is greater than the maximum value in the + * array then 0 is returned (The first item in the ring). If hashVal is less than or + * equal to the first element in the array then 0 is returned (The first item in the ring). + * + * Uses a binary search type algorithm ( O(log n) ) to find the closest largest value in the array, to the + * given hashVal + * + * @param hashVal The value to find the closest item + * @return The index in the array that the closest largest item exists + */ + public int findCeilIndex(final long hashVal) { + return findCeilIndex(hashVal,sortedNodePositions,lastIndexPosition); + } + + /** + * Find the index in the array at which the first value greater than or equal + * to the given hashVal is. If hashVal is greater than the maximum value in the + * array then 0 is returned (The first item in the ring). If hashVal is less than or + * equal to the first element in the array then 0 is returned (The first item in the ring). + * + * Uses a binary search type algorithm ( O(log n) ) to find the closest largest value in the array, to the + * given hashVal + * + * @param hashVal The value to find the closest item + * @return The index in the array that the closest largest item exists + */ + static int findCeilIndex(final long hashVal, final long sortedNodePositions[], final int lastIndexPosition) { + if (lastIndexPosition < 0) { + throw new IllegalArgumentException("array cannot be empty"); + } + + int low = 0; + int high = lastIndexPosition; + + // Check for edge cases + if (hashVal > sortedNodePositions[high]) return 0; + if (hashVal <= sortedNodePositions[low]) return 0; + + + while(true) { + // div by two + int mid = (low + high) >>> 1; + long midVal = sortedNodePositions[mid]; + if (midVal == hashVal) return mid; + else if (midVal < hashVal) { + low = mid + 1; + if (low <= high && sortedNodePositions[low] >= hashVal) return low; + } + else if (midVal > hashVal) { + high = mid - 1; + if (high >= low && sortedNodePositions[high] < hashVal) return mid; + } + } + } + + +} diff --git a/src/main/java/net/spy/memcached/ArrayBasedCeilRingIterator.java b/src/main/java/net/spy/memcached/ArrayBasedCeilRingIterator.java new file mode 100644 index 000000000..55441b274 --- /dev/null +++ b/src/main/java/net/spy/memcached/ArrayBasedCeilRingIterator.java @@ -0,0 +1,61 @@ +package net.spy.memcached; + +import java.util.Iterator; + +/** + * Implements an Iterator which the KetamaNodeLoctaor may return to a client for + * iterating through alternate nodes for a given key. + */ +public class ArrayBasedCeilRingIterator implements Iterator { + + private final String key; + private long hashVal; + private int remainingTries; + private int numTries = 0; + private final HashAlgorithm hashAlg; + private final ArrayBasedCeilRing ring; + + /** + * Create a new ArrayBasedCeilRingIterator to be used by a client for an operation. + * + * @param k the key to iterate for + * @param t the number of tries until giving up + * @param hashAlg the hash algorithm to use when selecting within the + * continuumq + */ + protected ArrayBasedCeilRingIterator(final String k, final int t, + final HashAlgorithm hashAlg, final ArrayBasedCeilRing ring) { + super(); + this.ring = ring; + this.hashAlg = hashAlg; + hashVal = hashAlg.hash(k); + remainingTries = t; + key = k; + } + + private void nextHash() { + // this.calculateHash(Integer.toString(tries)+key).hashCode(); + long tmpKey = hashAlg.hash((numTries++) + key); + // This echos the implementation of Long.hashCode() + hashVal += (int) (tmpKey ^ (tmpKey >>> 32)); + hashVal &= 0xffffffffL; /* truncate to 32-bits */ + remainingTries--; + } + + public boolean hasNext() { + return remainingTries > 0; + } + + public MemcachedNode next() { + try { + return ring.findClosestNode(hashVal); + } finally { + nextHash(); + } + } + + public void remove() { + throw new UnsupportedOperationException("remove not supported"); + } + +} diff --git a/src/main/java/net/spy/memcached/KetamaNodeLocator.java b/src/main/java/net/spy/memcached/KetamaNodeLocator.java index fd2e67208..1a46e801c 100644 --- a/src/main/java/net/spy/memcached/KetamaNodeLocator.java +++ b/src/main/java/net/spy/memcached/KetamaNodeLocator.java @@ -34,7 +34,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.SortedMap; import java.util.TreeMap; /** @@ -49,8 +48,7 @@ */ public final class KetamaNodeLocator extends SpyObject implements NodeLocator { - private volatile TreeMap ketamaNodes; - private volatile Collection allNodes; + private volatile ArrayBasedCeilRing ketamaNodes; private final HashAlgorithm hashAlg; private final Map weights; @@ -112,7 +110,7 @@ public KetamaNodeLocator(List nodes, HashAlgorithm alg, * continuum * @param alg The hash algorithm to use when choosing a node in the Ketama * consistent hash continuum - * @param weights node weights for ketama, a map from InetSocketAddress to + * @param nodeWeights node weights for ketama, a map from InetSocketAddress to * weight as Integer * @param configuration node locator configuration */ @@ -120,7 +118,6 @@ public KetamaNodeLocator(List nodes, HashAlgorithm alg, Map nodeWeights, KetamaNodeLocatorConfiguration configuration) { super(); - allNodes = nodes; hashAlg = alg; config = configuration; weights = nodeWeights; @@ -128,13 +125,12 @@ public KetamaNodeLocator(List nodes, HashAlgorithm alg, setKetamaNodes(nodes); } - private KetamaNodeLocator(TreeMap smn, - Collection an, HashAlgorithm alg, - Map nodeWeights, - KetamaNodeLocatorConfiguration conf) { + private KetamaNodeLocator(ArrayBasedCeilRing nodes, + HashAlgorithm alg, + Map nodeWeights, + KetamaNodeLocatorConfiguration conf) { super(); - ketamaNodes = smn; - allNodes = an; + ketamaNodes = nodes; hashAlg = alg; config = conf; weights = nodeWeights; @@ -142,7 +138,7 @@ private KetamaNodeLocator(TreeMap smn, } public Collection getAll() { - return allNodes; + return ketamaNodes.getAllNodes(); } public MemcachedNode getPrimary(final String k) { @@ -152,53 +148,25 @@ public MemcachedNode getPrimary(final String k) { } long getMaxKey() { - return getKetamaNodes().lastKey(); + return ketamaNodes.getMaxPosition(); } MemcachedNode getNodeForKey(long hash) { - final MemcachedNode rv; - if (!ketamaNodes.containsKey(hash)) { - // Java 1.6 adds a ceilingKey method, but I'm still stuck in 1.5 - // in a lot of places, so I'm doing this myself. - SortedMap tailMap = getKetamaNodes().tailMap(hash); - if (tailMap.isEmpty()) { - hash = getKetamaNodes().firstKey(); - } else { - hash = tailMap.firstKey(); - } - } - rv = getKetamaNodes().get(hash); - return rv; + return ketamaNodes.findClosestNode(hash); } public Iterator getSequence(String k) { // Seven searches gives us a 1 in 2^7 chance of hitting the // same dead node all of the time. - return new KetamaIterator(k, 7, getKetamaNodes(), hashAlg); + return new ArrayBasedCeilRingIterator(k, 7, hashAlg,ketamaNodes); } public NodeLocator getReadonlyCopy() { - TreeMap smn = - new TreeMap(getKetamaNodes()); - Collection an = - new ArrayList(allNodes.size()); - - // Rewrite the values a copy of the map. - for (Map.Entry me : smn.entrySet()) { - smn.put(me.getKey(), new MemcachedNodeROImpl(me.getValue())); - } - - // Copy the allNodes collection. - for (MemcachedNode n : allNodes) { - an.add(new MemcachedNodeROImpl(n)); - } - - return new KetamaNodeLocator(smn, an, hashAlg, weights, config); + return new KetamaNodeLocator(ketamaNodes.roClone(), hashAlg, weights, config); } @Override public void updateLocator(List nodes) { - allNodes = nodes; setKetamaNodes(nodes); } @@ -206,7 +174,7 @@ public void updateLocator(List nodes) { * @return the ketamaNodes */ protected TreeMap getKetamaNodes() { - return ketamaNodes; + return (TreeMap)ketamaNodes.asMap(); } /** @@ -260,7 +228,7 @@ protected void setKetamaNodes(List nodes) { } } assert newNodeMap.size() == numReps * nodes.size(); - ketamaNodes = newNodeMap; + ketamaNodes = new ArrayBasedCeilRing(newNodeMap,nodes); } private List ketamaNodePositionsAtIteration(MemcachedNode node, int iteration) { diff --git a/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java b/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java new file mode 100644 index 000000000..057fcdff4 --- /dev/null +++ b/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java @@ -0,0 +1,359 @@ +package net.spy.memcached; + +import junit.framework.TestCase; +import net.spy.memcached.ops.Operation; + +import java.io.IOException; +import java.net.SocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.SelectionKey; +import java.nio.channels.SocketChannel; +import java.util.*; + +public class ArrayBasedCeilRingTest extends TestCase { + + public void testArrayContainingAllNegatives() throws Exception { + long arr[] = new long[] { -5, -3, -1}; + int lastIndex = arr.length-1; + + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(9, arr, lastIndex)); // 0 + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(11,arr, lastIndex)); // 0 + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MAX_VALUE, arr,lastIndex)); // 0 + + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(-10,arr,lastIndex)); // 0 + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(-4,arr,lastIndex)); // 1 + assertEquals(2,ArrayBasedCeilRing.findCeilIndex(-2,arr,lastIndex)); // 2 + } + + public void testSingleValueArray() throws Exception { + long arr[] = new long[] {10}; + int lastIndex = arr.length-1; + + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(9, arr, lastIndex)); // 0 + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(11,arr, lastIndex)); // 0 + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MAX_VALUE, arr,lastIndex)); // 0 + } + + public void testNegativeToPositiveArray() { + // goes from -10000 (in evens) to 9998 + long arr[] = new long[10000]; + int j = -10000; + for(int i = 0;i<10000;i++) { + arr[i] = j + (i * 2); + + } + + int lastIndex = arr.length-1; + + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(-10000, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(-11000, arr, lastIndex)); + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(-9999, arr, lastIndex)); + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(-9998, arr, lastIndex)); + assertEquals(4,ArrayBasedCeilRing.findCeilIndex(-9993, arr, lastIndex)); + assertEquals(9999,ArrayBasedCeilRing.findCeilIndex(9998, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(14999, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(14996, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(156674996, arr, lastIndex)); + assertEquals(6000,ArrayBasedCeilRing.findCeilIndex(2000, arr, lastIndex)); + assertEquals(5000,ArrayBasedCeilRing.findCeilIndex(0, arr, lastIndex)); + assertEquals(5001,ArrayBasedCeilRing.findCeilIndex(1, arr, lastIndex)); + assertEquals(4999,ArrayBasedCeilRing.findCeilIndex(-2, arr, lastIndex)); + assertEquals(5000,ArrayBasedCeilRing.findCeilIndex(-1, arr, lastIndex)); + assertEquals(5005,ArrayBasedCeilRing.findCeilIndex(9, arr, lastIndex)); + + } + + public void testRandomPositiveArray() { + long arr[] = {1, 2, 8, 10, 12, 19, 21, 66, 67, 68, 69, 77, 101}; + int lastIndex = arr.length-1; + + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(-1, arr, lastIndex)); + assertEquals(6,ArrayBasedCeilRing.findCeilIndex(20, arr, lastIndex)); + assertEquals(5,ArrayBasedCeilRing.findCeilIndex(18, arr, lastIndex)); + assertEquals(3,ArrayBasedCeilRing.findCeilIndex(9, arr, lastIndex)); + assertEquals(2,ArrayBasedCeilRing.findCeilIndex(3, arr, lastIndex)); + assertEquals(2,ArrayBasedCeilRing.findCeilIndex(8, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(1, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(0, arr, lastIndex)); + assertEquals(5,ArrayBasedCeilRing.findCeilIndex(19, arr, lastIndex)); + assertEquals(4,ArrayBasedCeilRing.findCeilIndex(11, arr, lastIndex)); + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(2, arr, lastIndex)); + assertEquals(5,ArrayBasedCeilRing.findCeilIndex(13, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MAX_VALUE, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MIN_VALUE, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(1, arr, lastIndex)); + } + + public void test10000PositiveArray() { + long[] arr = new long[10000]; + for(int i = 0;i<10000;i++) { + arr[i] = i*2; + } + int lastIndex = arr.length-1; + + + assertEquals(9999,ArrayBasedCeilRing.findCeilIndex(19998, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(19999, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(20000, arr, lastIndex)); + assertEquals(1000,ArrayBasedCeilRing.findCeilIndex(2000, arr, lastIndex)); + assertEquals(500,ArrayBasedCeilRing.findCeilIndex(999, arr, lastIndex)); + assertEquals(500,ArrayBasedCeilRing.findCeilIndex(1000, arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(-99, arr, lastIndex)); + } + + public void testExceptionThrownOnEmptyArray() { + long[] arr = new long[0]; + int lastIndex = arr.length-1; + + try { + ArrayBasedCeilRing.findCeilIndex(Integer.MIN_VALUE,arr,lastIndex); + fail("IllegalArgumentException expect to be thrown on an empty array"); + } catch(IllegalArgumentException e) { + e.printStackTrace(System.out); + } + } + + + public void testEmptyMapThrowsException() { + Map nodes = new HashMap(); + ArrayBasedCeilRing ring = new ArrayBasedCeilRing(nodes, Collections.EMPTY_LIST); + + try { + ring.findCeilIndex(0); + fail("IllegalArgumentException expect to be thrown on an empty array"); + } catch(IllegalArgumentException e) { + e.printStackTrace(System.out); + } + } + + public void testMapResultsInCorrectlySortedRing() { + Map nodes = new HashMap(); + List nodesRepresented = new ArrayList(2); + + MemcachedNode node1 = new RingTestingMemcachedNode(1234); + MemcachedNode node2 = new RingTestingMemcachedNode(-100234); + + nodesRepresented.add(node1); + nodesRepresented.add(node2); + // 1, 2, 8, 10, 10, 12, 19, 21, 66, 67 }; + nodes.put(8l,node1); + nodes.put(1l,node1); + nodes.put(19l,node1); + nodes.put(10l,node1); + nodes.put(2l,node2); + nodes.put(21l,node2); + nodes.put(67l,node2); + nodes.put(12l,node2); + nodes.put(66l,node2); + + + ArrayBasedCeilRing ring = new ArrayBasedCeilRing(nodes,nodesRepresented); + + assertEquals(2,ring.getAllNodes().size()); + assertEquals(67,ring.getMaxPosition()); + + assertEquals(1234,ring.findClosestNode(7).lastReadDelta()); + assertEquals(-100234,ring.findClosestNode(62).lastReadDelta()); + + + } + + class RingTestingMemcachedNode implements MemcachedNode { + + + private final long lastReadDelta; + public RingTestingMemcachedNode(long lastReadDelta) { + this.lastReadDelta = lastReadDelta; + } + + @Override + public void copyInputQueue() { + + } + + @Override + public Collection destroyInputQueue() { + return null; + } + + @Override + public void setupResend() { + + } + + @Override + public void fillWriteBuffer(boolean optimizeGets) { + + } + + @Override + public void transitionWriteItem() { + + } + + @Override + public Operation getCurrentReadOp() { + return null; + } + + @Override + public Operation removeCurrentReadOp() { + return null; + } + + @Override + public Operation getCurrentWriteOp() { + return null; + } + + @Override + public Operation removeCurrentWriteOp() { + return null; + } + + @Override + public boolean hasReadOp() { + return false; + } + + @Override + public boolean hasWriteOp() { + return false; + } + + @Override + public void addOp(Operation op) { + + } + + @Override + public void insertOp(Operation o) { + + } + + @Override + public int getSelectionOps() { + return 0; + } + + @Override + public ByteBuffer getRbuf() { + return null; + } + + @Override + public ByteBuffer getWbuf() { + return null; + } + + @Override + public SocketAddress getSocketAddress() { + return null; + } + + @Override + public boolean isActive() { + return false; + } + + @Override + public boolean isAuthenticated() { + return false; + } + + @Override + public long lastReadDelta() { + return lastReadDelta; + } + + @Override + public void completedRead() { + + } + + @Override + public void reconnecting() { + + } + + @Override + public void connected() { + + } + + @Override + public int getReconnectCount() { + return 0; + } + + @Override + public void registerChannel(SocketChannel ch, SelectionKey selectionKey) { + + } + + @Override + public void setChannel(SocketChannel to) { + + } + + @Override + public SocketChannel getChannel() { + return null; + } + + @Override + public void setSk(SelectionKey to) { + + } + + @Override + public SelectionKey getSk() { + return null; + } + + @Override + public int getBytesRemainingToWrite() { + return 0; + } + + @Override + public int writeSome() throws IOException { + return 0; + } + + @Override + public void fixupOps() { + + } + + @Override + public void authComplete() { + + } + + @Override + public void setupForAuth() { + + } + + @Override + public void setContinuousTimeout(boolean timedOut) { + + } + + @Override + public int getContinuousTimeout() { + return 0; + } + + @Override + public MemcachedConnection getConnection() { + return null; + } + + @Override + public void setConnection(MemcachedConnection connection) { + + } + } +} \ No newline at end of file diff --git a/src/test/java/net/spy/memcached/QueueOverflowTest.java b/src/test/java/net/spy/memcached/QueueOverflowTest.java index ef18f1332..c17daead8 100644 --- a/src/test/java/net/spy/memcached/QueueOverflowTest.java +++ b/src/test/java/net/spy/memcached/QueueOverflowTest.java @@ -156,7 +156,7 @@ public void testOverflowingReadQueue() throws Exception { // OK, at least we got one back. } } - Thread.sleep(500); + Thread.sleep(5000); // give enough time for the client to reconnect assertTrue(client.set("kx", 0, "woo").get(5, TimeUnit.SECONDS)); } } diff --git a/src/test/java/net/spy/memcached/internal/DummyListenableFuture.java b/src/test/java/net/spy/memcached/internal/DummyListenableFuture.java index 86d89ff21..d5367163b 100644 --- a/src/test/java/net/spy/memcached/internal/DummyListenableFuture.java +++ b/src/test/java/net/spy/memcached/internal/DummyListenableFuture.java @@ -38,7 +38,7 @@ public class DummyListenableFuture private boolean done; private boolean cancelled = false; - private T content = null; + private volatile T content = null; public DummyListenableFuture(boolean alreadyDone, ExecutorService service) { super(service); @@ -78,8 +78,8 @@ public T get(long l, TimeUnit tu) } public void set(T c) { - notifyListeners(); content = c; + notifyListeners(); } @Override From 2d6149a9f53218770484235de2d8c322063f18b7 Mon Sep 17 00:00:00 2001 From: dominictootell Date: Sun, 16 Aug 2015 08:12:59 +0100 Subject: [PATCH 2/3] remove bounds checking branch, not required. Bounds are checked early --- .../net/spy/memcached/ArrayBasedCeilRing.java | 4 ++-- .../spy/memcached/ArrayBasedCeilRingTest.java | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java b/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java index ad1622ea3..bc41190f1 100644 --- a/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java +++ b/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java @@ -140,11 +140,11 @@ static int findCeilIndex(final long hashVal, final long sortedNodePositions[], f if (midVal == hashVal) return mid; else if (midVal < hashVal) { low = mid + 1; - if (low <= high && sortedNodePositions[low] >= hashVal) return low; + if (sortedNodePositions[low] >= hashVal) return low; } else if (midVal > hashVal) { high = mid - 1; - if (high >= low && sortedNodePositions[high] < hashVal) return mid; + if (sortedNodePositions[high] < hashVal) return mid; } } } diff --git a/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java b/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java index 057fcdff4..f5e84d62b 100644 --- a/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java +++ b/src/test/java/net/spy/memcached/ArrayBasedCeilRingTest.java @@ -34,6 +34,26 @@ public void testSingleValueArray() throws Exception { assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MAX_VALUE, arr,lastIndex)); // 0 } + public void testDoubleValueArray() throws Exception { + long arr[] = new long[] {5,10}; + int lastIndex = arr.length-1; + + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(4, arr, lastIndex)); + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(6,arr, lastIndex)); + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(6,arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MAX_VALUE, arr,lastIndex)); + } + + public void testThreeValueArray() throws Exception { + long arr[] = new long[] {5,7,10}; + int lastIndex = arr.length-1; + + assertEquals(1,ArrayBasedCeilRing.findCeilIndex(6, arr, lastIndex)); + assertEquals(2,ArrayBasedCeilRing.findCeilIndex(8,arr, lastIndex)); + assertEquals(2,ArrayBasedCeilRing.findCeilIndex(10,arr, lastIndex)); + assertEquals(0,ArrayBasedCeilRing.findCeilIndex(Integer.MAX_VALUE, arr,lastIndex)); + } + public void testNegativeToPositiveArray() { // goes from -10000 (in evens) to 9998 long arr[] = new long[10000]; From ffd11c8d7af44a4ff92e86dde984182c5d1ed7ab Mon Sep 17 00:00:00 2001 From: dominictootell Date: Sun, 16 Aug 2015 10:53:19 +0100 Subject: [PATCH 3/3] remove unneeded branch --- src/main/java/net/spy/memcached/ArrayBasedCeilRing.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java b/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java index bc41190f1..ee55a3a9f 100644 --- a/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java +++ b/src/main/java/net/spy/memcached/ArrayBasedCeilRing.java @@ -142,7 +142,7 @@ else if (midVal < hashVal) { low = mid + 1; if (sortedNodePositions[low] >= hashVal) return low; } - else if (midVal > hashVal) { + else { high = mid - 1; if (sortedNodePositions[high] < hashVal) return mid; }