Change timeout from milliseconds to seconds in ray.wait.

[robertnishihara]

This does part of #3411. Earlier versions include #3629 and #3666. This is a breaking API change. However, we log a warning when an integer is passed into ray.wait.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10641/
Test FAILed.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10646/
Test FAILed.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10655/
Test PASSed.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10665/
Test PASSed.

[suquark]

I think we should also document it about this change. It is quite important to notify existing users about it, because this breaking change will not introduce exceptions for old code; instead users could think that ray.wait just stop working. 1000x slower looks like being stuck in most cases.

[robertnishihara]

I needed to make this change to build the documentation locally.

[richardliaw]

lgtm

[robertnishihara]

@suquark I added a prominent warning to the API documentation. It should now look like this

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10688/
Test PASSed.

[raulchen]

What is the purpose of this change? Is it because we want wait to be more aligned with Python's built-in functions like time.sleep?

Also, I'm afraid this change may be dangerous . As @suquark mentioned, if users didn't notice the document, their old code can still run with no exception. But wait just get 1000x slow. And this may even cause application logic error (assuming application logic may depending on the timeout).

If we do want to make this change. I think a safer way is to add another timeout_seconds parameter, and throw an exception if the users still use timeout parameter.

[pcmoritz]

@raulchen The reason for this PR is that python uses function(timeout=seconds) universally when there are timeouts.

We could also deal with this is to raise a deprecation warning for some grace period and keep the interpretation as milliseconds if the timeout is an int, and interpret it in seconds if it is a float (and raise no deprecation warning). Once we have done that for one release say, we change the behavior to be seconds for both ints and floats and remove the warning.

[pcmoritz]

The warning looks good to me and should be enough.

[robertnishihara]

@raulchen I agree it is definitely not ideal, but I think for Python users this will be much more intuitive. I'm hoping the warning messages will be sufficiently prominent.

Shall we do the same thing in the Java API? I'm not actually sure what would be most intuitive for Java users, e.g., Thread.sleep seems to take milliseconds [1], future.get takes an int and a time unit [2]

[1] https://www.tutorialspoint.com/java/lang/thread_sleep_millis.htm
[2] https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Future.html#get-long-java.util.concurrent.TimeUnit-

[raulchen]

@raulchen I agree it is definitely not ideal, but I think for Python users this will be much more intuitive. I'm hoping the warning messages will be sufficiently prominent.

Shall we do the same thing in the Java API? I'm not actually sure what would be most intuitive for Java users, e.g., Thread.sleep seems to take milliseconds [1], future.get takes an int and a time unit [2]

[1] https://www.tutorialspoint.com/java/lang/thread_sleep_millis.htm
[2] https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Future.html#get-long-java.util.concurrent.TimeUnit-

1. old Java API usually use long values in milliseconds for timeouts;
2. TimeUnit was introduced in Java 1.5, and is commonly used in the concurrent lib.
3. Java 8 introduced another Duration class to represent a period of time.

I don't prefer 2, because it requires 2 parameters for timeout. 1 and 3 are both okay.