Removed useless results.put for fail fast

sarutak · sarutak · commit 85c89380b499 · 2014-08-01T00:20:53.000+09:00
diff --git a/core/src/main/scala/org/apache/spark/storage/BlockFetcherIterator.scala b/core/src/main/scala/org/apache/spark/storage/BlockFetcherIterator.scala
@@ -199,22 +199,18 @@ object BlockFetcherIterator {
       // Get the local blocks while remote blocks are being fetched. Note that it's okay to do
       // these all at once because they will just memory-map some files, so they won't consume
       // any memory that might exceed our maxBytesInFlight
-      var fetchIndex = 0
-      try {
-        for (id <- localBlocksToFetch) {
-
+      for (id <- localBlocksToFetch) {
+        try {
           // getLocalFromDisk never return None but throws BlockException
           val iter = getLocalFromDisk(id, serializer).get
           // Pass 0 as size since it's not in flight
           results.put(new FetchResult(id, 0, () => iter))
-          fetchIndex += 1
           logDebug("Got local block " + id)
-        }
-      } catch {
-        case e: Exception => {
-          logError(s"Error occurred while fetching local blocks", e)
-          for (id <- localBlocksToFetch.drop(fetchIndex)) {
+        } catch {
+          case e: Exception => {
+            logError(s"Error occurred while fetching local blocks", e)
             results.put(new FetchResult(id, -1, null))
+            return
           }
         }
       }
diff --git a/core/src/test/scala/org/apache/spark/storage/BlockFetcherIteratorSuite.scala b/core/src/test/scala/org/apache/spark/storage/BlockFetcherIteratorSuite.scala
@@ -81,14 +81,10 @@ class BlockFetcherIteratorSuite extends FunSuite with Matchers {
     assert(iterator.next._2.isDefined, "2nd element should be defined but is not actually defined") 
     assert(iterator.hasNext, "iterator should have 5 elements but actually has 2 elements")
     // 3rd fetch should be failed
-    assert(!iterator.next._2.isDefined, "3nd element should not be defined but is actually defined") 
+    assert(!iterator.next._2.isDefined, "3rd element should not be defined but is actually defined") 
     assert(iterator.hasNext, "iterator should have 5 elements but actually has 3 elements")
-    // And then, all of local fetches should be failed
-    assert(!iterator.next._2.isDefined, "Rest of elements after non-defined element should not be defined " +
-      "but 4th element is actually defined") 
-    assert(iterator.hasNext, "iterator should have 5 elements but actually has 4 elements")
-    assert(!iterator.next._2.isDefined, "Rest of elements after non-defined element should not be defined " +
-      "but 5th element is actually defined") 
+    // Don't call next() after fetching non-defined element even if thare are rest of elements in the iterator.
+    // Otherwise, BasicBlockFetcherIterator hangs up.
   }
 
 
