[TACHYON-646] Use createNewClassInstance to improve Allocator factory

add some comments for patch [TACHYON-646] Improve Allocator factory
Alluxio · Jul 20, 2015 · 1354f23 · 1354f23
1 parent f376544
commit 1354f23
Show file tree

Hide file tree

Showing 11 changed files with 116 additions and 127 deletions.
diff --git a/common/src/main/java/tachyon/Constants.java b/common/src/main/java/tachyon/Constants.java
@@ -172,7 +172,7 @@ public class Constants {
   public static final String WORKER_NETTY_SHUTDOWN_TIMEOUT =
       "tachyon.worker.network.netty.shutdown.timeout";
   public static final String WORKER_EVICT_STRATEGY_TYPE = "tachyon.worker.evict.strategy";
-  public static final String WORKER_ALLOCATE_STRATEGY_TYPE = "tachyon.worker.allocate.strategy";
+  public static final String WORKER_ALLOCATE_STRATEGY = "tachyon.worker.allocate.strategy.class";
   public static final String WORKER_MAX_TIERED_STORAGE_LEVEL =
       "tachyon.worker.tieredstore.level.max";
   /**

diff --git a/common/src/main/resources/tachyon-default.properties b/common/src/main/resources/tachyon-default.properties
@@ -69,7 +69,7 @@ tachyon.worker.network.netty.file.transfer=MAPPED
 tachyon.worker.memory.size=128MB
 tachyon.worker.network.netty.watermark.high=32KB
 tachyon.worker.network.netty.watermark.low=8KB
-tachyon.worker.allocate.strategy=MAX_FREE
+tachyon.worker.allocate.strategy.class=tachyon.worker.block.allocator.MaxFreeAllocator
 tachyon.worker.evict.strategy=LRU
 tachyon.worker.tieredstore.level.max=1
 tachyon.worker.tieredstore.level0.alias=MEM

diff --git a/servers/src/main/java/tachyon/worker/block/TieredBlockStore.java b/servers/src/main/java/tachyon/worker/block/TieredBlockStore.java
@@ -42,8 +42,6 @@
 import tachyon.thrift.InvalidPathException;
 import tachyon.util.CommonUtils;
 import tachyon.worker.block.allocator.Allocator;
-import tachyon.worker.block.allocator.AllocatorFactory;
-import tachyon.worker.block.allocator.AllocatorType;
 import tachyon.worker.block.evictor.EvictionPlan;
 import tachyon.worker.block.evictor.Evictor;
 import tachyon.worker.block.evictor.EvictorFactory;
@@ -94,12 +92,10 @@ public TieredBlockStore(TachyonConf tachyonConf) {
     mMetaManager = BlockMetadataManager.newBlockMetadataManager(mTachyonConf);
     mLockManager = new BlockLockManager(mMetaManager);
 
-    AllocatorType allocatorType =
-        mTachyonConf.getEnum(Constants.WORKER_ALLOCATE_STRATEGY_TYPE, AllocatorType.DEFAULT);
     BlockMetadataManagerView initManagerView =
         new BlockMetadataManagerView(mMetaManager, Collections.<Integer>emptySet(),
             Collections.<Long>emptySet());
-    mAllocator = AllocatorFactory.create(allocatorType, initManagerView);
+    mAllocator = Allocator.Factory.createAllocator(mTachyonConf, initManagerView);
     if (mAllocator instanceof BlockStoreEventListener) {
       registerBlockStoreEventListener((BlockStoreEventListener) mAllocator);
     }

diff --git a/servers/src/main/java/tachyon/worker/block/allocator/Allocator.java b/servers/src/main/java/tachyon/worker/block/allocator/Allocator.java
@@ -15,6 +15,12 @@
 
 package tachyon.worker.block.allocator;
 
+import com.google.common.base.Preconditions;
+import com.google.common.base.Throwables;
+
+import tachyon.Constants;
+import tachyon.conf.TachyonConf;
+import tachyon.util.CommonUtils;
 import tachyon.worker.block.BlockMetadataManagerView;
 import tachyon.worker.block.BlockStoreLocation;
 import tachyon.worker.block.meta.TempBlockMeta;
@@ -23,11 +29,32 @@
  * Interface for the allocation policy of Tachyon managed data.
  */
 public interface Allocator {
+
+  class Factory {
+    /**
+     * Create a new {@link Allocator} instance, will return {@link MaxFreeAllocator} by default
+     *
+     * @param conf Tachyon conf defined Allocator type
+     * @param view BlockMetadataManagerView to pass to Allocator
+     * @return the generated Allocator, it will be a {@link MaxFreeAllocator} by default
+     */
+    public static Allocator createAllocator(TachyonConf conf, BlockMetadataManagerView view) {
+      BlockMetadataManagerView managerView = Preconditions.checkNotNull(view);
+      try {
+        return CommonUtils.createNewClassInstance(
+            conf.getClass(Constants.WORKER_ALLOCATE_STRATEGY, MaxFreeAllocator.class),
+            new Class[]{BlockMetadataManagerView.class}, new Object[]{managerView});
+      } catch (Exception e) {
+        throw Throwables.propagate(e);
+      }
+    }
+  }
+
   /**
    * Allocates a block from the given block store location under a given view.
    * The location can be a specific location,
    * or {@link BlockStoreLocation#anyTier()} or {@link BlockStoreLocation#anyDirInTier(int)}.
-   * The view is generated by a {@link BlockStore}.
+   * The view is generated by a {@link tachyon.worker.block.BlockStore}.
    *
    * @param userId the ID of user to apply for the block allocation
    * @param blockId the ID of the block

diff --git a/servers/src/main/java/tachyon/worker/block/allocator/AllocatorFactory.java b/servers/src/main/java/tachyon/worker/block/allocator/AllocatorFactory.java
diff --git a/servers/src/main/java/tachyon/worker/block/allocator/AllocatorType.java b/servers/src/main/java/tachyon/worker/block/allocator/AllocatorType.java
diff --git a/servers/src/test/java/tachyon/worker/block/allocator/AllocatorContractTest.java b/servers/src/test/java/tachyon/worker/block/allocator/AllocatorContractTest.java
@@ -15,21 +15,58 @@
 
 package tachyon.worker.block.allocator;
 
-import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
+import org.junit.Assert;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
+import com.google.common.reflect.ClassPath;
+import com.google.common.reflect.Reflection;
+
+import tachyon.Constants;
+import tachyon.conf.TachyonConf;
+
 /**
  * This is the class to test the "contact" of different kinds of allocators,
  * i.e., the general properties the allocators need to follow
  */
 public class AllocatorContractTest extends BaseAllocatorTest {
+  protected static List<String> sStrategies = new ArrayList<String>();
+
+  @BeforeClass
+  /**
+   *  Try to find all implementation classes of {@link Allocator} in the same package
+   */
+  public static void beforeClass() {
+    try {
+      String packageName = Reflection.getPackageName(Allocator.class);
+      ClassPath path = ClassPath.from(Thread.currentThread().getContextClassLoader());
+      List<ClassPath.ClassInfo> clazzInPackage =
+          new ArrayList<ClassPath.ClassInfo>(path.getTopLevelClassesRecursive(packageName));
+      for (ClassPath.ClassInfo clazz : clazzInPackage) {
+        Set<Class<?>> interfaces =
+            new HashSet<Class<?>>(Arrays.asList(clazz.load().getInterfaces()));
+        if (interfaces.size() > 0 && interfaces.contains(Allocator.class)) {
+          sStrategies.add(clazz.getName());
+        }
+      }
+    } catch (Exception e) {
+      Assert.fail("Failed to find implementation of allocate strategy");
+    }
+  }
 
   @Test
   public void shouldNotAllocateTest() throws Exception {
-    for (AllocatorType type : AllocatorType.values()) {
+    TachyonConf conf = createTestTachyonConf();
+    for (String strategyName : sStrategies) {
+      conf.set(Constants.WORKER_ALLOCATE_STRATEGY, strategyName);
       resetManagerView();
-      Allocator allocator = AllocatorFactory.create(type, mManagerView);
+      Allocator allocator = Allocator.Factory.createAllocator(conf, mManagerView);
       assertTempBlockMeta(allocator, mAnyDirInTierLoc1, DEFAULT_RAM_SIZE + 1, false);
       assertTempBlockMeta(allocator, mAnyDirInTierLoc2, DEFAULT_SSD_SIZE + 1, false);
       assertTempBlockMeta(allocator, mAnyDirInTierLoc3, DEFAULT_HDD_SIZE + 1, false);
@@ -40,9 +77,11 @@ public void shouldNotAllocateTest() throws Exception {
 
   @Test
   public void shouldAllocateTest() throws Exception {
-    for (AllocatorType type : AllocatorType.values()) {
+    TachyonConf conf = createTestTachyonConf();
+    for (String strategyName : sStrategies) {
+      conf.set(Constants.WORKER_ALLOCATE_STRATEGY, strategyName);
       resetManagerView();
-      Allocator tierAllocator = AllocatorFactory.create(type, mManagerView);
+      Allocator tierAllocator = Allocator.Factory.createAllocator(conf, mManagerView);
       for (int i = 0; i < DEFAULT_RAM_NUM; i ++) {
         assertTempBlockMeta(tierAllocator, mAnyDirInTierLoc1, DEFAULT_RAM_SIZE - 1, true);
       }
@@ -54,7 +93,7 @@ public void shouldAllocateTest() throws Exception {
       }
 
       resetManagerView();
-      Allocator anyAllocator = AllocatorFactory.create(type, mManagerView);
+      Allocator anyAllocator = Allocator.Factory.createAllocator(conf, mManagerView);
       for (int i = 0; i < DEFAULT_RAM_NUM; i ++) {
         assertTempBlockMeta(anyAllocator, mAnyTierLoc, DEFAULT_RAM_SIZE - 1, true);
       }

diff --git a/servers/src/test/java/tachyon/worker/block/allocator/AllocatorFactoryTest.java b/servers/src/test/java/tachyon/worker/block/allocator/AllocatorFactoryTest.java
@@ -16,7 +16,6 @@
 package tachyon.worker.block.allocator;
 
 import java.io.File;
-import java.io.IOException;
 import java.util.Collections;
 
 import org.junit.Assert;
@@ -25,15 +24,18 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import tachyon.Constants;
+import tachyon.conf.TachyonConf;
 import tachyon.worker.block.BlockMetadataManager;
 import tachyon.worker.block.BlockMetadataManagerView;
 import tachyon.worker.block.evictor.EvictorTestUtils;
 
-/*
- * Test AllocatorFactory by passing different AllocatorType
- * and test if it generate the correct Allocator instance.
+/**
+ * Test {@link Allocator.Factory} by passing different allocate strategy class
+ * names with tachyon conf and test if it generates the correct Allocator instance.
  */
 public class AllocatorFactoryTest {
+  private TachyonConf mTachyonConf;
   private BlockMetadataManager mMetaManager;
   private BlockMetadataManagerView mManagerView;
 
@@ -43,26 +45,41 @@ public class AllocatorFactoryTest {
   @Before
   public void before() throws Exception {
     File tempFolder = mTestFolder.newFolder();
+    mTachyonConf = new TachyonConf();
     mMetaManager = EvictorTestUtils.defaultMetadataManager(tempFolder.getAbsolutePath());
     mManagerView = new BlockMetadataManagerView(mMetaManager, Collections.<Integer>emptySet(),
         Collections.<Long>emptySet());
   }
 
   @Test
   public void createGreedyAllocatorTest() {
-    Allocator allocator = AllocatorFactory.create(AllocatorType.GREEDY, mManagerView);
+    mTachyonConf.set(Constants.WORKER_ALLOCATE_STRATEGY, GreedyAllocator.class.getName());
+    Allocator allocator = Allocator.Factory.createAllocator(mTachyonConf, mManagerView);
     Assert.assertTrue(allocator instanceof GreedyAllocator);
   }
 
   @Test
   public void createMaxFreeAllocatorTest() {
-    Allocator allocator = AllocatorFactory.create(AllocatorType.MAX_FREE, mManagerView);
+    mTachyonConf.set(Constants.WORKER_ALLOCATE_STRATEGY, MaxFreeAllocator.class.getName());
+    Allocator allocator = Allocator.Factory.createAllocator(mTachyonConf, mManagerView);
     Assert.assertTrue(allocator instanceof MaxFreeAllocator);
   }
 
   @Test
-  public void createDefaultAllocatorTypeTest() {
-    Allocator allocator = AllocatorFactory.create(AllocatorType.DEFAULT, mManagerView);
+  public void createRoundRobinAllocatorTest() {
+    mTachyonConf.set(Constants.WORKER_ALLOCATE_STRATEGY, RoundRobinAllocator.class.getName());
+    Allocator allocator = Allocator.Factory.createAllocator(mTachyonConf, mManagerView);
+    Assert.assertTrue(allocator instanceof RoundRobinAllocator);
+  }
+
+  @Test
+  public void createDefaultAllocatorTest() {
+    /*
+     * create a new instance of TachyonConf with original
+     * properties to test the default behavior of createAllocator
+     */
+    TachyonConf conf = new TachyonConf();
+    Allocator allocator = Allocator.Factory.createAllocator(conf, mManagerView);
     Assert.assertTrue(allocator instanceof MaxFreeAllocator);
   }
 }
diff --git a/servers/src/test/java/tachyon/worker/block/allocator/GreedyAllocatorTest.java b/servers/src/test/java/tachyon/worker/block/allocator/GreedyAllocatorTest.java
@@ -15,17 +15,17 @@
 
 package tachyon.worker.block.allocator;
 
-import java.io.IOException;
-
 import org.junit.Test;
 
-import tachyon.worker.block.allocator.AllocatorFactory;
-import tachyon.worker.block.allocator.AllocatorType;
+import tachyon.Constants;
+import tachyon.conf.TachyonConf;
 
 public class GreedyAllocatorTest extends BaseAllocatorTest {
   @Test
   public void allocateBlockTest() throws Exception {
-    mAllocator = AllocatorFactory.create(AllocatorType.GREEDY, mManagerView);
+    TachyonConf conf = new TachyonConf();
+    conf.set(Constants.WORKER_ALLOCATE_STRATEGY, GreedyAllocator.class.getName());
+    mAllocator = Allocator.Factory.createAllocator(conf, mManagerView);
     //
     // idx | tier1 | tier2 | tier3
     //  0    1000

diff --git a/servers/src/test/java/tachyon/worker/block/allocator/MaxFreeAllocatorTest.java b/servers/src/test/java/tachyon/worker/block/allocator/MaxFreeAllocatorTest.java
@@ -15,17 +15,17 @@
 
 package tachyon.worker.block.allocator;
 
-import java.io.IOException;
-
 import org.junit.Test;
 
-import tachyon.worker.block.allocator.AllocatorFactory;
-import tachyon.worker.block.allocator.AllocatorType;
+import tachyon.Constants;
+import tachyon.conf.TachyonConf;
 
 public class MaxFreeAllocatorTest extends BaseAllocatorTest {
   @Test
   public void allocateBlockTest() throws Exception {
-    mAllocator = AllocatorFactory.create(AllocatorType.MAX_FREE, mManagerView);
+    TachyonConf conf = new TachyonConf();
+    conf.set(Constants.WORKER_ALLOCATE_STRATEGY, MaxFreeAllocator.class.getName());
+    mAllocator = Allocator.Factory.createAllocator(conf, mManagerView);
     //
     // idx | tier1 | tier2 | tier3
     //  0    1000