[Java] Fix setCurrentTask() in multi threading
#3821
Conversation
|
Test PASSed. |
|
Test FAILed. |
| // For normal task, we should set current task id to null as well. | ||
| runtime.getWorkerContext().setCurrentTask(null, null, null); | ||
| } | ||
|
|
||
| Thread.currentThread().setContextClassLoader(oldLoader); |
There was a problem hiding this comment.
Sorry I'm not clear on what classLoader is, but it seems like you could get rid of this line and pass in oldLoader in the setCurrentTask lines above?
There was a problem hiding this comment.
@stephanie-wang classLoader is used to tell JVM where and how to load a class into memory. We use different class loaders for different drivers. So different drivers can have classes with the same name without conflicts.
@jovany-wang classLoader should be set and reset in the same way as the driver id.
| @@ -63,18 +63,15 @@ public UniqueId getCurrentTaskId() { | |||
| * Set the current task which is being executed by the current worker. Note, this method can only | |||
| * be called from the main thread. | |||
| */ | |||
| public void setCurrentTask(TaskSpec task, ClassLoader classLoader) { | |||
| public void setCurrentTask(UniqueId currentTaskId, | |||
There was a problem hiding this comment.
Set currentTaskId to UniqueId.NIL if it's null?
There was a problem hiding this comment.
right, but maybe it's more clear to just pass in UniqueId.NIL
java/runtime/src/main/java/org/ray/runtime/AbstractRayRuntime.java
Outdated
Show resolved
Hide resolved
| @@ -63,18 +63,15 @@ public UniqueId getCurrentTaskId() { | |||
| * Set the current task which is being executed by the current worker. Note, this method can only | |||
| * be called from the main thread. | |||
| */ | |||
| public void setCurrentTask(TaskSpec task, ClassLoader classLoader) { | |||
| public void setCurrentTask(UniqueId currentTaskId, | |||
There was a problem hiding this comment.
right, but maybe it's more clear to just pass in UniqueId.NIL
| // For normal task, we should set current task id to null as well. | ||
| runtime.getWorkerContext().setCurrentTask(null, null, null); | ||
| } | ||
|
|
||
| Thread.currentThread().setContextClassLoader(oldLoader); |
There was a problem hiding this comment.
@stephanie-wang classLoader is used to tell JVM where and how to load a class into memory. We use different class loaders for different drivers. So different drivers can have classes with the same name without conflicts.
@jovany-wang classLoader should be set and reset in the same way as the driver id.
|
@raulchen @stephanie-wang |
|
Test PASSed. |
|
Test PASSed. |
What do these changes do?
For actor tasks , we should not set current driver id to null.
Related issue number
N/A