You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rdblue <gi...@git.apache.org> on 2016/07/07 16:38:50 UTC

[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

GitHub user rdblue opened a pull request:

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

    SPARK-16420: Ensure compression streams are closed.

    ## What changes were proposed in this pull request?
    
    This uses the try/finally pattern to ensure streams are closed after use. `UnsafeShuffleWriter` wasn't closing compression streams, causing them to leak resources until garbage collected. This was causing a problem with codecs that use off-heap memory.
    
    ## How was this patch tested?
    
    Current tests are sufficient. This should not change behavior.
    


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

    $ git pull https://github.com/rdblue/spark SPARK-16420-unsafe-shuffle-writer-leak

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

    https://github.com/apache/spark/pull/14093.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 #14093
    
----
commit 14ba62197e57ace8940ce71f5a6e5608b6765661
Author: Ryan Blue <bl...@apache.org>
Date:   2016-07-07T00:50:40Z

    SPARK-16420: Ensure compression streams are closed.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r70111252
  
    --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/UnsafeShuffleWriter.java ---
    @@ -349,12 +349,19 @@ void forceSorterToSpill() throws IOException {
             for (int i = 0; i < spills.length; i++) {
               final long partitionLengthInSpill = spills[i].partitionLengths[partition];
               if (partitionLengthInSpill > 0) {
    -            InputStream partitionInputStream =
    -              new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    -            if (compressionCodec != null) {
    -              partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +            InputStream partitionInputStream = null;
    +            boolean innerThrewException = true;
    --- End diff --
    
    Yeah, I agree with @rdblue here; note that we use this `Closables.close` pattern elsewhere in Spark's Java code as well.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    **[Test build #61916 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61916/consoleFull)** for PR 14093 at commit [`14ba621`](https://github.com/apache/spark/commit/14ba62197e57ace8940ce71f5a6e5608b6765661).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61926/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r69942754
  
    --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/UnsafeShuffleWriter.java ---
    @@ -349,12 +349,19 @@ void forceSorterToSpill() throws IOException {
             for (int i = 0; i < spills.length; i++) {
               final long partitionLengthInSpill = spills[i].partitionLengths[partition];
               if (partitionLengthInSpill > 0) {
    -            InputStream partitionInputStream =
    -              new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    -            if (compressionCodec != null) {
    -              partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +            InputStream partitionInputStream = null;
    +            boolean innerThrewException = true;
    +            try {
    +               partitionInputStream =
    +                       new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    +              if (compressionCodec != null) {
    +                partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +              }
    +              ByteStreams.copy(partitionInputStream, mergedFileOutputStream);
    +              innerThrewException = false;
    +            } finally {
    +              Closeables.close(partitionInputStream, innerThrewException);
    --- End diff --
    
    Not that it matters much, but what about also using the `Utils.tryWithSafeFinally` pattern here for some consistency?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r70105908
  
    --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/UnsafeShuffleWriter.java ---
    @@ -349,12 +349,19 @@ void forceSorterToSpill() throws IOException {
             for (int i = 0; i < spills.length; i++) {
               final long partitionLengthInSpill = spills[i].partitionLengths[partition];
               if (partitionLengthInSpill > 0) {
    -            InputStream partitionInputStream =
    -              new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    -            if (compressionCodec != null) {
    -              partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +            InputStream partitionInputStream = null;
    +            boolean innerThrewException = true;
    --- End diff --
    
    This is the typical way to use `Closeables.close` in Java. The same pattern is used for the outer try/finally that is already there and is in [Guava's documentation|https://google.github.io/guava/releases/19.0/api/docs/com/google/common/io/Closeables.html#close(java.io.Closeable, boolean)].
    
    I'm reluctant to change from the standard pattern for aesthetic reasons when `finally` is the correct control flow -- the stream should always be closed regardless of exceptions.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    LGTM pending Jenkins.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61916/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    **[Test build #61926 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61926/consoleFull)** for PR 14093 at commit [`601f934`](https://github.com/apache/spark/commit/601f934372922b3b68424d3ef5a3cc81fd0f4500).
     * 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    **[Test build #61916 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61916/consoleFull)** for PR 14093 at commit [`14ba621`](https://github.com/apache/spark/commit/14ba62197e57ace8940ce71f5a6e5608b6765661).
     * This patch **fails Spark unit 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    **[Test build #61926 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61926/consoleFull)** for PR 14093 at commit [`601f934`](https://github.com/apache/spark/commit/601f934372922b3b68424d3ef5a3cc81fd0f4500).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r69943578
  
    --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/UnsafeShuffleWriter.java ---
    @@ -349,12 +349,19 @@ void forceSorterToSpill() throws IOException {
             for (int i = 0; i < spills.length; i++) {
               final long partitionLengthInSpill = spills[i].partitionLengths[partition];
               if (partitionLengthInSpill > 0) {
    -            InputStream partitionInputStream =
    -              new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    -            if (compressionCodec != null) {
    -              partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +            InputStream partitionInputStream = null;
    +            boolean innerThrewException = true;
    +            try {
    +               partitionInputStream =
    +                       new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    +              if (compressionCodec != null) {
    +                partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +              }
    +              ByteStreams.copy(partitionInputStream, mergedFileOutputStream);
    +              innerThrewException = false;
    +            } finally {
    +              Closeables.close(partitionInputStream, innerThrewException);
    --- End diff --
    
    Oh duh it's Java. I have made that mistake about 10 times now


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r70028071
  
    --- Diff: common/network-common/src/main/java/org/apache/spark/network/util/LimitedInputStream.java ---
    @@ -102,4 +118,10 @@ public LimitedInputStream(InputStream in, long limit) {
         left -= skipped;
         return skipped;
       }
    +  @Override
    --- End diff --
    
    add a blank line here?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r69943044
  
    --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/UnsafeShuffleWriter.java ---
    @@ -349,12 +349,19 @@ void forceSorterToSpill() throws IOException {
             for (int i = 0; i < spills.length; i++) {
               final long partitionLengthInSpill = spills[i].partitionLengths[partition];
               if (partitionLengthInSpill > 0) {
    -            InputStream partitionInputStream =
    -              new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    -            if (compressionCodec != null) {
    -              partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +            InputStream partitionInputStream = null;
    +            boolean innerThrewException = true;
    +            try {
    +               partitionInputStream =
    +                       new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    +              if (compressionCodec != null) {
    +                partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +              }
    +              ByteStreams.copy(partitionInputStream, mergedFileOutputStream);
    +              innerThrewException = false;
    +            } finally {
    +              Closeables.close(partitionInputStream, innerThrewException);
    --- End diff --
    
    This is the usual equivalent for Java code.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61988/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    **[Test build #61988 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61988/consoleFull)** for PR 14093 at commit [`661539f`](https://github.com/apache/spark/commit/661539f82ef924cfa4fe8a43f570c1a93dac816f).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

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

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    **[Test build #61988 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61988/consoleFull)** for PR 14093 at commit [`661539f`](https://github.com/apache/spark/commit/661539f82ef924cfa4fe8a43f570c1a93dac816f).
     * 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #14093: SPARK-16420: Ensure compression streams are closed.

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/14093
  
    Merging in master/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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #14093: SPARK-16420: Ensure compression streams are close...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14093#discussion_r70028149
  
    --- Diff: core/src/main/java/org/apache/spark/shuffle/sort/UnsafeShuffleWriter.java ---
    @@ -349,12 +349,19 @@ void forceSorterToSpill() throws IOException {
             for (int i = 0; i < spills.length; i++) {
               final long partitionLengthInSpill = spills[i].partitionLengths[partition];
               if (partitionLengthInSpill > 0) {
    -            InputStream partitionInputStream =
    -              new LimitedInputStream(spillInputStreams[i], partitionLengthInSpill);
    -            if (compressionCodec != null) {
    -              partitionInputStream = compressionCodec.compressedInputStream(partitionInputStream);
    +            InputStream partitionInputStream = null;
    +            boolean innerThrewException = true;
    --- End diff --
    
    this is a little bit strange - can we change it to do try catch and at the end of try we close with the flag false, and in catch we close with the flag true and then eliminate the variable?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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