[SPARK-12267][Core]Store the remote RpcEnv address to send the correct disconnetion message #10261
Conversation
|
CC @vanzin could you take a look? Thanks! |
| // the listening address | ||
| val remoteEnvAddress = requestMessage.senderAddress | ||
| if (remoteAddresses.putIfAbsent(clientAddr, remoteEnvAddress) == null) { | ||
| dispatcher.postToAll(RemoteProcessConnected(remoteEnvAddress)) |
There was a problem hiding this comment.
We don't need to concern about multiple network messages with different addresses since the codes already handle the bad addresses.
|
So, I don't think this is gonna work, and that's why my change is taking longer than I hoped to finish... while this might fix the immediate problem listed in the bug, there are other problems related to this that need to be fixed. Basically, standalone master HA is currently broken, and because there are no unit tests at all for that part of the code, nobody caught it... I'm trying to fix it, I think I almost got it, have one last exception to get rid of. |
|
Cool. I will close this PR. On Thu, Dec 10, 2015 at 6:28 PM Marcelo Vanzin notifications@github.com
|
|
hey guys, I know this is still in flight, but we are pretty behind on the release. I'm going to cut RC2 and we can always make a judgement call about doing an RC3 once this is fixed. |
|
Test build #47561 has finished for PR 10261 at commit
|
|
I saw your following comment in vanzin@3848bf5#diff-acd05d6d379b6ef6ccf36bd3db5614f6R69 But I don't get it. The communication in master-worker, worker-driver and master-driver are all in non-client mode. So for one-way messages, |
|
Test build #47588 has finished for PR 10261 at commit
|
That's because we took opposite approaches. Your PR's approach is "if this connection says the sender address is a listening socket, then also consider that address when sending events about the remote process". My PR takes the opposite approach: the address of the remote process is always the address of the socket used to connect, regardless of whether its also listening in another socket. I think my approach is in the end more correct, but requires more code to fix existing code. In my view, |
|
I see. However, for 1.6, we still need to support Akka RPC. Is it possible to get the client address from |
|
Akka doesn't have this problem as far as I understand the code; also because the akka rpc backend never runs in "client mode" (that's a netty rpc-only thing). |
|
BTW I'm fine with this change for 1.6; we can revisit this later to avoid having to keep extra state in the RPC layer. |
I didn't mean that. I just want to mention that if we want to change the semantics of
Great. I'm going to merge this one to master and 1.6. Let's revisit this later. |
…ct disconnetion message Author: Shixiong Zhu <shixiong@databricks.com> Closes #10261 from zsxwing/SPARK-12267. (cherry picked from commit 8af2f8c) Signed-off-by: Shixiong Zhu <shixiong@databricks.com>
|
@vanzin I created SPARK-12308 to remind us of your approach |
No description provided.