Merge pull request Alluxio#24 from TachyonNexus/reset-contexts

[SMALLFIX] Reset contexts when HDFS is initialized
apc999 · Feb 8, 2016 · 3037a1e · 3037a1e
2 parents abfaf47 + 9665190
commit 3037a1e
Show file tree

Hide file tree

Showing 9 changed files with 173 additions and 42 deletions.
diff --git a/core/client-internal/pom.xml b/core/client-internal/pom.xml
@@ -48,6 +48,15 @@
       <groupId>com.google.code.findbugs</groupId>
       <artifactId>annotations</artifactId>
     </dependency>
+
+    <!-- Other projects' test-jars -->
+    <dependency>
+      <groupId>org.alluxio.alluxio</groupId>
+      <artifactId>alluxio-core-common</artifactId>
+      <version>${project.version}</version>
+      <type>test-jar</type>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/core/client-internal/src/main/java/alluxio/client/ClientContext.java b/core/client-internal/src/main/java/alluxio/client/ClientContext.java
@@ -23,8 +23,8 @@
 
 import com.google.common.base.Preconditions;
 
-import alluxio.Constants;
 import alluxio.Configuration;
+import alluxio.Constants;
 import alluxio.util.ThreadFactoryUtils;
 import alluxio.worker.ClientMetrics;
 
