From 57968cc1d459161c7d29916fdc39cfe149629a30 Mon Sep 17 00:00:00 2001 From: Francisco Perez-Sorrosal Date: Thu, 1 Feb 2018 15:19:09 -0800 Subject: [PATCH] [OMID-87] Fix BatchPool initialization Added config.setMaxIdle(poolSize + 1) to the configuration. Added some tests to prove it works. Change-Id: Idc32ffa8472e87defdc540abbb901cfba700eb05 --- .../org/apache/omid/tso/BatchPoolModule.java | 6 +- .../org/apache/omid/tso/TestBatchPool.java | 128 ++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tso-server/src/test/java/org/apache/omid/tso/TestBatchPool.java diff --git a/tso-server/src/main/java/org/apache/omid/tso/BatchPoolModule.java b/tso-server/src/main/java/org/apache/omid/tso/BatchPoolModule.java index c28f3aa30..4e4c26a56 100644 --- a/tso-server/src/main/java/org/apache/omid/tso/BatchPoolModule.java +++ b/tso-server/src/main/java/org/apache/omid/tso/BatchPoolModule.java @@ -52,11 +52,15 @@ ObjectPool getBatchPool() throws Exception { LOG.info("Pool Size (# of Batches) {}; Batch Size {}", poolSize, batchSize); LOG.info("Total Batch Size (Pool size * Batch Size): {}", poolSize * batchSize); + // Setup ObjectPool behaviour GenericObjectPoolConfig config = new GenericObjectPoolConfig(); config.setMaxTotal(poolSize); + config.setMaxIdle(poolSize + 1); // This avoids GenericObjectPool to destroy the batches when returned to + // the pool during the pre-creation below config.setBlockWhenExhausted(true); GenericObjectPool batchPool = new GenericObjectPool<>(new Batch.BatchFactory(batchSize), config); - LOG.info("Pre-creating objects in the pool..."); // TODO There should be a better way to do this + LOG.info("Pre-creating objects in the pool..."); + // TODO There should be a better way to do the pre-creation below avoiding the two loops List batches = new ArrayList<>(poolSize); for (int i = 0; i < poolSize; i++) { batches.add(batchPool.borrowObject()); diff --git a/tso-server/src/test/java/org/apache/omid/tso/TestBatchPool.java b/tso-server/src/test/java/org/apache/omid/tso/TestBatchPool.java new file mode 100644 index 000000000..f4c187516 --- /dev/null +++ b/tso-server/src/test/java/org/apache/omid/tso/TestBatchPool.java @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.omid.tso; + +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.TypeLiteral; +import org.apache.commons.pool2.ObjectPool; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; + +public class TestBatchPool { + + private static final Logger LOG = LoggerFactory.getLogger(TestBatchPool.class); + + private static final int CONCURRENT_WRITERS = 16; + private static final int BATCH_SIZE = 1000; + + + private Injector injector; + + @BeforeMethod + void setup() { + + TSOServerConfig tsoServerConfig = new TSOServerConfig(); + tsoServerConfig.setNumConcurrentCTWriters(CONCURRENT_WRITERS); + tsoServerConfig.setBatchSizePerCTWriter(BATCH_SIZE); + + // Injector to get the element under test: the ObjectPool returned by Guice's BatchPoolModule + injector = Guice.createInjector(new BatchPoolModule(tsoServerConfig)); + + } + + @Test(timeOut = 10_000) + public void testBatchPoolObtainedIsSingleton() { + + final ObjectPool instance1 = injector.getInstance(Key.get(new TypeLiteral>() {})); + final ObjectPool instance2 = injector.getInstance(Key.get(new TypeLiteral>() {})); + + assertEquals(instance1, instance2, "Objects are NOT equal !"); + + } + + @Test(timeOut = 10_000) + public void testBatchPoolInitializesAllBatchObjectsAsIdle() throws Exception { + + final ObjectPool batchPool = injector.getInstance(Key.get(new TypeLiteral>() {})); + + assertEquals(batchPool.getNumActive(), 0); + assertEquals(batchPool.getNumIdle(), CONCURRENT_WRITERS); + + // Now make all of them active and check it below + for (int i = 0; i < CONCURRENT_WRITERS; i++) { + batchPool.borrowObject(); + } + + assertEquals(batchPool.getNumActive(), CONCURRENT_WRITERS); + assertEquals(batchPool.getNumIdle(), 0); + + } + + @Test(timeOut = 10_000) + public void testBatchPoolBlocksWhenAllObjectsAreActive() throws Exception { + + ExecutorService executor = Executors.newCachedThreadPool(); + + final ObjectPool batchPool = injector.getInstance(Key.get(new TypeLiteral>() {})); + + // Try to get one more batch than the number of concurrent writers set + for (int i = 0; i < CONCURRENT_WRITERS + 1; i++) { + + // Wrap the call to batchPool.borrowObject() in a task to detect when is blocked by the ObjectPool + Callable task = new Callable() { + public Batch call() throws Exception { + return batchPool.borrowObject(); + } + }; + + Future future = executor.submit(task); + + try { + /** The future below should return immediately except for the last one, which should be blocked by + the ObjectPool as per the configuration setup in the {@link BatchPoolModule} */ + Batch batch = future.get(1, TimeUnit.SECONDS); + LOG.info("Batch {} returned with success", batch.toString()); + } catch (TimeoutException ex) { + if (i < CONCURRENT_WRITERS) { + fail(); + } else { + LOG.info("Yaaaayyyyy! This is the blocked call!"); + } + } finally { + future.cancel(true); // may or may not desire this + } + + } + + } + +} \ No newline at end of file