You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/09/14 18:49:32 UTC
[kudu-CR] kudu-spark-tools: change default master address to local fqdn
Hello Attila Bukor, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/8072
to review the following change.
Change subject: kudu-spark-tools: change default master address to local fqdn
......................................................................
kudu-spark-tools: change default master address to local fqdn
The previous default was 'localhost', which isn't resolvable in a
cluster context. It would also cause issues on Kerberized clusters due
to 'localhost' not matching the Kerberos principal name of the master
(see KUDU-2142).
Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
---
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
1 file changed, 6 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/8072/1
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2: -Code-Review
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
Line 24: import org.apache.spark.sql.SparkSession
> never mind, checked it in the meantime, it was unused
Done
PS2, Line 38: InetAddress.getLocalHost.getCanonicalHostName
> We could change the method to be
I believe that would happen if the local hostname is not added to the /etc/hosts file, but let me try to reproduce it. I don't think that can happen otherwise.
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
PS2, Line 38: InetAddress.getLocalHost.getCanonicalHostName
> InetAddress.getLocalHost can throw. What happens if we throw in this contex
We could change the method to be
def defaultMasterAddrs: String = Try(InetAddress.getLocalHost.getCanonicalHostName).getOrElse("localhost");
But that has its own issues. Not really sure how to take this. How common is it for the local hostname not to be resolvable?
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
PS2, Line 38: InetAddress.getLocalHost.getCanonicalHostName
> I couldn't trigger an UnknownHostException environment despite I changed ns
k, seems like the output is reasonable at least.
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
Line 24: import org.apache.spark.sql.SparkSession
what happened to SQLContext? was it unused?
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
kudu-spark: change default master address to local fqdn
The previous default was 'localhost', which isn't resolvable in a
cluster context. It would also cause issues on Kerberized clusters due
to 'localhost' not matching the Kerberos principal name of the master
(see KUDU-2142).
Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Reviewed-on: http://gerrit.cloudera.org:8080/8072
Reviewed-by: Attila Bukor <ab...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
3 files changed, 23 insertions(+), 13 deletions(-)
Approvals:
Attila Bukor: Looks good to me, but someone else must approve
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
Line 24: import org.apache.spark.sql.SparkSession
> what happened to SQLContext? was it unused?
never mind, checked it in the meantime, it was unused
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
PS2, Line 38: InetAddress.getLocalHost.getCanonicalHostName
InetAddress.getLocalHost can throw. What happens if we throw in this context? Will the error message be interpretable?
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change.
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8072/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala:
PS2, Line 38: InetAddress.getLocalHost.getCanonicalHostName
> I believe that would happen if the local hostname is not added to the /etc/
I couldn't trigger an UnknownHostException environment despite I changed nsswitch.conf to "hosts: files" only, removing each line from /etc/hosts and setting hostname to "qwer". Running a simple Java application printing the result of InetAddress.getLocalHost().getHostName() printed out "qwer".
I don't know what I missed, but if someone can mess up their host worse than this, I believe it's better to fail with an exception than silently switch over to localhost (which would likely also fail):
It looks something like this (hardcoded asdfadfasdfas instead of getLocalHost):
17/09/19 10:47:21 INFO state.StateStoreCoordinatorRef: Registered StateStoreCoordinator endpoint
Exception in thread "main" java.net.UnknownHostException: asdfadfasdfas: Name or service not known
at java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method)
at java.net.InetAddress$2.lookupAllHostAddr(InetAddress.java:928)
at java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1323)
at java.net.InetAddress.getAllByName0(InetAddress.java:1276)
at java.net.InetAddress.getAllByName(InetAddress.java:1192)
at java.net.InetAddress.getAllByName(InetAddress.java:1126)
at java.net.InetAddress.getByName(InetAddress.java:1076)
at org.apache.kudu.spark.tools.IntegrationTestBigLinkedList$.defaultMasterAddrs(IntegrationTestBigLinkedList.scala:86)
at org.apache.kudu.spark.tools.Generator$Args$.apply$default$7(IntegrationTestBigLinkedList.scala:125)
at org.apache.kudu.spark.tools.Generator$Args$.parse(IntegrationTestBigLinkedList.scala:153)
at org.apache.kudu.spark.tools.Generator$.main(IntegrationTestBigLinkedList.scala:177)
at org.apache.kudu.spark.tools.IntegrationTestBigLinkedList$.main(IntegrationTestBigLinkedList.scala:92)
at org.apache.kudu.spark.tools.IntegrationTestBigLinkedList.main(IntegrationTestBigLinkedList.scala)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.spark.deploy.SparkSubmit$.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:755)
at org.apache.spark.deploy.SparkSubmit$.doRunMain$1(SparkSubmit.scala:180)
at org.apache.spark.deploy.SparkSubmit$.submit(SparkSubmit.scala:205)
at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:119)
at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala)
17/09/19 10:47:21 INFO spark.SparkContext: Invoking stop() from shutdown hook
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] kudu-spark: change default master address to local fqdn
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8072
to look at the new patch set (#2).
Change subject: kudu-spark: change default master address to local fqdn
......................................................................
kudu-spark: change default master address to local fqdn
The previous default was 'localhost', which isn't resolvable in a
cluster context. It would also cause issues on Kerberized clusters due
to 'localhost' not matching the Kerberos principal name of the master
(see KUDU-2142).
Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
---
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
3 files changed, 23 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/8072/2
--
To view, visit http://gerrit.cloudera.org:8080/8072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I10ec7414c451f54b95d86663d743162688e304ba
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>