From 750e550074f11de4c9d546957c33704aa5f86283 Mon Sep 17 00:00:00 2001 From: vitrioil Date: Fri, 11 Nov 2022 19:30:07 +0530 Subject: [PATCH 1/5] Make ray.get(timeout=0) to throw timeout error --- python/ray/_private/worker.py | 2 +- python/ray/tests/test_basic_2.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 095dadb4d119..125618a20c79 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -664,7 +664,7 @@ def get_objects(self, object_refs: list, timeout: Optional[float] = None): "which is not an ray.ObjectRef." ) - timeout_ms = int(timeout * 1000) if timeout else -1 + timeout_ms = int(timeout * 1000) if timeout is not None else -1 data_metadata_pairs = self.core_worker.get_objects( object_refs, self.current_task_id, timeout_ms ) diff --git a/python/ray/tests/test_basic_2.py b/python/ray/tests/test_basic_2.py index b83280fcecb6..7c78747148f0 100644 --- a/python/ray/tests/test_basic_2.py +++ b/python/ray/tests/test_basic_2.py @@ -384,6 +384,10 @@ def test_get_with_timeout(ray_start_regular_shared): with pytest.raises(TimeoutError): ray.get(result_id, timeout=0.1) + # timeout of 0 should raise an error + with pytest.raises(GetTimeoutError): + ray.get(result_id, timeout=0) + # Check that a subsequent get() returns early. ray.get(signal.send.remote()) start = time.time() From 7c91c4ccf1a92731b7ec343281c5d69afc675097 Mon Sep 17 00:00:00 2001 From: vitrioil Date: Sun, 13 Nov 2022 19:17:52 +0530 Subject: [PATCH 2/5] Adapt timeout change in tests --- python/ray/tests/test_object_spilling.py | 6 +++--- python/ray/tests/test_object_spilling_2.py | 6 +++--- python/ray/tests/test_object_spilling_3.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/python/ray/tests/test_object_spilling.py b/python/ray/tests/test_object_spilling.py index 50d85d4f9ef1..11fd750bfdfa 100644 --- a/python/ray/tests/test_object_spilling.py +++ b/python/ray/tests/test_object_spilling.py @@ -319,7 +319,7 @@ def test_spill_objects_automatically(fs_only_object_spilling_config, shutdown_on index = random.choice(list(range(buffer_length))) ref = replay_buffer[index] solution = solution_buffer[index] - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, solution) assert_no_thrashing(address["address"]) @@ -359,7 +359,7 @@ def test_unstable_spill_objects_automatically(unstable_spilling_config, shutdown index = random.choice(list(range(buffer_length))) ref = replay_buffer[index] solution = solution_buffer[index] - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, solution) assert_no_thrashing(address["address"]) @@ -397,7 +397,7 @@ def test_slow_spill_objects_automatically(slow_spilling_config, shutdown_only): index = random.choice(list(range(buffer_length))) ref = replay_buffer[index] solution = solution_buffer[index] - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, solution) assert_no_thrashing(address["address"]) diff --git a/python/ray/tests/test_object_spilling_2.py b/python/ray/tests/test_object_spilling_2.py index 9781952574f6..29b3e00ac13c 100644 --- a/python/ray/tests/test_object_spilling_2.py +++ b/python/ray/tests/test_object_spilling_2.py @@ -77,7 +77,7 @@ def test_delete_objects_delete_while_creating(object_spilling_config, shutdown_o # Do random sampling. for _ in range(200): ref = random.choice(replay_buffer) - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, arr) # After all, make sure all objects are killed without race condition. @@ -126,7 +126,7 @@ def create_objects(self): # Do random sampling. for _ in range(200): ref = random.choice(self.replay_buffer) - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, arr) a = Actor.remote() @@ -288,7 +288,7 @@ def test_fusion_objects(fs_only_object_spilling_config, shutdown_only): index = random.choice(list(range(buffer_length))) ref = replay_buffer[index] solution = solution_buffer[index] - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, solution) is_test_passing = False diff --git a/python/ray/tests/test_object_spilling_3.py b/python/ray/tests/test_object_spilling_3.py index 6b36245905af..aa3dcc9c978b 100644 --- a/python/ray/tests/test_object_spilling_3.py +++ b/python/ray/tests/test_object_spilling_3.py @@ -315,7 +315,7 @@ def test_spill_deadlock(object_spilling_config, shutdown_only): if random.randint(0, 9) < 5: for _ in range(5): ref = random.choice(replay_buffer) - sample = ray.get(ref, timeout=0) + sample = ray.get(ref, timeout=None) assert np.array_equal(sample, arr) assert_no_thrashing(address["address"]) From 56db6cacc6487ec4ded8cbc3af125870e3bfe327 Mon Sep 17 00:00:00 2001 From: Ricky Xu Date: Mon, 8 May 2023 17:21:56 +0800 Subject: [PATCH 3/5] update Signed-off-by: Ricky Xu --- python/ray/_private/worker.py | 23 +---------------------- src/ray/protobuf/usage.proto | 1 + 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index caa8abe089b7..692b49a40e6c 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -743,7 +743,7 @@ def get_objects(self, object_refs: list, timeout: Optional[float] = None): object_refs, self.current_task_id, timeout_ms ) debugger_breakpoint = b"" - for (data, metadata) in data_metadata_pairs: + for data, metadata in data_metadata_pairs: if metadata: metadata_fields = metadata.split(b",") if len(metadata_fields) >= 2 and metadata_fields[1].startswith( @@ -2480,26 +2480,6 @@ def get( Exception: An exception is raised if the task that created the object or that created one of the objects raised an exception. """ - if timeout == 0: - if os.environ.get("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "1") == "1": - import warnings - - warnings.warn( - ( - "Please use timeout=None if you expect ray.get() to block. " - "Setting timeout=0 in future ray releases will raise " - "GetTimeoutError if the objects references are not available. " - "You could suppress this warning by setting " - "RAY_WARN_RAY_GET_TIMEOUT_ZERO=0." - ), - UserWarning, - ) - - # Record this usage in telemetry - import ray._private.usage.usage_lib as usage_lib - - usage_lib.record_extra_usage_tag(usage_lib.TagKey.RAY_GET_TIMEOUT_ZERO, "True") - worker = global_worker worker.check_connected() @@ -2710,7 +2690,6 @@ def wait( worker.check_connected() # TODO(swang): Check main thread. with profiling.profile("ray.wait"): - # TODO(rkn): This is a temporary workaround for # https://github.com/ray-project/ray/issues/997. However, it should be # fixed in Arrow instead of here. diff --git a/src/ray/protobuf/usage.proto b/src/ray/protobuf/usage.proto index 0f5197cf1529..a9aa7d32ab94 100644 --- a/src/ray/protobuf/usage.proto +++ b/src/ray/protobuf/usage.proto @@ -108,6 +108,7 @@ enum TagKey { // If {true, false} setting of timeout =0 in `ray.get``, i.e. ray.get(..., timeout=0) // This is to track usage of the buggy behavior that will be fixed. // See https://github.com/ray-project/ray/issues/28465 for more details. + // Deprecated. RAY_GET_TIMEOUT_ZERO = 304; // Total number of tasks created. NUM_ACTOR_CREATION_TASKS = 305; From c404fd34a567d99eb6923e03ba925fecdaa70e68 Mon Sep 17 00:00:00 2001 From: Ricky Xu Date: Mon, 8 May 2023 17:31:32 +0800 Subject: [PATCH 4/5] doc Signed-off-by: Ricky Xu --- python/ray/_private/worker.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 692b49a40e6c..a9b81d672fb3 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2464,12 +2464,9 @@ def get( to get. timeout (Optional[float]): The maximum amount of time in seconds to wait before returning. Set this to None will block until the - corresponding object becomes available. - WARNING: In future ray releases ``timeout=0`` will return the object - immediately if it's available, else raise GetTimeoutError in accordance with - the above docstring. The current behavior of blocking until objects become - available of ``timeout=0`` is considered to be a bug, see - https://github.com/ray-project/ray/issues/28465. + corresponding object becomes available. Setting ``timeout=0`` will + return the object immediately if it's available, else raise + GetTimeoutError in accordance with the above docstring. Returns: A Python object or a list of Python objects. From 5cbdc3a9f500bcfd75eeee552e22df2963278b5c Mon Sep 17 00:00:00 2001 From: Ricky Xu Date: Tue, 9 May 2023 07:06:59 +0800 Subject: [PATCH 5/5] revert Signed-off-by: Ricky Xu --- src/ray/protobuf/usage.proto | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ray/protobuf/usage.proto b/src/ray/protobuf/usage.proto index a9aa7d32ab94..0f5197cf1529 100644 --- a/src/ray/protobuf/usage.proto +++ b/src/ray/protobuf/usage.proto @@ -108,7 +108,6 @@ enum TagKey { // If {true, false} setting of timeout =0 in `ray.get``, i.e. ray.get(..., timeout=0) // This is to track usage of the buggy behavior that will be fixed. // See https://github.com/ray-project/ray/issues/28465 for more details. - // Deprecated. RAY_GET_TIMEOUT_ZERO = 304; // Total number of tasks created. NUM_ACTOR_CREATION_TASKS = 305;