You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by liu-zhaokun <gi...@git.apache.org> on 2017/11/30 11:06:45 UTC
[GitHub] spark pull request #19856: [SPARK-22664] The logs about "Connected to Zookee...
GitHub user liu-zhaokun opened a pull request:
https://github.com/apache/spark/pull/19856
[SPARK-22664] The logs about "Connected to Zookeeper" in ReliableKafkaReceiver.scala are in wrong position
[https://issues.apache.org/jira/browse/SPARK-22664](https://issues.apache.org/jira/browse/SPARK-22664)
The logs about "Connecting to Zookeeper" and "Connected to Zookeeper" in ReliableKafkaReceiver.scala should be printed when new a zkClient() in line 122.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liu-zhaokun/spark master1130
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19856.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 #19856
----
commit 5b5cb46ff83b06d28c26a238cc5e10dadf3bde3f
Author: liuzhaokun <li...@zte.com.cn>
Date: 2017-11-30T11:03:48Z
[SPARK-22664] The logs about Connected to Zookeeper in ReliableKafkaReceiver.scala are in wrong position
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19856
You shouldn't create a JIRA for items like this, where the description is the same as the PR. I wouldn't move the initial log, as creating the Consumer is part of connecting. It won't matter much. But in case of error you will have logged what you were connecting to first.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/19856
This is trivial and not obviously a useful change, so it should be closed
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19856
**[Test build #4002 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4002/testReport)** for PR 19856 at commit [`652068b`](https://github.com/apache/spark/commit/652068b58f4140206d461bc95cdac4531f750e65).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:
https://github.com/apache/spark/pull/19856
@jerryshao
Yes,you are right,but this log is not accurate,I think it should log like this "consumerconnector has been created",it is too ambiguity now.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19856
>I think the log can't reflect the behavior of consumer connection,because consumer.create doesn't do any connect,it only construct a ZookeeperConsumerConnector instance
That's not true, `ZookeeperConsumerConnector` will also connect to ZK during initialization, please see here (https://github.com/apache/kafka/blob/c9f930e3fe25e5675e64550c8b396356c5349ca7/core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala#L126).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:
https://github.com/apache/spark/pull/19856
@srowen
Thanks for your reply.Could you help me review it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:
https://github.com/apache/spark/pull/19856
@jerryshao
I think the log can't reflect the behavior of consumer connection,because consumer.create doesn't do any connect,it only construct a ZookeeperConsumerConnector instance,so the right position is where zkclient connect to zk. ReliableKafkaReceiver is not marked deprecated,so users will use it.To avoid misleading by this log when a problem of connect to zk occur ,I think we should fix it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:
https://github.com/apache/spark/pull/19856
@srowen
Please help merge this PR as it has passed all tests.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19856: [SPARK-22664] The logs about "Connected to Zookee...
Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun closed the pull request at:
https://github.com/apache/spark/pull/19856
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19856
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19856
Actually there's no issue here, IMHO I think your understanding of this log is slightly different from the original purpose.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/19856
I guess the original purpose of such log is to reflect the behavior of consumer connection. It is not super necessary to do such trivial change. Also `ReliableKafkaReceiver` is not recommended any more, let's keep as it is.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19856: [SPARK-22664] The logs about "Connected to Zookeeper" in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19856
**[Test build #4002 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4002/testReport)** for PR 19856 at commit [`652068b`](https://github.com/apache/spark/commit/652068b58f4140206d461bc95cdac4531f750e65).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org