Don't print Redis connection warning in ray.init(). #8475
Conversation
|
Can one of the admins verify this patch? |
| else: | ||
| break | ||
| if counter == num_retries: | ||
| else: |
There was a problem hiding this comment.
I was a little hesitant to use this syntax because it's not widely known and non-obvious what it does, but thought it was ok since the intended behavior should be clear from the error message.
There was a problem hiding this comment.
I like it. Very pythonic.
| @@ -559,12 +556,10 @@ def wait_for_redis_to_start(redis_ip_address, | |||
| redis_client.client_list() | |||
| except redis.ConnectionError: | |||
| # Wait a little bit. | |||
| time.sleep(1) | |||
| logger.info("Failed to connect to the redis server, retrying.") | |||
There was a problem hiding this comment.
The important change is just removing this logging statement.
| @@ -559,12 +556,10 @@ def wait_for_redis_to_start(redis_ip_address, | |||
| redis_client.client_list() | |||
| except redis.ConnectionError: | |||
| # Wait a little bit. | |||
| time.sleep(1) | |||
There was a problem hiding this comment.
Also, this time.sleep was probably making ray.init() a second slower than it needed to be.
|
Test PASSed. |
python/ray/services.py
Outdated
|
|
||
| Raises: | ||
| Exception: An exception is raised if we could not connect with Redis. | ||
| """ | ||
| redis_client = redis.StrictRedis( | ||
| host=redis_ip_address, port=redis_port, password=password) | ||
| # Wait for the Redis server to start. | ||
| counter = 0 | ||
| while counter < num_retries: | ||
| num_retries = 5000 |
There was a problem hiding this comment.
Actually, is there a reason to not have num_retries as an arg anymore?
There was a problem hiding this comment.
My reasoning was that we may want to change the meaning of num_retries, e.g., in this PR we are changing it from seconds to milliseconds, and that could break things for callers who are using that argument.
|
Test FAILed. |
python/ray/services.py
Outdated
| time.sleep(1) | ||
| logger.info("Failed to connect to the redis server, retrying.") | ||
| counter += 1 | ||
| time.sleep(0.001) |
There was a problem hiding this comment.
Any reason not to just do exponential backoff here?
There was a problem hiding this comment.
No reason, just changed to use exponential backoff.
|
Test PASSed. |
This removes the warning that always happens when starting Ray but is useless.