From aea1b9069c24060ff443b2a62021cae94a327353 Mon Sep 17 00:00:00 2001 From: Bhavani Sudha Saktheeswaran Date: Tue, 15 Apr 2014 16:21:26 -0700 Subject: [PATCH] Wrapping RESTClient in LazyStoreClient 1) Separated inner config class from RESTClientFactory to RESTClientFactoryConfig 2) RESTClientFactory.getSToreClient(storeName) now returns a LazyStoreClient 3) Removed the instantInit boolean in LazyStoreClientConstructor. Instead introducing a boolean for use by RESTClientFactory Removing LazyStoreClientTest.java --- .../voldemort/restclient/RESTClientFactory.java | 56 +++++----------- .../restclient/RESTClientFactoryConfig.java | 34 ++++++++++ .../voldemort/restclient/SampleRESTClient.java | 2 +- .../restclient/VoldemortThinClientShell.java | 2 +- .../test/voldemort/client/RestClientTest.java | 4 +- src/java/voldemort/client/LazyStoreClient.java | 18 ++--- .../unit/voldemort/client/LazyStoreClientTest.java | 77 ---------------------- 7 files changed, 64 insertions(+), 129 deletions(-) create mode 100644 contrib/restclient/src/java/voldemort/restclient/RESTClientFactoryConfig.java delete mode 100644 test/unit/voldemort/client/LazyStoreClientTest.java diff --git a/contrib/restclient/src/java/voldemort/restclient/RESTClientFactory.java b/contrib/restclient/src/java/voldemort/restclient/RESTClientFactory.java index c876db263..eb6eb6c47 100644 --- a/contrib/restclient/src/java/voldemort/restclient/RESTClientFactory.java +++ b/contrib/restclient/src/java/voldemort/restclient/RESTClientFactory.java @@ -4,11 +4,12 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Properties; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import org.apache.log4j.Logger; +import voldemort.client.LazyStoreClient; import voldemort.client.StoreClient; import voldemort.client.StoreClientFactory; import voldemort.cluster.failuredetector.FailureDetector; @@ -54,6 +55,7 @@ public class RESTClientFactory implements StoreClientFactory { private final TransportClient transportClient; private final StoreClientFactoryStats RESTClientFactoryStats; private Client d2Client; + private RESTClientFactoryConfig restClientFactoryConfig; /** * This list holds a reference to all the raw stores created by this @@ -65,10 +67,9 @@ public class RESTClientFactory implements StoreClientFactory { private HashMap keySerializerMap; private HashMap valueSerializerMap; - public RESTClientFactory(Config config) { - this.config = new RESTClientConfig(config.getClientConfig()); - this.d2Client = config.getD2Client(); - + public RESTClientFactory(RESTClientFactoryConfig config) { + this.restClientFactoryConfig = config; + this.config = new RESTClientConfig(restClientFactoryConfig.getClientConfig()); this.stats = new StoreStats(); this.rawStoreList = new ArrayList(); @@ -92,8 +93,15 @@ public class RESTClientFactory implements StoreClientFactory { * @return */ @Override - public StoreClient getStoreClient(String storeName) { - return getStoreClient(storeName, null); + public StoreClient getStoreClient(final String storeName) { + return new LazyStoreClient(new Callable>() { + + @Override + public StoreClient call() throws Exception { + return getStoreClient(storeName, null); + } + }, true); + } /** @@ -120,6 +128,7 @@ public class RESTClientFactory implements StoreClientFactory { // The lowest layer : Transporting request to coordinator R2Store r2store = null; + this.d2Client = restClientFactoryConfig.getD2Client(); if(this.d2Client == null) { r2store = new R2Store(storeName, this.config.getHttpBootstrapURL(), @@ -224,37 +233,4 @@ public class RESTClientFactory implements StoreClientFactory { return null; } - /** - * Inner configuration class. - */ - public static class Config { - - private Properties clientConfig; - private Client d2Client; - - public Config() {} - - public Config(Properties clientConfig, Client d2Client) { - setClientConfig(clientConfig); - setD2Client(d2Client); - } - - public Properties getClientConfig() { - return clientConfig; - } - - public void setClientConfig(Properties clientConfig) { - this.clientConfig = clientConfig; - } - - public Client getD2Client() { - return d2Client; - } - - public void setD2Client(Client d2Client) { - this.d2Client = d2Client; - } - - } - } diff --git a/contrib/restclient/src/java/voldemort/restclient/RESTClientFactoryConfig.java b/contrib/restclient/src/java/voldemort/restclient/RESTClientFactoryConfig.java new file mode 100644 index 000000000..8433d6c54 --- /dev/null +++ b/contrib/restclient/src/java/voldemort/restclient/RESTClientFactoryConfig.java @@ -0,0 +1,34 @@ +package voldemort.restclient; + +import java.util.Properties; + +import com.linkedin.r2.transport.common.Client; + +public class RESTClientFactoryConfig { + + private Properties clientConfig; + private Client d2Client; + + public RESTClientFactoryConfig() {} + + public RESTClientFactoryConfig(Properties clientConfig, Client d2Client) { + setClientConfig(clientConfig); + setD2Client(d2Client); + } + + public Properties getClientConfig() { + return clientConfig; + } + + public void setClientConfig(Properties clientConfig) { + this.clientConfig = clientConfig; + } + + public Client getD2Client() { + return d2Client; + } + + public void setD2Client(Client d2Client) { + this.d2Client = d2Client; + } +} diff --git a/contrib/restclient/src/java/voldemort/restclient/SampleRESTClient.java b/contrib/restclient/src/java/voldemort/restclient/SampleRESTClient.java index 5badc6a36..06ea82e0f 100644 --- a/contrib/restclient/src/java/voldemort/restclient/SampleRESTClient.java +++ b/contrib/restclient/src/java/voldemort/restclient/SampleRESTClient.java @@ -40,7 +40,7 @@ public class SampleRESTClient { props.setProperty(ClientConfig.BOOTSTRAP_URLS_PROPERTY, "http://localhost:8080"); props.setProperty(ClientConfig.ROUTING_TIMEOUT_MS_PROPERTY, "1500"); - RESTClientFactory.Config mainConfig = new RESTClientFactory.Config(props, null); + RESTClientFactoryConfig mainConfig = new RESTClientFactoryConfig(props, null); RESTClientFactory factory = new RESTClientFactory(mainConfig); StoreClient clientStore = factory.getStoreClient("test"); diff --git a/contrib/restclient/src/java/voldemort/restclient/VoldemortThinClientShell.java b/contrib/restclient/src/java/voldemort/restclient/VoldemortThinClientShell.java index cd93432e8..be5c0a40f 100644 --- a/contrib/restclient/src/java/voldemort/restclient/VoldemortThinClientShell.java +++ b/contrib/restclient/src/java/voldemort/restclient/VoldemortThinClientShell.java @@ -55,7 +55,7 @@ public class VoldemortThinClientShell extends VoldemortClientShell { Properties properties = new Properties(); properties.setProperty(ClientConfig.BOOTSTRAP_URLS_PROPERTY, bootstrapUrl); properties.setProperty(ClientConfig.ROUTING_TIMEOUT_MS_PROPERTY, "1500"); - RESTClientFactory.Config mainConfig = new RESTClientFactory.Config(properties, null); + RESTClientFactoryConfig mainConfig = new RESTClientFactoryConfig(properties, null); restClientFactory = new RESTClientFactory(mainConfig); this.client = restClientFactory.getStoreClient(storeName); } diff --git a/contrib/restclient/test/voldemort/client/RestClientTest.java b/contrib/restclient/test/voldemort/client/RestClientTest.java index 15a2827ac..44e6cfd68 100644 --- a/contrib/restclient/test/voldemort/client/RestClientTest.java +++ b/contrib/restclient/test/voldemort/client/RestClientTest.java @@ -21,6 +21,7 @@ import voldemort.cluster.Cluster; import voldemort.rest.coordinator.CoordinatorConfig; import voldemort.rest.coordinator.CoordinatorService; import voldemort.restclient.RESTClientFactory; +import voldemort.restclient.RESTClientFactoryConfig; import voldemort.server.VoldemortServer; import voldemort.store.socket.SocketStoreFactory; import voldemort.store.socket.clientrequest.ClientRequestExecutorPool; @@ -93,10 +94,9 @@ public class RestClientTest extends DefaultStoreClientTest { props.setProperty(ClientConfig.BOOTSTRAP_URLS_PROPERTY, "http://localhost:9999"); props.setProperty(ClientConfig.ROUTING_TIMEOUT_MS_PROPERTY, "1500"); - RESTClientFactory.Config mainConfig = new RESTClientFactory.Config(props, null); + RESTClientFactoryConfig mainConfig = new RESTClientFactoryConfig(props, null); RESTClientFactory factory = new RESTClientFactory(mainConfig); - this.client = factory.getStoreClient(STORE_NAME); } diff --git a/src/java/voldemort/client/LazyStoreClient.java b/src/java/voldemort/client/LazyStoreClient.java index e7524d2d2..4e925c207 100644 --- a/src/java/voldemort/client/LazyStoreClient.java +++ b/src/java/voldemort/client/LazyStoreClient.java @@ -41,7 +41,7 @@ public class LazyStoreClient implements StoreClient { private StoreClient storeClient; public LazyStoreClient(Callable> storeClientThunk) { - this(storeClientThunk, true); + this(storeClientThunk, false); } /** @@ -53,15 +53,17 @@ public class LazyStoreClient implements StoreClient { * @param instantInit A boolean flag when set indicates that we should try * to immediately bootstrap */ - public LazyStoreClient(Callable> storeClientThunk, boolean instantInit) { + public LazyStoreClient(Callable> storeClientThunk, boolean wrapsRESTClient) { this.storeClientThunk = storeClientThunk; - if(instantInit) { - try { - storeClient = initStoreClient(); - } catch(Exception e) { - storeClient = null; - e.printStackTrace(); + try { + storeClient = initStoreClient(); + } catch(Exception e) { + storeClient = null; + logger.info(e.getMessage()); + if(wrapsRESTClient) { + logger.info("D2 client might not have been completely initialized. Trying on the next call ..."); + } else { logger.info("Could not bootstrap right away. Trying on the next call ... "); } } diff --git a/test/unit/voldemort/client/LazyStoreClientTest.java b/test/unit/voldemort/client/LazyStoreClientTest.java deleted file mode 100644 index 2baec6be1..000000000 --- a/test/unit/voldemort/client/LazyStoreClientTest.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2008-2011 LinkedIn, Inc - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ - -package voldemort.client; - -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - -import java.util.concurrent.Callable; - -import org.junit.Before; -import org.junit.Test; - -import voldemort.serialization.Serializer; -import voldemort.serialization.StringSerializer; -import voldemort.utils.SystemTime; - -/** - */ -public class LazyStoreClientTest extends DefaultStoreClientTest { - - private MockStoreClientFactory factory; - - @Override - @Before - public void setUp() { - this.nodeId = 0; - this.time = SystemTime.INSTANCE; - Serializer serializer = new StringSerializer(); - this.factory = new MockStoreClientFactory(serializer, - serializer, - null, - serializer, - nodeId, - time); - this.client = newLazyStoreClient(factory); - } - - @Test - public void testInitializationShouldBeLazy() { - StoreClientFactory spyFactory = spy(factory); - LazyStoreClient spyLazyClient = spy(newLazyStoreClient(spyFactory)); - - // Check that we don't initialize upon construction - verify(spyFactory, times(0)).getStoreClient("test"); - - // Check that we initialize once and only once - for(int i = 0; i < 10; i++) - spyLazyClient.get("test"); - - verify(spyFactory, times(1)).getStoreClient("test"); - verify(spyLazyClient, times(1)).initStoreClient(); - } - - private LazyStoreClient newLazyStoreClient(final StoreClientFactory factory) { - return new LazyStoreClient(new Callable>() { - - public StoreClient call() throws Exception { - return factory.getStoreClient("test"); - } - }, false); - } -} -- 2.11.4.GIT