[SPARK-16587] [CORE] The annotation for the abstract Class 'BlockTransferMessage' missing several kind of messages. #14232
Conversation
biaoma-ty
changed the title
biaoma-ty
changed the title
|
Can one of the admins verify this patch? |
| * - RegisterExecutor is only handled by the external shuffle service. | ||
| * - StreamHandle is only handled by the OneForOneBlockFetcher. | ||
| * - RegisterDriver is only handled by the MesosExternalShuffleService. | ||
| * - HearBeat is only handled by the MesosExternalShuffleService. |
There was a problem hiding this comment.
HeartBeat. (This could also be an HTML list.) I am not sure these docs match the others. They're describing what's handled by the Netty service for external shuffle block handler, not just a class where they're used.
There was a problem hiding this comment.
Is it right to modify them like below ?
StreamHandle is only handled by the external shuffle service
RegisterDriver & HeatBeat is only handled by the external shuffle service while the cluster in the mesos deploy mode.
There was a problem hiding this comment.
Best to ask the author where possible; use Blame to see who wrote these parts. @aarondav ?
|
I'm not clear if this matches the intent of the other docs. I'd close this unless we can get a clearer picture of the intent of these messages. |
|
Hm, not sure what to do here. Unless we can get an informed double-check on the correctness of this, I think I'd just not do it. |
| * shuffle service. It returns a StreamHandle. | ||
| * - UploadBlock is only handled by the NettyBlockTransferService. | ||
| * - RegisterExecutor is only handled by the external shuffle service. | ||
| * - StreamHandle is only handled by the OneForOneBlockFetcher. |
There was a problem hiding this comment.
This is already covered in the documentation of OpenBlock, since StreamHandle is the reply to OpenBlock.
|
@f7753 I think just listing the message types here is not super useful. Yes, new messages were added and not mentioned in the documentation, but then adding just "foo is used by bar" doesn't really help in understanding the protocol at all. Could you close this PR please? |
4 participants
# What changes were proposed in this pull request?
Currently, the Messages used by shuffle: OpenBlock, UploadBlock, RegisterExecutor has a simple introduction for the usage, while the other three (StreamHandle, RegisterDriver, HearBeat) haven' t.
This PR added them.
# How was this patch tested?
Please let some commiters to review the added annotation to judge them.