[SPARK-39468][CORE] Improve `RpcAddress` to add `[]` to `IPv6` if needed #36868

dongjoon-hyun · 2022-06-14T18:07:00Z

What changes were proposed in this pull request?

This PR aims to extend the IPv6 support in RpcAddress additionally when the input doesn't have [] properly.

Why are the changes needed?

Note that Apache Spark already depends on java.net.URI getHost and getPort and it assumpts []-style IPv6. This PR additionally handles the case where the given host string doesn't have [].

RpcAddress.fromURIString

spark/core/src/main/scala/org/apache/spark/rpc/RpcAddress.scala

Lines 40 to 43 in 683179c

    
           def fromURIString(uri: String): RpcAddress = { 
        
             val uriObj = new java.net.URI(uri) 
        
             RpcAddress(uriObj.getHost, uriObj.getPort) 
        
           }

Utils.extractHostPortFromSparkUrl

spark/core/src/main/scala/org/apache/spark/util/Utils.scala

Lines 2520 to 2524 in 683179c

    
           def extractHostPortFromSparkUrl(sparkUrl: String): (String, Int) = { 
        
             try { 
        
               val uri = new java.net.URI(sparkUrl) 
        
               val host = uri.getHost 
        
               val port = uri.getPort

We need to handle Java URI IPv6 style additionally.

jshell> var uri = new java.net.URI("https://[::1]:80")
uri ==> https://[::1]:80

jshell> uri.getHost()
$4 ==> "[::1]"

jshell> uri.getPort()
$5 ==> 80

Does this PR introduce any user-facing change?

No. This is private[spark] class.

How was this patch tested?

Pass the CIs with newly added test cases.

This is also tested manually on IPv6-only environment with the following command.

$ SERIAL_SBT_TESTS=1 SPARK_LOCAL_HOSTNAME='[2600:.(omitted)..:60cd]' build/sbt "core/test" -Djava.net.preferIPv6Addresses=true -Dtest.exclude.tags=org.apache.spark.tags.ExtendedLevelDBTest
...
[info] Run completed in 18 minutes, 43 seconds.
[info] Total number of tests run: 2950
[info] Suites: completed 284, aborted 0
[info] Tests: succeeded 2950, failed 0, canceled 4, ignored 8, pending 0
[info] All tests passed.
[info] Passed: Total 3214, Failed 0, Errors 0, Passed 3214, Ignored 8, Canceled 4
[success] Total time: 1189 s (19:49), completed Jun 14, 2022, 4:45:55 PM

dongjoon-hyun · 2022-06-14T20:17:58Z

Could you review this, @Ngone51 ?

dongjoon-hyun · 2022-06-14T20:50:49Z

core/src/test/scala/org/apache/spark/rpc/RpcAddressSuite.scala

  }
+
+  test("SPARK-39468: IPv6 hostPort") {
+    val address = RpcAddress("::1", 1234)


This is the main use case of this PR.

dongjoon-hyun · 2022-06-14T22:59:38Z

cc @cloud-fan and @HyukjinKwon , too

dongjoon-hyun · 2022-06-15T02:59:04Z

Thank you so much, @cloud-fan . Merged to master

mridulm · 2022-06-15T17:22:49Z

core/src/main/scala/org/apache/spark/rpc/RpcAddress.scala

-private[spark] case class RpcAddress(host: String, port: Int) {
+private[spark] case class RpcAddress(_host: String, port: Int) {
+
+  val host: String = Utils.addBracketsIfNeeded(_host)


This can be a lazy val instead of val ?
We end up almost doubling the size of the serialized instance of this object otherwise.

Ya, of course, we can. Thank you for review, @mridulm .

Actually, at the first commit, I used def host first to save the size . Later I switched to val in the second commit.

ed10d51

If the memory consumption of RpcAddress instances matter, we can revert back def host instead of lazy val.

Among three options, which one do you prefer, @mridulm and @cloud-fan ?

def host (Initial commit)

lazy val host (New alternative)

val host (AS-IS)

I prefer lazy val - it keeps the serialized size the same, while making the runtime cost of accessing host is almost the same as before (which is increased by def host).

Thank you, @mridulm . Here is the followup.

https://github.com/apache/spark/pull/36882/files

### What changes were proposed in this pull request? This PR aims to use `lazy val host` instead of `val host`. ### Why are the changes needed? To address the review comments about `RpcAddress` object size. - #36868 (comment) - #36868 (comment) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. Closes #36882 from dongjoon-hyun/SPARK-39468-2. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

Resolves kubeflow#1344 Spark 3.4 supports IPv6: - apache/spark#36868 So I want to make the operator support IPv6. I can confirm that this can submit the spark-job in IPv6-only environment. Although it is necessary to add the following environment variables to the operator ```yaml apiVersion: apps/v1 kind: Deployment metadata: name: spark-on-k8s-spark-operator spec: template: spec: containers: - name: spark-operator env: - name: _JAVA_OPTIONS value: "-Djava.net.preferIPv6Addresses=true" - name: KUBERNETES_DISABLE_HOSTNAME_VERIFICATION value: "true" ```

Resolves #1344 Spark 3.4 supports IPv6: - apache/spark#36868 So I want to make the operator support IPv6. I can confirm that this can submit the spark-job in IPv6-only environment. Although it is necessary to add the following environment variables to the operator ```yaml apiVersion: apps/v1 kind: Deployment metadata: name: spark-on-k8s-spark-operator spec: template: spec: containers: - name: spark-operator env: - name: _JAVA_OPTIONS value: "-Djava.net.preferIPv6Addresses=true" - name: KUBERNETES_DISABLE_HOSTNAME_VERIFICATION value: "true" ```

Resolves kubeflow#1344 Spark 3.4 supports IPv6: - apache/spark#36868 So I want to make the operator support IPv6. I can confirm that this can submit the spark-job in IPv6-only environment. Although it is necessary to add the following environment variables to the operator ```yaml apiVersion: apps/v1 kind: Deployment metadata: name: spark-on-k8s-spark-operator spec: template: spec: containers: - name: spark-operator env: - name: _JAVA_OPTIONS value: "-Djava.net.preferIPv6Addresses=true" - name: KUBERNETES_DISABLE_HOSTNAME_VERIFICATION value: "true" ``` Signed-off-by: Peter McClonski <[email protected]>

Resolves kubeflow#1344 Spark 3.4 supports IPv6: - apache/spark#36868 So I want to make the operator support IPv6. I can confirm that this can submit the spark-job in IPv6-only environment. Although it is necessary to add the following environment variables to the operator ```yaml apiVersion: apps/v1 kind: Deployment metadata: name: spark-on-k8s-spark-operator spec: template: spec: containers: - name: spark-operator env: - name: _JAVA_OPTIONS value: "-Djava.net.preferIPv6Addresses=true" - name: KUBERNETES_DISABLE_HOSTNAME_VERIFICATION value: "true" ```

github-actions bot added the CORE label Jun 14, 2022

[SPARK-39468][CORE] Support IPv6 in RpcAddress

8793ef6

dongjoon-hyun marked this pull request as draft June 14, 2022 18:18

dongjoon-hyun marked this pull request as ready for review June 14, 2022 19:23

dongjoon-hyun commented Jun 14, 2022

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-39468][CORE] Support IPv6 in RpcAddress~~ [SPARK-39468][CORE] Improve RpcAddress to add [] to IPv6 if needed Jun 14, 2022

def -> val

ed10d51

cloud-fan approved these changes Jun 15, 2022

View reviewed changes

dongjoon-hyun closed this in 7cfc40f Jun 15, 2022

dongjoon-hyun deleted the SPARK-39468 branch June 15, 2022 03:00

LuciferYang mentioned this pull request Jun 15, 2022

[SPARK-39464][CORE][TESTS][FOLLOWUP] Use Utils.localHostNameForURI instead of Utils.localCanonicalHostName in tests #36876

Closed

mridulm reviewed Jun 15, 2022

View reviewed changes

dongjoon-hyun mentioned this pull request Jun 15, 2022

[SPARK-39468][CORE][FOLLOWUP] Use lazy val for host #36882

Closed

LittleWat mentioned this pull request Aug 30, 2023

Improve getMasterURL() to add [] to IPv6 if needed kubeflow/spark-operator#1825

Merged

julianStreibel mentioned this pull request Mar 21, 2025

Add support for IPv6 and iceberg with spark >= 3.4 flyteorg/flytekit#3206

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-39468][CORE] Improve `RpcAddress` to add `[]` to `IPv6` if needed #36868

[SPARK-39468][CORE] Improve `RpcAddress` to add `[]` to `IPv6` if needed #36868

Uh oh!

dongjoon-hyun commented Jun 14, 2022 •

edited

Loading

Uh oh!

dongjoon-hyun commented Jun 14, 2022

Uh oh!

dongjoon-hyun Jun 14, 2022

Uh oh!

dongjoon-hyun commented Jun 14, 2022

Uh oh!

dongjoon-hyun commented Jun 15, 2022

Uh oh!

mridulm Jun 15, 2022

Uh oh!

dongjoon-hyun Jun 15, 2022 •

edited

Loading

Uh oh!

mridulm Jun 15, 2022

Uh oh!

dongjoon-hyun Jun 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	def fromURIString(uri: String): RpcAddress = {
	val uriObj = new java.net.URI(uri)
	RpcAddress(uriObj.getHost, uriObj.getPort)
	}

	def extractHostPortFromSparkUrl(sparkUrl: String): (String, Int) = {
	try {
	val uri = new java.net.URI(sparkUrl)
	val host = uri.getHost
	val port = uri.getPort

[SPARK-39468][CORE] Improve RpcAddress to add [] to IPv6 if needed #36868

[SPARK-39468][CORE] Improve RpcAddress to add [] to IPv6 if needed #36868

Uh oh!

Conversation

dongjoon-hyun commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Jun 14, 2022

Uh oh!

dongjoon-hyun Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Jun 14, 2022

Uh oh!

dongjoon-hyun commented Jun 15, 2022

Uh oh!

mridulm Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mridulm Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SPARK-39468][CORE] Improve `RpcAddress` to add `[]` to `IPv6` if needed #36868

[SPARK-39468][CORE] Improve `RpcAddress` to add `[]` to `IPv6` if needed #36868

dongjoon-hyun commented Jun 14, 2022 •

edited

Loading

dongjoon-hyun Jun 15, 2022 •

edited

Loading