diff --git a/src/java/voldemort/store/routed/action/PerformParallelGetAllRequests.java b/src/java/voldemort/store/routed/action/PerformParallelGetAllRequests.java index 444fdd930e..357f30c97f 100644 --- a/src/java/voldemort/store/routed/action/PerformParallelGetAllRequests.java +++ b/src/java/voldemort/store/routed/action/PerformParallelGetAllRequests.java @@ -16,6 +16,7 @@ package voldemort.store.routed.action; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -137,24 +138,26 @@ public void requestComplete(Object result, long requestTime) { successCount.increment(); List> retrieved = values.get(key); + if(retrieved == null) { + retrieved = new ArrayList>(); + } /* * retrieved can be null if there are no values for the key * provided */ - if(retrieved != null) { - List> existing = pipelineData.getResult().get(key); + List> existing = pipelineData.getResult().get(key); - if(existing == null) - pipelineData.getResult().put(key, Lists.newArrayList(retrieved)); - else - existing.addAll(retrieved); - } + if(existing == null) + pipelineData.getResult().put(key, Lists.newArrayList(retrieved)); + else + existing.addAll(retrieved); HashSet zoneResponses = null; if(pipelineData.getKeyToZoneResponse().containsKey(key)) { zoneResponses = pipelineData.getKeyToZoneResponse().get(key); } else { zoneResponses = new HashSet(); + pipelineData.getKeyToZoneResponse().put(key, zoneResponses); } zoneResponses.add(response.getNode().getZoneId()); } diff --git a/src/java/voldemort/store/routed/action/PerformSerialGetAllRequests.java b/src/java/voldemort/store/routed/action/PerformSerialGetAllRequests.java index c8135781d4..f1719a8f55 100644 --- a/src/java/voldemort/store/routed/action/PerformSerialGetAllRequests.java +++ b/src/java/voldemort/store/routed/action/PerformSerialGetAllRequests.java @@ -79,13 +79,13 @@ public void execute(Pipeline pipeline) { boolean zoneRequirement = false; MutableInt successCount = pipelineData.getSuccessCount(key); - if(logger.isDebugEnabled()) - logger.debug("GETALL for key " + key + " (keyRef: " + System.identityHashCode(key) - + ") successes: " + successCount.intValue() + " preferred: " + preferred - + " required: " + required); + if(logger.isDebugEnabled()) + logger.debug("GETALL for key " + key + " (keyRef: " + System.identityHashCode(key) + + ") successes: " + successCount.intValue() + " preferred: " + + preferred + " required: " + required); if(successCount.intValue() >= preferred) { - if(pipelineData.getZonesRequired() != null) { + if(pipelineData.getZonesRequired() != null && pipelineData.getZonesRequired() > 0) { if(pipelineData.getKeyToZoneResponse().containsKey(key)) { int zonesSatisfied = pipelineData.getKeyToZoneResponse().get(key).size(); @@ -149,6 +149,7 @@ public void execute(Pipeline pipeline) { zoneResponses = pipelineData.getKeyToZoneResponse().get(key); } else { zoneResponses = new HashSet(); + pipelineData.getKeyToZoneResponse().put(key, zoneResponses); } zoneResponses.add(response.getNode().getZoneId()); diff --git a/test/unit/voldemort/store/AbstractStoreTest.java b/test/unit/voldemort/store/AbstractStoreTest.java index 93c5efb719..e2adf72f01 100644 --- a/test/unit/voldemort/store/AbstractStoreTest.java +++ b/test/unit/voldemort/store/AbstractStoreTest.java @@ -125,8 +125,8 @@ public void testNullKeys() throws Exception { // this is good } try { - store.getAll(Collections. singleton(null), Collections. singletonMap(null, - null)); + store.getAll(Collections. singleton(null), + Collections. singletonMap(null, null)); fail("Store should not getAll null keys!"); } catch(IllegalArgumentException e) { // this is good @@ -141,12 +141,13 @@ public void testNullKeys() throws Exception { @Test public void testPutNullValue() { - // Store store = getStore(); - // K key = getKey(); - // store.put(key, new Versioned(null)); - // List> found = store.get(key); - // assertEquals("Wrong number of values.", 1, found.size()); - // assertEquals("Returned non-null value.", null, found.get(0).getValue()); + // Store store = getStore(); + // K key = getKey(); + // store.put(key, new Versioned(null)); + // List> found = store.get(key); + // assertEquals("Wrong number of values.", 1, found.size()); + // assertEquals("Returned non-null value.", null, + // found.get(0).getValue()); } @Test @@ -155,12 +156,8 @@ public void testGetAndDeleteNonExistentKey() throws Exception { Store store = getStore(); List> found = store.get(key, null); assertEquals("Found non-existent key: " + found, 0, found.size()); - assertTrue("Delete of non-existent key succeeded.", !store.delete(key, getClock(1, - 1, - 2, - 2, - 3, - 3))); + assertTrue("Delete of non-existent key succeeded.", + !store.delete(key, getClock(1, 1, 2, 2, 3, 3))); } private void testObsoletePutFails(String message, @@ -315,7 +312,21 @@ public void testGetAll() throws Exception { public void testGetAllWithAbsentKeys() throws Exception { Store store = getStore(); Map>> result = store.getAll(getKeys(3), null); - assertEquals(0, result.size()); + boolean resultZero = (result.size() == 0); + boolean resultEmpty = true; + if(!resultZero) { + if(result.get(result.keySet().toArray()[0]).size() != 0) { + resultEmpty = false; + } + if(result.get(result.keySet().toArray()[1]).size() != 0) { + resultEmpty = false; + } + if(result.get(result.keySet().toArray()[2]).size() != 0) { + resultEmpty = false; + } + } + assertTrue(resultZero || resultEmpty); + } @Test diff --git a/test/unit/voldemort/store/routed/GetallNodeReachTest.java b/test/unit/voldemort/store/routed/GetallNodeReachTest.java new file mode 100644 index 0000000000..5a31b0b93c --- /dev/null +++ b/test/unit/voldemort/store/routed/GetallNodeReachTest.java @@ -0,0 +1,91 @@ +package voldemort.store.routed; + +import static org.junit.Assert.assertEquals; +import static voldemort.VoldemortTestConstants.getFourNodeClusterWithZones; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.Executors; + +import org.junit.Before; +import org.junit.Test; + +import voldemort.TestUtils; +import voldemort.client.RoutingTier; +import voldemort.client.TimeoutConfig; +import voldemort.cluster.Cluster; +import voldemort.cluster.Node; +import voldemort.cluster.failuredetector.NoopFailureDetector; +import voldemort.routing.RoutingStrategyType; +import voldemort.serialization.SerializerDefinition; +import voldemort.store.Store; +import voldemort.store.StoreDefinition; +import voldemort.store.StoreDefinitionBuilder; +import voldemort.store.memory.InMemoryStorageConfiguration; +import voldemort.store.memory.InMemoryStorageEngine; +import voldemort.utils.ByteArray; +import voldemort.versioning.Versioned; + +import com.google.common.collect.Maps; + +public class GetallNodeReachTest { + + private Cluster cluster; + + @Before + public void setUp() throws Exception { + cluster = getFourNodeClusterWithZones(); + } + + @Test + public void testGetallTouchOne() throws Exception { + RoutedStore store = null; + HashMap zoneReplicationFactor = new HashMap(); + zoneReplicationFactor.put(0, 2); + zoneReplicationFactor.put(1, 1); + zoneReplicationFactor.put(2, 1); + StoreDefinition storeDef = new StoreDefinitionBuilder().setName("test") + .setType(InMemoryStorageConfiguration.TYPE_NAME) + .setRoutingPolicy(RoutingTier.CLIENT) + .setRoutingStrategyType(RoutingStrategyType.ZONE_STRATEGY) + .setReplicationFactor(4) + .setZoneReplicationFactor(zoneReplicationFactor) + .setKeySerializer(new SerializerDefinition("string")) + .setValueSerializer(new SerializerDefinition("string")) + .setPreferredReads(2) + .setRequiredReads(1) + .setPreferredWrites(1) + .setRequiredWrites(1) + .setZoneCountReads(0) + .setZoneCountWrites(0) + .build(); + Map> subStores = Maps.newHashMap(); + for(Node n: cluster.getNodes()) { + Store subStore = new InMemoryStorageEngine("test"); + subStores.put(n.getId(), subStore); + + } + RoutedStoreFactory routedStoreFactory = new RoutedStoreFactory(true, + Executors.newFixedThreadPool(2), + new TimeoutConfig(1000L, + false)); + + store = routedStoreFactory.create(cluster, + storeDef, + subStores, + true, + new NoopFailureDetector()); + Versioned v = Versioned.value("v".getBytes()); + subStores.get(0).put(TestUtils.toByteArray("k011"), v, null); + subStores.get(1).put(TestUtils.toByteArray("k011"), v, null); + subStores.get(2).put(TestUtils.toByteArray("k100"), v, null); + List keys011 = new ArrayList(); + keys011.add(TestUtils.toByteArray("k011")); + List keys100 = new ArrayList(); + keys100.add(TestUtils.toByteArray("k100")); + assertEquals(store.getAll(keys011, null).get(TestUtils.toByteArray("k011")).size(), 2); + assertEquals(store.getAll(keys100, null).get(TestUtils.toByteArray("k100")).size(), 0); + } +}