[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-11 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62642430
  
Done - https://github.com/netty/netty/issues/3129


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-11 Thread trustin
Github user trustin commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62521117
  
@rxin, @normanmaurer told me via IM that he has a patch pending. Could you 
file an issue and CC him?


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-11 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/3172


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-11 Thread aarondav
Github user aarondav commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62515931
  
Merging into master and branch-1.2.0.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62503515
  
  [Test build #23190 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23190/consoleFull)
 for   PR 3172 at commit 
[`0bdcdc6`](https://github.com/apache/spark/commit/0bdcdc6cd85400744f9c1cf67d48ed3b0780a2cc).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:
  * `class IndexShuffleBlockManager(conf: SparkConf) extends 
ShuffleBlockManager `
  * `public final class LazyFileRegion extends AbstractReferenceCounted 
implements FileRegion `



---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62503519
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23190/
Test PASSed.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62498606
  
  [Test build #23190 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23190/consoleFull)
 for   PR 3172 at commit 
[`0bdcdc6`](https://github.com/apache/spark/commit/0bdcdc6cd85400744f9c1cf67d48ed3b0780a2cc).
 * This patch merges cleanly.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62498230
  
cc @normanmaurer @trustin

Do you think we can add this directly to Netty? Doing this at Spark level 
means Epoll won't support this feature. 


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread aarondav
Github user aarondav commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62475386
  
Cool, everything LGTM. Would you mind adding a unit test for 
LazyFileRegion, though, in case people try to modify it in the future?


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62458882
  
@aarondav 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 this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62451826
  
  [Test build #515 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/515/consoleFull)
 for   PR 3172 at commit 
[`d4564ae`](https://github.com/apache/spark/commit/d4564aeb3adb1bc9b5d35b0fa5db4f9c188369d1).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62438643
  
  [Test build #515 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/515/consoleFull)
 for   PR 3172 at commit 
[`d4564ae`](https://github.com/apache/spark/commit/d4564aeb3adb1bc9b5d35b0fa5db4f9c188369d1).
 * This patch merges cleanly.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62360009
  
  [Test build #23147 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23147/consoleFull)
 for   PR 3172 at commit 
[`d4564ae`](https://github.com/apache/spark/commit/d4564aeb3adb1bc9b5d35b0fa5db4f9c188369d1).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:
  * `class IndexShuffleBlockManager(conf: SparkConf) extends 
ShuffleBlockManager `
  * `public final class LazyFileRegion extends AbstractReferenceCounted 
implements FileRegion `



---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62360017
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23147/
Test FAILed.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62353061
  
  [Test build #23147 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23147/consoleFull)
 for   PR 3172 at commit 
[`d4564ae`](https://github.com/apache/spark/commit/d4564aeb3adb1bc9b5d35b0fa5db4f9c188369d1).
 * This patch merges cleanly.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-09 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62349907
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23143/
Test FAILed.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-09 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62349897
  
  [Test build #23143 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23143/consoleFull)
 for   PR 3172 at commit 
[`6ed369e`](https://github.com/apache/spark/commit/6ed369e777fb82a86a8afe866f93bb6f80fad33b).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:
  * `public final class LazyFileRegion extends AbstractReferenceCounted 
implements FileRegion `



---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-09 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62348934
  
  [Test build #23143 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23143/consoleFull)
 for   PR 3172 at commit 
[`6ed369e`](https://github.com/apache/spark/commit/6ed369e777fb82a86a8afe866f93bb6f80fad33b).
 * This patch merges cleanly.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62291934
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23111/
Test FAILed.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62291933
  
  [Test build #23111 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23111/consoleFull)
 for   PR 3172 at commit 
[`04cddc8`](https://github.com/apache/spark/commit/04cddc82343a698b4b128782c2982d646afd0672).
 * This patch **fails to build**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:
  * `public final class LazyFileRegion extends AbstractReferenceCounted 
implements FileRegion `



---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread aarondav
Github user aarondav commented on a diff in the pull request:

https://github.com/apache/spark/pull/3172#discussion_r20056740
  
--- Diff: 
network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockManager.java
 ---
@@ -56,16 +58,24 @@
   // Single-threaded Java executor used to perform expensive recursive 
directory deletion.
   private final Executor directoryCleaner;
 
-  public ExternalShuffleBlockManager() {
+  private final TransportConf conf;
+
+  public ExternalShuffleBlockManager(TransportConf conf) {
 // TODO: Give this thread a name.
-this(Executors.newSingleThreadExecutor());
+this(Executors.newSingleThreadExecutor(), conf);
   }
 
   // Allows tests to have more control over when directories are cleaned 
up.
   @VisibleForTesting
-  ExternalShuffleBlockManager(Executor directoryCleaner) {
+  ExternalShuffleBlockManager(Executor directoryCleaner, TransportConf 
conf) {
--- End diff --

nit: please make conf first parameter to be consistent with other classes


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread aarondav
Github user aarondav commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62291922
  
LGTM, just a few minor style nits.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread aarondav
Github user aarondav commented on a diff in the pull request:

https://github.com/apache/spark/pull/3172#discussion_r20056737
  
--- Diff: 
network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockManager.java
 ---
@@ -56,16 +58,24 @@
   // Single-threaded Java executor used to perform expensive recursive 
directory deletion.
   private final Executor directoryCleaner;
 
-  public ExternalShuffleBlockManager() {
+  private final TransportConf conf;
+
+  public ExternalShuffleBlockManager(TransportConf conf) {
 // TODO: Give this thread a name.
-this(Executors.newSingleThreadExecutor());
+this(Executors.newSingleThreadExecutor(), conf);
   }
 
   // Allows tests to have more control over when directories are cleaned 
up.
   @VisibleForTesting
-  ExternalShuffleBlockManager(Executor directoryCleaner) {
+  ExternalShuffleBlockManager(Executor directoryCleaner, TransportConf 
conf) {
 this.executors = Maps.newConcurrentMap();
 this.directoryCleaner = directoryCleaner;
+this.conf = conf;
+  }
+
+  @VisibleForTesting
--- End diff --

we probably don't need 2 VisibleForTesting constructors, do we? If someone 
wants to provide an explicit Executor, they can damn well create their own 
TransportConf. This is only done in a single unit test anyway.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3172#issuecomment-62291803
  
  [Test build #23111 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23111/consoleFull)
 for   PR 3172 at commit 
[`04cddc8`](https://github.com/apache/spark/commit/04cddc82343a698b4b128782c2982d646afd0672).
 * This patch merges cleanly.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread aarondav
Github user aarondav commented on a diff in the pull request:

https://github.com/apache/spark/pull/3172#discussion_r20056709
  
--- Diff: 
network/common/src/main/java/org/apache/spark/network/buffer/FileSegmentManagedBuffer.java
 ---
@@ -31,27 +31,27 @@
 
 import org.apache.spark.network.util.JavaUtils;
 import org.apache.spark.network.util.LimitedInputStream;
+import org.apache.spark.network.util.SystemPropertyConfigProvider;
+import org.apache.spark.network.util.TransportConf;
 
 /**
  * A {@link ManagedBuffer} backed by a segment in a file.
  */
 public final class FileSegmentManagedBuffer extends ManagedBuffer {
-
-  /**
-   * Memory mapping is expensive and can destabilize the JVM (SPARK-1145, 
SPARK-3889).
-   * Avoid unless there's a good reason not to.
-   */
-  // TODO: Make this configurable
-  private static final long MIN_MEMORY_MAP_BYTES = 2 * 1024 * 1024;
-
   private final File file;
   private final long offset;
   private final long length;
+  private final TransportConf conf;
 
-  public FileSegmentManagedBuffer(File file, long offset, long length) {
+  public FileSegmentManagedBuffer(File file, long offset, long length, 
TransportConf conf) {
 this.file = file;
 this.offset = offset;
 this.length = length;
+this.conf = conf;
+  }
+
+  public FileSegmentManagedBuffer(File file, long offset, long length) {
--- End diff --

Please add a comment that the other constructor is preferred (this creates 
an extra object, for instance, and gets configuration from a random place).


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread aarondav
Github user aarondav commented on a diff in the pull request:

https://github.com/apache/spark/pull/3172#discussion_r20056703
  
--- Diff: 
core/src/main/scala/org/apache/spark/shuffle/FileShuffleBlockManager.scala ---
@@ -22,6 +22,9 @@ import java.nio.ByteBuffer
 import java.util.concurrent.ConcurrentLinkedQueue
 import java.util.concurrent.atomic.AtomicInteger
 
+import org.apache.spark.network.netty.SparkTransportConf
--- End diff --

nit: import order


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4307] Initialize FileDescriptor lazily ...

2014-11-08 Thread rxin
GitHub user rxin opened a pull request:

https://github.com/apache/spark/pull/3172

[SPARK-4307] Initialize FileDescriptor lazily in FileRegion.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rxin/spark lazyFD

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/3172.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #3172


commit 04cddc82343a698b4b128782c2982d646afd0672
Author: Reynold Xin 
Date:   2014-11-09T05:04:10Z

[SPARK-4307] Initialize FileDescriptor lazily in FileRegion.




---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org