HeartSaVioR commented on a change in pull request #27010: [SPARK-30313][CORE]
Ensure EndpointRef is available MasterWebUI/WorkerPage
URL: https://github.com/apache/spark/pull/27010#discussion_r362117776
##########
File path: core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala
##########
@@ -68,11 +82,10 @@ private[netty] class Dispatcher(nettyEnv: NettyRpcEnv,
numUsableCores: Int) exte
if (stopped) {
throw new IllegalStateException("RpcEnv has been stopped")
}
- if (endpoints.putIfAbsent(name, getMessageLoop(name, endpoint)) != null)
{
+ if (endpoints.putIfAbsent(name, assignToMessageLoop(name, endpoint,
endpointRef)) != null) {
Review comment:
> To be fair, the previous code also has that problem w.r.t. the message
loop being modified.
Yes, that's the reason I just put the band-aid there and rename the method
as well. I tried to provide smallest change as the goal of patch is to just fix
the thread-safety issue.
But basically I totally agree about your suggestion, especially having
`register` to DedicatedMessageLoop explicitly and not registering endpoint in
`findMessageLoop`. Will reflect. Thanks for the suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
