[GitHub] spark pull request: [SPARK-12871][SQL] Support to specify the opti...

2016-01-19 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/10805#discussion_r50206066 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVParameters.scala --- @@ -107,3 +114,28 @@ private[csv] object

[GitHub] spark pull request: [SPARK-12871][SQL] Support to specify the opti...

2016-01-19 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/10805#discussion_r50206079 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVParameters.scala --- @@ -107,3 +114,28 @@ private[csv] object

[GitHub] spark pull request: [SPARK-9808] Remove spark.shuffle.consolidateF...

2015-08-10 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/8089#issuecomment-129673926 Heartbroken. Let's remove it. Perhaps we can document its impending deprecation in 1.5. --- If your project is set up for it, you can reply to this email

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-30 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7014#issuecomment-126415566 Looks good from my end. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] spark pull request: [SPARK-9397] DataFrame should provide an API t...

2015-07-28 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7717#issuecomment-125634257 Renamed to inputFiles and updated description. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] spark pull request: [SPARK-9397] DataFrame should provide an API t...

2015-07-27 Thread aarondav
GitHub user aarondav opened a pull request: https://github.com/apache/spark/pull/7717 [SPARK-9397] DataFrame should provide an API to find source data files if applicable Certain applications would benefit from being able to inspect DataFrames that are straightforwardly produced

[GitHub] spark pull request: [SPARK-9397] DataFrame should provide an API t...

