You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by neoremind <gi...@git.apache.org> on 2017/08/16 14:45:06 UTC
[GitHub] spark pull request #18965: [SPARK-21749][CORE] Add comments for MessageEncod...
GitHub user neoremind opened a pull request:
https://github.com/apache/spark/pull/18965
[SPARK-21749][CORE] Add comments for MessageEncoder to explain the wire format
## What changes were proposed in this pull request?
Spark RPC is built upon TCP tier and leverage netty framework. It would be better to have some comments in RPC module especially to explain the RPC message wire format since it is critical to understand what Message consists (header + payload). So Javadoc comments can be added into MessageEncoder class.
## How was this patch tested?
Only comments added. Existing unit tests are all passed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neoremind/spark add_comments
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18965.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18965
----
commit a65bc214be8c05a3901193f9cc57d00fbbf506d5
Author: xu.zhang <xu...@hulu.com>
Date: 2017-08-16T14:37:23Z
Add comments for MessageEncoder to explain the wire format
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][DOC] Add comments for MessageEncoder to ex...
Posted by neoremind <gi...@git.apache.org>.
Github user neoremind commented on the issue:
https://github.com/apache/spark/pull/18965
@srowen I see your concern, it is more internal oriented and maybe updated by developer as lib evolves, I will close the PR then. Thanks for reviewing!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][CORE] Add comments for MessageEncoder to e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18965
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][DOC] Add comments for MessageEncoder to ex...
Posted by neoremind <gi...@git.apache.org>.
Github user neoremind commented on the issue:
https://github.com/apache/spark/pull/18965
I see, anyway this is what I found when I dig into the wire protocol of spark rpc since wire format is a big part of understanding the message structure. If someone thinks this is not necessary I can close the PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18965: [SPARK-21749][DOC] Add comments for MessageEncode...
Posted by neoremind <gi...@git.apache.org>.
Github user neoremind closed the pull request at:
https://github.com/apache/spark/pull/18965
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][DOC] Add comments for MessageEncoder to ex...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/18965
I think the more important question is, is it a protocol that we are guaranteeing? not sure that is. If not then I don't think it should be in user-facing docs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][CORE] Add comments for MessageEncoder to e...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/18965
Seems you should add the `[DOC]` tag in your title.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][DOC] Add comments for MessageEncoder to ex...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/18965
We usually don't have the PRs which only add comments to explain something, so I'm neutral to this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18965: [SPARK-21749][CORE] Add comments for MessageEncoder to e...
Posted by neoremind <gi...@git.apache.org>.
Github user neoremind commented on the issue:
https://github.com/apache/spark/pull/18965
@jerryshao please review my separated PR. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org