-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-3797] Minor addendum to Yarn shuffle service #3144
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,9 @@ | |
|
||
package org.apache.spark.network.util; | ||
|
||
import java.nio.ByteBuffer; | ||
import java.nio.charset.Charset; | ||
|
||
import java.io.ByteArrayInputStream; | ||
import java.io.ByteArrayOutputStream; | ||
import java.io.Closeable; | ||
|
@@ -30,6 +33,7 @@ | |
|
||
public class JavaUtils { | ||
private static final Logger logger = LoggerFactory.getLogger(JavaUtils.class); | ||
private static final Charset UTF8_CHARSET = Charset.forName("UTF-8"); | ||
|
||
/** Closes the given object, ignoring IOExceptions. */ | ||
public static void closeQuietly(Closeable closeable) { | ||
|
@@ -73,4 +77,20 @@ public static int nonNegativeHash(Object obj) { | |
int hash = obj.hashCode(); | ||
return hash != Integer.MIN_VALUE ? Math.abs(hash) : 0; | ||
} | ||
|
||
/** | ||
* Convert the given string to a byte buffer. The resulting buffer can be | ||
* converted back to the same string through {@link #bytesToString(ByteBuffer)}. | ||
*/ | ||
public static ByteBuffer stringToBytes(String s) { | ||
return ByteBuffer.wrap(s.getBytes(UTF8_CHARSET)); | ||
} | ||
|
||
/** | ||
* Convert the given byte buffer to a string. The resulting string can be | ||
* converted back to the same byte buffer through {@link #stringToBytes(String)}. | ||
*/ | ||
public static String bytesToString(ByteBuffer b) { | ||
return new String(b.array(), UTF8_CHARSET); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not safe for bytebuffers which are allocated off-heap -- Netty has already looked at this problem, so maybe we should just use their solution:
For symmetry, we could use |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ import org.apache.hadoop.yarn.ipc.YarnRPC | |
import org.apache.hadoop.yarn.util.{Apps, ConverterUtils, Records, ProtoUtils} | ||
|
||
import org.apache.spark.{SecurityManager, SparkConf, Logging} | ||
import org.apache.spark.network.sasl.ShuffleSecretManager | ||
import org.apache.spark.network.util.JavaUtils | ||
|
||
@deprecated("use yarn/stable", "1.2.0") | ||
class ExecutorRunnable( | ||
|
@@ -98,7 +98,9 @@ class ExecutorRunnable( | |
val secretString = securityMgr.getSecretKey() | ||
val secretBytes = | ||
if (secretString != null) { | ||
ShuffleSecretManager.stringToBytes(secretString) | ||
// This uses a JavaUtils method because the reverse conversion takes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: not sure the comment is useful. Might as well say it uses JavaUtils because that's where the method is implemented... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah maybe. It's just that it's not super intuitive why we need to use |
||
// place in the Yarn shuffle service, which is implemented in Java | ||
JavaUtils.stringToBytes(secretString) | ||
} else { | ||
// Authentication is not enabled, so just provide dummy metadata | ||
ByteBuffer.allocate(0) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Charsets.UTF_8