Simplify imports and exports and provide driver isolation for remote functions. (#288)

* Remove import counter and export counter.

* Provide isolation between drivers for remote functions.

* Add test for driver function isolation.

* Hash source code into function ID to reduce likelihood of collisions.

* Fix failure test example.

* Replace assertTrue with assertIn to improve failure messages in tests.

* Fix failure test.

test/failure_test.py

@@ -2,10 +2,12 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 
-import unittest
+import os
 import ray
 import sys
+import tempfile
 import time
+import unittest
 
 if sys.version_info >= (3, 0):
   from importlib import reload
@@ -67,13 +69,13 @@ class TaskStatusTest(unittest.TestCase):
     result = ray.error_info()
     self.assertEqual(len(relevant_errors(b"task")), 2)
     for task in relevant_errors(b"task"):
-      self.assertTrue(b"Test function 1 intentionally failed." in task.get(b"message"))
+      self.assertIn(b"Test function 1 intentionally failed.", task.get(b"message"))
 
     x = test_functions.throw_exception_fct2.remote()
     try:
       ray.get(x)
     except Exception as e:
-      self.assertTrue("Test function 2 intentionally failed." in str(e))
+      self.assertIn("Test function 2 intentionally failed.", str(e))
     else:
       self.assertTrue(False) # ray.get should throw an exception
 
@@ -82,7 +84,7 @@ class TaskStatusTest(unittest.TestCase):
       try:
         ray.get(ref)
       except Exception as e:
-        self.assertTrue("Test function 3 intentionally failed." in str(e))
+        self.assertIn("Test function 3 intentionally failed.", str(e))
       else:
         self.assertTrue(False) # ray.get should throw an exception
 
@@ -91,29 +93,40 @@ class TaskStatusTest(unittest.TestCase):
   def testFailImportingRemoteFunction(self):
     ray.init(num_workers=2, driver_mode=ray.SILENT_MODE)
 
-    # This example is somewhat contrived. It should be successfully pickled, and
-    # then it should throw an exception when it is unpickled. This may depend a
-    # bit on the specifics of our pickler.
-    def reducer(*args):
-      raise Exception("There is a problem here.")
-    class Foo(object):
-      def __init__(self):
-        self.__name__ = "Foo_object"
-        self.func_doc = ""
-        self.__globals__ = {}
-      def __reduce__(self):
-        return reducer, ()
-      def __call__(self):
-        return
-    f = ray.remote(Foo())
+    # Create the contents of a temporary Python file.
+    temporary_python_file = """
+def temporary_helper_function():
+  return 1
+"""
+
+    f = tempfile.NamedTemporaryFile(suffix=".py")
+    f.write(temporary_python_file.encode("ascii"))
+    f.flush()
+    directory = os.path.dirname(f.name)
+    # Get the module name and strip ".py" from the end.
+    module_name = os.path.basename(f.name)[:-3]
+    sys.path.append(directory)
+    module = __import__(module_name)
+
+    # Define a function that closes over this temporary module. This should fail
+    # when it is unpickled.
+    @ray.remote
+    def g():
+      return module.temporary_python_file()
+
     wait_for_errors(b"register_remote_function", 2)
-    self.assertTrue(b"There is a problem here." in ray.error_info()[0][b"message"])
+    self.assertIn(b"No module named", ray.error_info()[0][b"message"])
+    self.assertIn(b"No module named", ray.error_info()[1][b"message"])
 
     # Check that if we try to call the function it throws an exception and does
     # not hang.
     for _ in range(10):
-      self.assertRaises(Exception, lambda : ray.get(f.remote()))
+      self.assertRaises(Exception, lambda : ray.get(g.remote()))
 
+    f.close()
+
+    # Clean up the junk we added to sys.path.
+    sys.path.pop(-1)
     ray.worker.cleanup()
 
   def testFailImportingEnvironmentVariable(self):
@@ -128,7 +141,7 @@ class TaskStatusTest(unittest.TestCase):
     ray.env.foo = ray.EnvironmentVariable(initializer)
     wait_for_errors(b"register_environment_variable", 2)
     # Check that the error message is in the task info.
-    self.assertTrue(b"The initializer failed." in ray.error_info()[0][b"message"])
+    self.assertIn(b"The initializer failed.", ray.error_info()[0][b"message"])
 
     ray.worker.cleanup()
 
@@ -146,7 +159,7 @@ class TaskStatusTest(unittest.TestCase):
     use_foo.remote()
     wait_for_errors(b"reinitialize_environment_variable", 1)
     # Check that the error message is in the task info.
-    self.assertTrue(b"The reinitializer failed." in ray.error_info()[0][b"message"])
+    self.assertIn(b"The reinitializer failed.", ray.error_info()[0][b"message"])
 
     ray.worker.cleanup()
 
@@ -160,8 +173,8 @@ class TaskStatusTest(unittest.TestCase):
     wait_for_errors(b"function_to_run", 2)
     # Check that the error message is in the task info.
     self.assertEqual(len(ray.error_info()), 2)
-    self.assertTrue(b"Function to run failed." in ray.error_info()[0][b"message"])
-    self.assertTrue(b"Function to run failed." in ray.error_info()[1][b"message"])
+    self.assertIn(b"Function to run failed.", ray.error_info()[0][b"message"])
+    self.assertIn(b"Function to run failed.", ray.error_info()[1][b"message"])
 
     ray.worker.cleanup()
 

test/multi_node_test.py

@@ -16,16 +16,22 @@ stop_ray_script = os.path.join(os.path.dirname(os.path.abspath(__file__)), "../s
 
 class MultiNodeTest(unittest.TestCase):
 
-  def testErrorIsolation(self):
+  def setUp(self):
     # Start the Ray processes on this machine.
     out = subprocess.check_output([start_ray_script, "--head"]).decode("ascii")
     # Get the redis address from the output.
     redis_substring_prefix = "redis_address=\""
     redis_address_location = out.find(redis_substring_prefix) + len(redis_substring_prefix)
     redis_address = out[redis_address_location:]
-    redis_address = redis_address.split("\"")[0]
+    self.redis_address = redis_address.split("\"")[0]
+
+  def tearDown(self):
+    # Kill the Ray cluster.
+    subprocess.Popen([stop_ray_script]).wait()
+
+  def testErrorIsolation(self):
     # Connect a driver to the Ray cluster.
-    ray.init(redis_address=redis_address, driver_mode=ray.SILENT_MODE)
+    ray.init(redis_address=self.redis_address, driver_mode=ray.SILENT_MODE)
 
     # There shouldn't be any errors yet.
     self.assertEqual(len(ray.error_info()), 0)
@@ -79,7 +85,7 @@ assert len(ray.error_info()) == 1
 assert "{}" in ray.error_info()[0][b"message"].decode("ascii")
 
 print("success")
-""".format(redis_address, error_string2, error_string2)
+""".format(self.redis_address, error_string2, error_string2)
 
     # Save the driver script as a file so we can call it using subprocess.
     with tempfile.NamedTemporaryFile() as f:
@@ -95,7 +101,53 @@ print("success")
     self.assertIn(error_string1, ray.error_info()[0][b"message"].decode("ascii"))
 
     ray.worker.cleanup()
-    subprocess.Popen([stop_ray_script]).wait()
+
+  def testRemoteFunctionIsolation(self):
+    # This test will run multiple remote functions with the same names in two
+    # different drivers.
+    # Connect a driver to the Ray cluster.
+    ray.init(redis_address=self.redis_address, driver_mode=ray.SILENT_MODE)
+
+    # Start another driver and make sure that it can define and call its own
+    # commands with the same names.
+    driver_script = """
+import ray
+import time
+ray.init(redis_address="{}")
+@ray.remote
+def f():
+  return 3
+@ray.remote
+def g(x, y):
+  return 4
+for _ in range(10000):
+  result = ray.get([f.remote(), g.remote(0, 0)])
+  assert result == [3, 4]
+print("success")
+""".format(self.redis_address)
+
+    # Save the driver script as a file so we can call it using subprocess.
+    with tempfile.NamedTemporaryFile() as f:
+      f.write(driver_script.encode("ascii"))
+      f.flush()
+      out = subprocess.check_output(["python", f.name]).decode("ascii")
+
+    @ray.remote
+    def f():
+      return 1
+
+    @ray.remote
+    def g(x):
+      return 2
+
+    for _ in range(10000):
+      result = ray.get([f.remote(), g.remote(0)])
+      self.assertEqual(result, [1, 2])
+
+    # Make sure the other driver succeeded.
+    self.assertIn("success", out)
+
+    ray.worker.cleanup()
 
 class StartRayScriptTest(unittest.TestCase):
 

test/runtest.py

@@ -599,6 +599,62 @@ class APITest(unittest.TestCase):
 
     ray.worker.cleanup()
 
+  def testIdenticalFunctionNames(self):
+    # Define a bunch of remote functions and make sure that we don't
+    # accidentally call an older version.
+    ray.init(num_workers=2)
+
+    num_remote_functions = 100
+    num_calls = 200
+
+    @ray.remote
+    def f():
+      return 1
+    results1 = [f.remote() for _ in range(num_calls)]
+    @ray.remote
+    def f():
+      return 2
+    results2 = [f.remote() for _ in range(num_calls)]
+    @ray.remote
+    def f():
+      return 3
+    results3 = [f.remote() for _ in range(num_calls)]
+    @ray.remote
+    def f():
+      return 4
+    results4 = [f.remote() for _ in range(num_calls)]
+    @ray.remote
+    def f():
+      return 5
+    results5 = [f.remote() for _ in range(num_calls)]
+
+    self.assertEqual(ray.get(results1), num_calls * [1])
+    self.assertEqual(ray.get(results2), num_calls * [2])
+    self.assertEqual(ray.get(results3), num_calls * [3])
+    self.assertEqual(ray.get(results4), num_calls * [4])
+    self.assertEqual(ray.get(results5), num_calls * [5])
+
+    @ray.remote
+    def g():
+      return 1
+    @ray.remote
+    def g():
+      return 2
+    @ray.remote
+    def g():
+      return 3
+    @ray.remote
+    def g():
+      return 4
+    @ray.remote
+    def g():
+      return 5
+
+    result_values = ray.get([g.remote() for _ in range(num_calls)])
+    self.assertEqual(result_values, num_calls * [5])
+
+    ray.worker.cleanup()
+
 class PythonModeTest(unittest.TestCase):
 
   def testPythonMode(self):