diff options
author | Aaron Davidson <aaron@databricks.com> | 2013-10-12 12:18:23 -0700 |
---|---|---|
committer | Aaron Davidson <aaron@databricks.com> | 2013-10-12 22:44:57 -0700 |
commit | a3959111387ae955c63284b389cc888cbd19e399 (patch) | |
tree | 1b17b4b4a87a8ac6d50c299dfe3a5b82ddb4d3d5 /core/src/main/java/org | |
parent | dca80094d317363e1e0d7e32bc7dfd99faf943cf (diff) | |
download | spark-a3959111387ae955c63284b389cc888cbd19e399.tar.gz spark-a3959111387ae955c63284b389cc888cbd19e399.tar.bz2 spark-a3959111387ae955c63284b389cc888cbd19e399.zip |
Refactor BlockId into an actual type
This is an unfortunately invasive change which converts all of our BlockId
strings into actual BlockId types. Here are some advantages of doing this now:
+ Type safety
+ Code clarity - it's now obvious what the key of a shuffle or rdd block is,
for instance. Additionally, appearing in tuple/map type signatures is a big
readability bonus. A Seq[(String, BlockStatus)] is not very clear.
Further, we can now use more Scala features, like matching on BlockId types.
+ Explicit usage - we can now formally tell where various BlockIds are being used
(without doing string searches); this makes updating current BlockIds a much
clearer process, and compiler-supported.
(I'm looking at you, shuffle file consolidation.)
+ It will only get harder to make this change as time goes on.
Since this touches a lot of files, it'd be best to either get this patch
in quickly or throw it on the ground to avoid too many secondary merge conflicts.
Diffstat (limited to 'core/src/main/java/org')
-rw-r--r-- | core/src/main/java/org/apache/spark/network/netty/FileClientHandler.java | 3 | ||||
-rw-r--r-- | core/src/main/java/org/apache/spark/network/netty/FileServerHandler.java | 6 |
2 files changed, 6 insertions, 3 deletions
diff --git a/core/src/main/java/org/apache/spark/network/netty/FileClientHandler.java b/core/src/main/java/org/apache/spark/network/netty/FileClientHandler.java index c4aa2669e0..8a09210245 100644 --- a/core/src/main/java/org/apache/spark/network/netty/FileClientHandler.java +++ b/core/src/main/java/org/apache/spark/network/netty/FileClientHandler.java @@ -21,6 +21,7 @@ import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundByteHandlerAdapter; +import org.apache.spark.storage.BlockId; abstract class FileClientHandler extends ChannelInboundByteHandlerAdapter { @@ -33,7 +34,7 @@ abstract class FileClientHandler extends ChannelInboundByteHandlerAdapter { } public abstract void handle(ChannelHandlerContext ctx, ByteBuf in, FileHeader header); - public abstract void handleError(String blockId); + public abstract void handleError(BlockId blockId); @Override public ByteBuf newInboundBuffer(ChannelHandlerContext ctx) { diff --git a/core/src/main/java/org/apache/spark/network/netty/FileServerHandler.java b/core/src/main/java/org/apache/spark/network/netty/FileServerHandler.java index d3d57a0255..a5ec5e2fb1 100644 --- a/core/src/main/java/org/apache/spark/network/netty/FileServerHandler.java +++ b/core/src/main/java/org/apache/spark/network/netty/FileServerHandler.java @@ -24,6 +24,7 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundMessageHandlerAdapter; import io.netty.channel.DefaultFileRegion; +import org.apache.spark.storage.BlockId; class FileServerHandler extends ChannelInboundMessageHandlerAdapter<String> { @@ -34,8 +35,9 @@ class FileServerHandler extends ChannelInboundMessageHandlerAdapter<String> { } @Override - public void messageReceived(ChannelHandlerContext ctx, String blockId) { - String path = pResolver.getAbsolutePath(blockId); + public void messageReceived(ChannelHandlerContext ctx, String blockIdString) { + BlockId blockId = BlockId.fromString(blockIdString); + String path = pResolver.getAbsolutePath(blockId.filename()); // if getFilePath returns null, close the channel if (path == null) { //ctx.close(); |