Cleanup

Author: aarondav

core/src/test/scala/org/apache/spark/ExternalShuffleServiceSuite.scala

@@ -65,7 +65,7 @@ class ExternalShuffleServiceSuite extends ShuffleSuite with BeforeAndAfterAll {
 
     // Invalidate the registered executors, disallowing access to their shuffle blocks (without
     // deleting the actual shuffle files, so we could access them without the shuffle service).
-    rpcHandler.removeApplication(sc.conf.getAppId, false /* cleanupLocalDirs */)
+    rpcHandler.applicationRemoved(sc.conf.getAppId, false /* cleanupLocalDirs */)
 
     // Now Spark will receive FetchFailed, and not retry the stage due to "spark.test.noStageRetry"
     // being set.

network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockHandler.java

@@ -98,7 +98,7 @@ public StreamManager getStreamManager() {
    * Removes an application (once it has been terminated), and optionally will clean up any
    * local directories associated with the executors of that application in a separate thread.
    */
-  public void removeApplication(String appId, boolean cleanupLocalDirs) {
-    blockManager.removeApplication(appId, cleanupLocalDirs);
+  public void applicationRemoved(String appId, boolean cleanupLocalDirs) {
+    blockManager.applicationRemoved(appId, cleanupLocalDirs);
   }
 }

network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockManager.java

@@ -118,7 +118,7 @@ public ManagedBuffer getBlockData(String appId, String execId, String blockId) {
    * It is not valid to call registerExecutor() for an executor with this appId after invoking
    * this method.
    */
-  public void removeApplication(String appId, boolean cleanupLocalDirs) {
+  public void applicationRemoved(String appId, boolean cleanupLocalDirs) {
     logger.info("Application {} removed, cleanupLocalDirs = {}", appId, cleanupLocalDirs);
     Iterator<Map.Entry<AppExecId, ExecutorShuffleInfo>> it = executors.entrySet().iterator();
     while (it.hasNext()) {
@@ -131,12 +131,11 @@ public void removeApplication(String appId, boolean cleanupLocalDirs) {
         it.remove();
 
         if (cleanupLocalDirs) {
-          logger.debug("Cleaning up executor {}'s {} local dirs", fullId, executor.localDirs.length);
+          logger.info("Cleaning up executor {}'s {} local dirs", fullId, executor.localDirs.length);
 
           // Execute the actual deletion in a different thread, as it may take some time.
           directoryCleanupExecutor.execute(new Runnable() {
-            @Override
-            public void run() {
+            @Override public void run() {
               deleteExecutorDirs(executor.localDirs);
             }
           });
@@ -153,7 +152,7 @@ private void deleteExecutorDirs(String[] dirs) {
     for (String localDir : dirs) {
       try {
         JavaUtils.deleteRecursively(new File(localDir));
-        logger.info("Successfully cleaned up directory: " + localDir);
+        logger.debug("Successfully cleaned up directory: " + localDir);
       } catch (Exception e) {
         logger.error("Failed to delete directory: " + localDir, e);
       }

network/shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleCleanupSuite.java

@@ -30,21 +30,21 @@
 
 public class ExternalShuffleCleanupSuite {
 
-  // Executor used to ensure cleanup happens synchronously in this thread.
+  // Same-thread Executor used to ensure cleanup happens synchronously in test thread.
   Executor sameThreadExecutor = MoreExecutors.sameThreadExecutor();
 
   @Test
-  public void cleanupDirs() throws IOException {
+  public void noCleanupAndCleanup() throws IOException {
     TestShuffleDataContext dataContext = createSomeData();
 
     ExternalShuffleBlockManager manager = new ExternalShuffleBlockManager(sameThreadExecutor);
     manager.registerExecutor("app", "exec0", dataContext.createExecutorInfo("shuffleMgr"));
-    manager.removeApplication("app", false /* cleanup */);
+    manager.applicationRemoved("app", false /* cleanup */);
 
     assertStillThere(dataContext);
 
     manager.registerExecutor("app", "exec1", dataContext.createExecutorInfo("shuffleMgr"));
-    manager.removeApplication("app", true /* cleanup */);
+    manager.applicationRemoved("app", true /* cleanup */);
 
     assertCleanedUp(dataContext);
   }
@@ -61,7 +61,7 @@ public void cleanupUsesExecutor() throws IOException {
     ExternalShuffleBlockManager manager = new ExternalShuffleBlockManager(noThreadExecutor);
 
     manager.registerExecutor("app", "exec0", dataContext.createExecutorInfo("shuffleMgr"));
-    manager.removeApplication("app", true /* cleanup */);
+    manager.applicationRemoved("app", true);
 
     assertStillThere(dataContext);
 
@@ -70,15 +70,15 @@ public void cleanupUsesExecutor() throws IOException {
   }
 
   @Test
-  public void cleanupMultipleExecutrs() throws IOException {
+  public void cleanupMultipleExecutors() throws IOException {
     TestShuffleDataContext dataContext0 = createSomeData();
     TestShuffleDataContext dataContext1 = createSomeData();
 
     ExternalShuffleBlockManager manager = new ExternalShuffleBlockManager(sameThreadExecutor);
 
     manager.registerExecutor("app", "exec0", dataContext0.createExecutorInfo("shuffleMgr"));
     manager.registerExecutor("app", "exec1", dataContext1.createExecutorInfo("shuffleMgr"));
-    manager.removeApplication("app", true /* cleanup */);
+    manager.applicationRemoved("app", true);
 
     assertCleanedUp(dataContext0);
     assertCleanedUp(dataContext1);
@@ -94,20 +94,20 @@ public void cleanupOnlyRemovedApp() throws IOException {
     manager.registerExecutor("app-0", "exec0", dataContext0.createExecutorInfo("shuffleMgr"));
     manager.registerExecutor("app-1", "exec0", dataContext1.createExecutorInfo("shuffleMgr"));
 
-    manager.removeApplication("app-nonexistent", true /* cleanup */);
+    manager.applicationRemoved("app-nonexistent", true);
     assertStillThere(dataContext0);
     assertStillThere(dataContext1);
 
-    manager.removeApplication("app-0", true /* cleanup */);
+    manager.applicationRemoved("app-0", true);
     assertCleanedUp(dataContext0);
     assertStillThere(dataContext1);
 
-    manager.removeApplication("app-1", true /* cleanup */);
+    manager.applicationRemoved("app-1", true);
     assertCleanedUp(dataContext0);
     assertCleanedUp(dataContext1);
 
     // Make sure it's not an error to cleanup multiple times
-    manager.removeApplication("app-1", true /* cleanup */);
+    manager.applicationRemoved("app-1", true);
     assertCleanedUp(dataContext0);
     assertCleanedUp(dataContext1);
   }

network/shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java

@@ -105,7 +105,7 @@ public static void afterAll() {
 
   @After
   public void afterEach() {
-    handler.removeApplication(APP_ID, false /* cleanupLocalDirs */);
+    handler.applicationRemoved(APP_ID, false /* cleanupLocalDirs */);
   }
 
   class FetchResult {