2015-07-27 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7717#issuecomment-125458969 cc @marmbrus @rxin about exposing a new DataFrame API. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-22 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r35280503 --- Diff: core/src/test/scala/org/apache/spark/FailureSuite.scala --- @@ -141,5 +141,73 @@ class FailureSuite extends SparkFunSuite with LocalSparkContext

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-22 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r35280447 --- Diff: core/src/test/scala/org/apache/spark/FailureSuite.scala --- @@ -141,5 +141,73 @@ class FailureSuite extends SparkFunSuite with LocalSparkContext

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-22 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r35280981 --- Diff: core/src/test/scala/org/apache/spark/FailureSuite.scala --- @@ -141,5 +141,73 @@ class FailureSuite extends SparkFunSuite with LocalSparkContext

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-17 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34903771 --- Diff: core/src/main/scala/org/apache/spark/TaskEndReason.scala --- @@ -127,6 +146,19 @@ case class ExceptionFailure( } } +class

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-17 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34903699 --- Diff: core/src/main/scala/org/apache/spark/TaskEndReason.scala --- @@ -97,13 +103,26 @@ case class ExceptionFailure( description: String

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-17 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34903555 --- Diff: core/src/main/scala/org/apache/spark/TaskEndReason.scala --- @@ -127,6 +146,19 @@ case class ExceptionFailure( } } +class

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-17 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34903975 --- Diff: core/src/main/scala/org/apache/spark/TaskEndReason.scala --- @@ -97,13 +103,26 @@ case class ExceptionFailure( description: String

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-17 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34903614 --- Diff: core/src/main/scala/org/apache/spark/TaskEndReason.scala --- @@ -97,13 +103,26 @@ case class ExceptionFailure( description: String

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-07-16 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-122142207 Thanks! Merged into master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-16 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34855831 --- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala --- @@ -300,8 +300,16 @@ private[spark] class Executor( m

[GitHub] spark pull request: [SPARK-8625] [Core] Propagate user exceptions ...

2015-07-16 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/7014#discussion_r34855806 --- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala --- @@ -300,8 +300,16 @@ private[spark] class Executor( m

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-07-12 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-120777385 @squito, I have updated the PR and simplified it greatly, thus removing most of the magic. Now we simply append the current stack trace inside DAGScheduler

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-07-12 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-120778945 fetch failure - jenkins test this please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-07-03 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-118447192 The problem with any approach that wraps the exception is that we no longer throw an exception of the original type; we instead always throw SparkExceptions

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-30 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33596669 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -466,23 +531,13 @@ private[worker] class Worker( case

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-30 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5392#issuecomment-117354387 Just one comment thanks to your proactive TODOifying :) LGTM, feel free to merge after. --- If your project is set up for it, you can reply to this email and have

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-30 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33627528 --- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala --- @@ -504,6 +518,7 @@ private[master] class Master

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-06-30 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-117353792 retest this please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33515404 --- Diff: core/src/main/scala/org/apache/spark/deploy/client/AppClient.scala --- @@ -40,98 +37,139 @@ import org.apache.spark.util.{ActorLogReceive

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33518468 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -235,21 +250,47 @@ private[worker] class Worker( * still

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33519252 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -302,27 +367,27 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33519250 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -302,27 +367,27 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33519340 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -466,23 +531,13 @@ private[worker] class Worker( case

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33513145 --- Diff: core/src/main/scala/org/apache/spark/deploy/Client.scala --- @@ -36,20 +36,30 @@ import org.apache.spark.util.{ActorLogReceive, AkkaUtils

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33513862 --- Diff: core/src/main/scala/org/apache/spark/deploy/Client.scala --- @@ -82,29 +92,38 @@ private class ClientActor(driverArgs: ClientArguments, conf

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33513905 --- Diff: core/src/main/scala/org/apache/spark/deploy/Client.scala --- @@ -82,29 +92,38 @@ private class ClientActor(driverArgs: ClientArguments, conf

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33515502 --- Diff: core/src/main/scala/org/apache/spark/deploy/client/AppClient.scala --- @@ -40,98 +37,139 @@ import org.apache.spark.util.{ActorLogReceive

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33514893 --- Diff: core/src/main/scala/org/apache/spark/deploy/client/AppClient.scala --- @@ -40,98 +37,139 @@ import org.apache.spark.util.{ActorLogReceive

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33519086 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -258,41 +299,65 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33514995 --- Diff: core/src/main/scala/org/apache/spark/deploy/client/AppClient.scala --- @@ -40,98 +37,139 @@ import org.apache.spark.util.{ActorLogReceive

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33518043 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -181,24 +190,31 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33518507 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -235,21 +250,47 @@ private[worker] class Worker( * still

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33519394 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -510,13 +580,25 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33517979 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -181,24 +190,31 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5392#discussion_r33517925 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala --- @@ -181,24 +190,31 @@ private[worker] class Worker

[GitHub] spark pull request: [SPARK-6602][Core] Update Master, Worker, Clie...

2015-06-29 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5392#issuecomment-116860938 The core logic all looks good to me, just had some nits. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-06-25 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-115526307 I've updated the code to move the injection to runJob. This means that exceptions thrown from any path inside the DAGScheduler that fail the job will now have

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-06-25 Thread aarondav
GitHub user aarondav opened a pull request: https://github.com/apache/spark/pull/7028 [SPARK-8644] Include call site in SparkException stack traces thrown by job failures Example exception (new part at bottom, clearly demarcated): ``` org.apache.spark.SparkException

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-06-25 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-115422594 I think the most important thing is to include the user stack trace somewhere. Users don't really care what's going on as long as they can identify a line of code from

[GitHub] spark pull request: [SPARK-8644] Include call site in SparkExcepti...

2015-06-25 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/7028#issuecomment-115404640 cc @JoshRosen --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] spark pull request: [SPARK-8161] Set externalBlockStoreInitialized...

2015-06-17 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/6702#issuecomment-112904974 LGMT --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] spark pull request: [SPARK-8306] AddJar command needs to set the n...

2015-06-11 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/6758#discussion_r32253172 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/commands.scala --- @@ -91,7 +91,7 @@ case class AddJar(path: String) extends

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-05-08 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5575#issuecomment-100320805 cc @andrewor14 @pwendell We should consider merging this into the 1.4 branch in case there is another RC. It has been an outstanding issue for a while. --- If your

[GitHub] spark pull request: [SPARK-6627] Finished rename to ShuffleBlockRe...

2015-05-05 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5764#issuecomment-99260768 Yeah, it should be safe to rename that one. It's public due to being in Java, no cool `private[visibility]` modifiers. --- If your project is set up for it, you can

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-05-01 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5743#issuecomment-98166558 Just a couple remaining comments, otherwise this LGTM! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-05-01 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29510283 --- Diff: network/common/src/main/java/org/apache/spark/network/server/TransportRequestHandler.java --- @@ -82,10 +75,8 @@ public void exceptionCaught

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-05-01 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29510060 --- Diff: network/common/src/main/java/org/apache/spark/network/server/OneForOneStreamManager.java --- @@ -80,12 +95,17 @@ public ManagedBuffer getChunk

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-05-01 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29510128 --- Diff: network/common/src/main/java/org/apache/spark/network/server/OneForOneStreamManager.java --- @@ -42,11 +46,15 @@ private static class

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-05-01 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29510421 --- Diff: network/common/src/main/java/org/apache/spark/network/server/TransportRequestHandler.java --- @@ -102,7 +93,8 @@ public void handle

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-05-01 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5743#issuecomment-98205477 LGTM, merging in to master. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-04-30 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29453281 --- Diff: network/common/src/main/java/org/apache/spark/network/server/StreamManager.java --- @@ -44,6 +46,17 @@ public abstract ManagedBuffer

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-04-30 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29453868 --- Diff: network/common/src/main/java/org/apache/spark/network/server/OneForOneStreamManager.java --- @@ -56,6 +62,15 @@ public OneForOneStreamManager

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-04-30 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29452859 --- Diff: network/common/src/main/java/org/apache/spark/network/server/StreamManager.java --- @@ -44,6 +46,17 @@ public abstract ManagedBuffer

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-04-30 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5743#discussion_r29453981 --- Diff: network/common/src/main/java/org/apache/spark/network/server/OneForOneStreamManager.java --- @@ -56,6 +62,15 @@ public OneForOneStreamManager

[GitHub] spark pull request: [SPARK-7183][Network] Fix memory leak of Trans...

2015-04-28 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5743#issuecomment-97192903 This is a reasonable solution, but I think that the actual issue that TransportRequestHandler has a list of streamIds at all. I think a different solution would

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-27 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29195262 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-27 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29196457 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-27 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29204220 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,302 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121679 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121182 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121766 --- Diff: network/common/src/main/java/org/apache/spark/network/TransportContext.java --- @@ -99,9 +108,9 @@ public TransportServer createServer

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121411 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslRpcHandler.java --- @@ -46,19 +47,30 @@ /** Class which provides secret

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121094 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121561 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SparkSaslServer.java --- @@ -75,11 +81,20 @@ private final SecretKeyHolder

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121544 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SparkSaslServer.java --- @@ -60,13 +60,19 @@ static final String DIGEST

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121527 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SparkSaslServer.java --- @@ -60,13 +60,19 @@ static final String DIGEST

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5377#issuecomment-96514035 LGTM, only minor comments. The tests look good. Apologies for taking so long to review! --- If your project is set up for it, you can reply to this email and have your

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121332 --- Diff: network/common/src/main/java/org/apache/spark/network/sasl/SaslEncryption.java --- @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121814 --- Diff: network/common/src/test/java/org/apache/spark/network/sasl/SparkSaslSuite.java --- @@ -86,4 +117,237 @@ public void testNonMatching

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121626 --- Diff: network/common/src/test/resources/log4j.properties --- @@ -23,5 +23,5 @@ log4j.appender.file.file=target/unit-tests.log

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-26 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r29121639 --- Diff: network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleClient.java --- @@ -58,10 +60,15 @@ public

[GitHub] spark pull request: [SPARK-7120][SPARK-7121][WIP] Closure cleaner ...

2015-04-24 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5685#discussion_r29065533 --- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala --- @@ -167,15 +294,17 @@ private[spark] object ClosureCleaner extends Logging

[GitHub] spark pull request: [SPARK-7120][SPARK-7121][WIP] Closure cleaner ...

2015-04-24 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5685#discussion_r29081651 --- Diff: core/src/test/scala/org/apache/spark/util/ClosureCleanerSuite.scala --- @@ -50,7 +50,7 @@ class ClosureCleanerSuite extends FunSuite

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-20 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5584#issuecomment-94507189 Merging into master. We may consider backporting to 1.3 if it turns out this fixes [SPARK-6962](https://issues.apache.org/jira/browse/SPARK-6962), which is a pretty

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-04-20 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5575#discussion_r28721339 --- Diff: core/src/test/scala/org/apache/spark/network/netty/NettyBlockTransferServiceSuite.scala --- @@ -0,0 +1,78 @@ +/* + * Licensed

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-04-19 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5575#issuecomment-94243393 Jenkins, retest this please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-04-19 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5575#issuecomment-94307038 Jenkins, retest this please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-19 Thread aarondav
GitHub user aarondav opened a pull request: https://github.com/apache/spark/pull/5584 [SPARK-7003] Improve reliability of connection failure detection between Netty block transfer service endpoints Currently we rely on the assumption that an exception will be raised

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-19 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5584#issuecomment-94344712 cc @rxin --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-19 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5584#discussion_r28662180 --- Diff: network/common/src/main/java/org/apache/spark/network/util/MapConfigProvider.java --- @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-19 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5584#discussion_r28662130 --- Diff: network/common/src/main/java/org/apache/spark/network/util/MapConfigProvider.java --- @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-19 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5584#discussion_r28662125 --- Diff: network/common/src/test/java/org/apache/spark/network/RequestTimeoutIntegrationSuite.java --- @@ -0,0 +1,277 @@ +/* + * Licensed

[GitHub] spark pull request: [SPARK-7003] Improve reliability of connection...

2015-04-19 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5584#issuecomment-94347409 All comments addressed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-04-18 Thread aarondav
GitHub user aarondav opened a pull request: https://github.com/apache/spark/pull/5575 [SPARK-6955] Perform port retries at NettyBlockTransferService level Currently we're doing port retries in the TransportServer level, but this is not specified by the TransportContext API

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-04-18 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5575#issuecomment-94188006 err, cc @andrewor14, not @andrewor13. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] spark pull request: [SPARK-6955] Perform port retries at NettyBloc...

2015-04-18 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5575#issuecomment-94187977 cc @SaintBacchus @andrewor13 @vanzin This patch aims to resolve SPARK-6955 while keeping the fix for SPARK-5444. --- If your project is set up for it, you can reply

[GitHub] spark pull request: [SPARK-6955][NETWORK]Do not let Yarn Shuffle S...

2015-04-18 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5537#issuecomment-94187864 Ah, I realize now that the original change that brought this bug up (#4240) was perhaps not the right solution. The issue that #4240 solved was actually brought up

[GitHub] spark pull request: [SPARK-6955][NETWORK]Do not let Yarn Shuffle S...

2015-04-17 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5537#discussion_r28624950 --- Diff: core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala --- @@ -43,8 +43,12 @@ object SparkTransportConf { * @param

[GitHub] spark pull request: [SPARK-6955][NETWORK]Do not let Yarn Shuffle S...

2015-04-17 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5537#issuecomment-94062816 The changes as they currently stand LGTM, let's just fix up the style as @vanzin pointed out. --- If your project is set up for it, you can reply to this email

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-16 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r28489592 --- Diff: core/src/main/scala/org/apache/spark/network/netty/NettyBlockTransferService.scala --- @@ -49,18 +49,21 @@ class NettyBlockTransferService(conf

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-16 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r28489665 --- Diff: network/common/src/main/java/org/apache/spark/network/TransportContext.java --- @@ -55,14 +56,14 @@ private final Logger logger

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-16 Thread aarondav
Github user aarondav commented on the pull request: https://github.com/apache/spark/pull/5377#issuecomment-93677780 This is looking very good to me. I've reviewed the core transport part and I like the API. I will have to defer reviewing the rest of the SASL Encryption side

[GitHub] spark pull request: [SPARK-6229] Add SASL encryption to network li...

2015-04-16 Thread aarondav
Github user aarondav commented on a diff in the pull request: https://github.com/apache/spark/pull/5377#discussion_r28491206 --- Diff: network/common/src/main/java/org/apache/spark/network/TransportContext.java --- @@ -55,14 +56,14 @@ private final Logger logger

  1   2   3   4   5   6   7   8   9   10   >