@@ -61,7 +61,7 @@ private static void reset() {
    * {@link Configuration} when {@link Configuration} is changed, e.g. the master hostname or port.
    * This method requires that configuration has been initialized.
    */
-  private static void init() {
+  public static void init() {
     String masterHostname = Preconditions.checkNotNull(sConf.get(Constants.MASTER_HOSTNAME));
     int masterPort = sConf.getInt(Constants.MASTER_RPC_PORT);
     sMasterAddress = new InetSocketAddress(masterHostname, masterPort);

diff --git a/core/client-internal/src/main/java/alluxio/hadoop/AbstractFileSystem.java b/core/client-internal/src/main/java/alluxio/hadoop/AbstractFileSystem.java
@@ -36,9 +36,11 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 
-import alluxio.Constants;
 import alluxio.AlluxioURI;
+import alluxio.Configuration;
+import alluxio.Constants;
 import alluxio.client.ClientContext;
+import alluxio.client.block.BlockStoreContext;
 import alluxio.client.file.FileOutStream;
 import alluxio.client.file.FileSystem;
 import alluxio.client.file.FileSystemContext;
@@ -48,13 +50,13 @@
 import alluxio.client.file.options.CreateFileOptions;
 import alluxio.client.file.options.DeleteOptions;
 import alluxio.client.file.options.SetAttributeOptions;
-import alluxio.Configuration;
+import alluxio.client.lineage.LineageContext;
+import alluxio.exception.AlluxioException;
 import alluxio.exception.ConnectionFailedException;
 import alluxio.exception.ExceptionMessage;
 import alluxio.exception.FileDoesNotExistException;
 import alluxio.exception.InvalidPathException;
 import alluxio.exception.PreconditionMessage;
-import alluxio.exception.AlluxioException;
 import alluxio.util.CommonUtils;
 import alluxio.wire.FileBlockInfo;
 import alluxio.wire.WorkerNetAddress;
@@ -431,6 +433,13 @@ public void initialize(URI uri, org.apache.hadoop.conf.Configuration conf) throw
     ClientContext.getConf().set(Constants.MASTER_RPC_PORT, Integer.toString(uri.getPort()));
     ClientContext.getConf().set(Constants.ZOOKEEPER_ENABLED, Boolean.toString(isZookeeperMode()));
 
+    // These must be reset to pick up the change to the master address.
+    // TODO(andrew): We should reset key value system in this situation - see TACHYON-1706.
+    ClientContext.init();
+    FileSystemContext.INSTANCE.reset();
+    BlockStoreContext.INSTANCE.reset();
+    LineageContext.INSTANCE.reset();
+
     mFileSystem = FileSystem.Factory.get();
     mUri = URI.create(mAlluxioHeader);
     mUnderFSAddress = getUfsAddress();

diff --git a/...src/test/java/alluxio/hadoop/TFSTest.java → ...lluxio/hadoop/AbstractFileSystemTest.java b/...src/test/java/alluxio/hadoop/TFSTest.java → ...lluxio/hadoop/AbstractFileSystemTest.java
@@ -17,6 +17,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.net.InetSocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -37,10 +38,13 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import alluxio.CommonTestUtils;
 import alluxio.Constants;
 import alluxio.client.ClientContext;
+import alluxio.client.block.BlockStoreContext;
 import alluxio.client.file.FileSystemContext;
 import alluxio.client.file.FileSystemMasterClient;
+import alluxio.client.lineage.LineageContext;
 import alluxio.client.util.ClientTestUtils;
 
 /**
@@ -54,8 +58,31 @@
  * See https://code.google.com/p/powermock/wiki/FAQ.
  */
 @PowerMockIgnore("javax.security.*")
-public class TFSTest {
-  private static final Logger LOG = LoggerFactory.getLogger(TFSTest.class.getName());
+/**
+ * Tests for {@link AbstractFileSystem}.
+ */
+public class AbstractFileSystemTest {
+  private static final Logger LOG = LoggerFactory.getLogger(Constants.LOGGER_TYPE);
+  private FileSystemContext mMockFileSystemContext;
+
+  /**
+   * Sets up the configuration before a test runs.
+   *
+   * @throws Exception when creating the mock fails
+   */
+  @Before
+  public void setup() throws Exception {
+    mockUserGroupInformation();
+    mockMasterClient();
+
+    if (isHadoop1x()) {
+      LOG.debug("Running TFS tests against hadoop 1x");
+    } else if (isHadoop2x()) {
+      LOG.debug("Running TFS tests against hadoop 2x");
+    } else {
+      LOG.warn("Running TFS tests against untargeted Hadoop version: " + getHadoopVersion());
+    }
+  }
 
   private ClassLoader getClassLoader(Class<?> clazz) {
     // Power Mock makes this hard, so try to hack it
@@ -88,12 +115,12 @@ private URL getSourcePath(Class<?> clazz) {
   }
 
   /**
-   * Ensures that Hadoop loads TFSFT when configured.
+   * Ensures that Hadoop loads {@link FaultTolerantFileSystem} when configured.
    *
    * @throws IOException when the file system cannot be retrieved
    */
   @Test
-  public void hadoopShouldLoadTfsFtWhenConfigured() throws IOException {
+  public void hadoopShouldLoadFaultTolerantFileSystemWhenConfiguredTest() throws IOException {
     final Configuration conf = new Configuration();
     if (isHadoop1x()) {
       conf.set("fs." + Constants.SCHEME_FT + ".impl", FaultTolerantFileSystem.class.getName());
@@ -105,7 +132,6 @@ public void hadoopShouldLoadTfsFtWhenConfigured() throws IOException {
     ClientContext.getConf().set(Constants.MASTER_HOSTNAME, uri.getHost());
     ClientContext.getConf().set(Constants.MASTER_RPC_PORT, Integer.toString(uri.getPort()));
     ClientContext.getConf().set(Constants.ZOOKEEPER_ENABLED, "true");
-    mockMasterClient();
 
     final org.apache.hadoop.fs.FileSystem fs = org.apache.hadoop.fs.FileSystem.get(uri, conf);
 
@@ -122,7 +148,7 @@ public void hadoopShouldLoadTfsFtWhenConfigured() throws IOException {
    * @throws IOException when the file system cannot be retrieved
    */
   @Test
-  public void hadoopShouldLoadTfsWhenConfigured() throws IOException {
+  public void hadoopShouldLoadFileSystemWhenConfiguredTest() throws IOException {
     final Configuration conf = new Configuration();
     if (isHadoop1x()) {
       conf.set("fs." + Constants.SCHEME + ".impl", FileSystem.class.getName());
@@ -134,7 +160,6 @@ public void hadoopShouldLoadTfsWhenConfigured() throws IOException {
     ClientContext.getConf().set(Constants.MASTER_HOSTNAME, uri.getHost());
     ClientContext.getConf().set(Constants.MASTER_RPC_PORT, Integer.toString(uri.getPort()));
     ClientContext.getConf().set(Constants.ZOOKEEPER_ENABLED, "false");
-    mockMasterClient();
 
     final org.apache.hadoop.fs.FileSystem fs = org.apache.hadoop.fs.FileSystem.get(uri, conf);
 
@@ -145,6 +170,32 @@ public void hadoopShouldLoadTfsWhenConfigured() throws IOException {
     ClientTestUtils.resetClientContext();
   }
 
+  /**
+   * Tests that initializing the {@link AbstractFileSystem} will reinitialize contexts to pick up
+   * changes to the master address.
+   */
+  @Test
+  public void resetContextTest() throws Exception {
+    // Create system with master at localhost:19998
+    URI uri = URI.create(Constants.HEADER + "localhost:19998/");
+    Configuration conf = new Configuration();
+    org.apache.hadoop.fs.FileSystem fs = org.apache.hadoop.fs.FileSystem.get(uri, conf);
+
+    // Change to otherhost:410
+    URI newUri = URI.create(Constants.HEADER + "otherhost:410/");
+    fs.initialize(newUri, conf);
+
+    // Make sure all contexts are using the new address
+    InetSocketAddress newAddress = new InetSocketAddress("otherhost", 410);
+    Assert.assertEquals(newAddress, ClientContext.getMasterAddress());
+    Assert.assertEquals(newAddress, CommonTestUtils.getInternalState(BlockStoreContext.INSTANCE,
+        "mBlockMasterClientPool", "mMasterAddress"));
+    // Once from calling FileSystem.get, once from calling initialize.
+    Mockito.verify(mMockFileSystemContext, Mockito.times(2)).reset();
+    Assert.assertEquals(newAddress, CommonTestUtils.getInternalState(LineageContext.INSTANCE,
+        "mLineageMasterClientPool", "mMasterAddress"));
+  }
+
   private boolean isHadoop1x() {
     return getHadoopVersion().startsWith("1");
   }
@@ -155,11 +206,11 @@ private boolean isHadoop2x() {
 
   private void mockMasterClient() {
     PowerMockito.mockStatic(FileSystemContext.class);
-    FileSystemContext mockContext = PowerMockito.mock(FileSystemContext.class);
+    mMockFileSystemContext = PowerMockito.mock(FileSystemContext.class);
     FileSystemMasterClient mockMaster =
         PowerMockito.mock(FileSystemMasterClient.class);
-    Whitebox.setInternalState(FileSystemContext.class, "INSTANCE", mockContext);
-    Mockito.when(mockContext.acquireMasterClient()).thenReturn(mockMaster);
+    Whitebox.setInternalState(FileSystemContext.class, "INSTANCE", mMockFileSystemContext);
+    Mockito.when(mMockFileSystemContext.acquireMasterClient()).thenReturn(mockMaster);
   }
 
   private void mockUserGroupInformation() throws IOException {
@@ -168,22 +219,4 @@ private void mockUserGroupInformation() throws IOException {
     final UserGroupInformation ugi = Mockito.mock(UserGroupInformation.class);
     Mockito.when(UserGroupInformation.getCurrentUser()).thenReturn(ugi);
   }
-
-  /**
-   * Sets up the configuration before a test runs.
-   *
-   * @throws Exception when creating the mock fails
-   */
-  @Before
-  public void setup() throws Exception {
-    mockUserGroupInformation();
-
-    if (isHadoop1x()) {
-      LOG.debug("Running TFS tests against hadoop 1x");
-    } else if (isHadoop2x()) {
-      LOG.debug("Running TFS tests against hadoop 2x");
-    } else {
-      LOG.warn("Running TFS tests against untargeted Hadoop version: " + getHadoopVersion());
-    }
-  }
 }
diff --git a/core/common/pom.xml b/core/common/pom.xml
@@ -98,5 +98,20 @@
         <filtering>true</filtering>
       </resource>
     </resources>
+    <plugins>
+      <!-- Export test classes in a test-jar so that other projects can use them for testing -->
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-jar-plugin</artifactId>
+        <version>2.4</version>
+        <executions>
+          <execution>
+            <goals>
+              <goal>test-jar</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
   </build>
 </project>
diff --git a/core/common/src/test/java/alluxio/CommonTestUtils.java b/core/common/src/test/java/alluxio/CommonTestUtils.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the University of California, Berkeley 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 alluxio;
+
+import org.powermock.reflect.Whitebox;
+
+/**
+ * Common utilities for testing.
+ */
+public final class CommonTestUtils {
+
+  /**
+   * Traverses a chain of potentially private fields using {@link Whitebox}.
+   *
+   * For example, if you have the classes
+   *
+   *<pre>{@code
+   *public class Foo {
+   *  private Bar myBar = new Bar();
+   *}
+   *
+   *public class Bar {
+   *  private String secret = "puppy";
+   *}
+   *}</pre>
+   *
+   * then you can access {@code "puppy"} with
+   * {@code CommonTestUtils.getInternalState(new Foo(), "myBar", "secret")}.
+   *
+   * @param instance the object to start the traversal from
+   * @param fieldNames the field names to traverse
+   * @return the final value at the end of the traversal
+   */
+  public static <T> T getInternalState(Object instance, String... fieldNames) {
+    Object current = instance;
+    for (String fieldName : fieldNames) {
+      Object next = Whitebox.getInternalState(current, fieldName);
+      if (next == null) {
+        throw new RuntimeException(
+            "Couldn't find field " + fieldName + " in " + current.getClass());
+      }
+      current = next;
+    }
+    @SuppressWarnings("unchecked")
+    T finalObject = (T) current;
+    return finalObject;
+  }
+}
diff --git a/keyvalue/client-internal/pom.xml b/keyvalue/client-internal/pom.xml
@@ -34,8 +34,8 @@
       <groupId>org.alluxio.alluxio</groupId>
       <artifactId>alluxio-core-client-internal</artifactId>
       <version>${project.version}</version>
-      <scope>test</scope>
       <type>test-jar</type>
+      <scope>test</scope>
     </dependency>
   </dependencies>
 </project>
diff --git a/minicluster/pom.xml b/minicluster/pom.xml
@@ -47,12 +47,14 @@
       <artifactId>alluxio-core-client-internal</artifactId>
       <version>${project.version}</version>
       <type>test-jar</type>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.alluxio.alluxio</groupId>
       <artifactId>alluxio-core-server</artifactId>
-      <type>test-jar</type>
       <version>${project.version}</version>
+      <type>test-jar</type>
+      <scope>test</scope>
     </dependency>
   </dependencies>
 </project>
diff --git a/tests/pom.xml b/tests/pom.xml
@@ -53,13 +53,6 @@
       <artifactId>alluxio-core-common</artifactId>
       <version>${project.version}</version>
     </dependency>
-    <dependency>
-      <groupId>org.alluxio.alluxio</groupId>
-      <artifactId>alluxio-core-server</artifactId>
-      <type>test-jar</type>
-      <version>${project.version}</version>
-      <scope>test</scope>
-    </dependency>
     <dependency>
       <groupId>org.alluxio.alluxio</groupId>
       <artifactId>alluxio-keyvalue-client-internal</artifactId>
@@ -90,6 +83,15 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+
+    <!-- Other projects' test-jars -->
+    <dependency>
+      <groupId>org.alluxio.alluxio</groupId>
+      <artifactId>alluxio-core-server</artifactId>
+      <version>${project.version}</version>
+      <type>test-jar</type>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <profiles>