Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1258

Close() for classes derived from FileAppender should be robust

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.0
    • Fix Version/s: 0.10.0
    • Component/s: Storage
    • Labels:
      None

      Description

      As I investigated, many appenders have possibility to cause NPE such as TAJO-1227.

      It seems checking null for each variables is ineffective.

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          ABORTED: Integrated in Tajo-master-CODEGEN-build #181 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/181/)
          TAJO-1258: Close() for classes derived from FileAppender should be robust. (Jongyoung Park via jinho) (jhkim: rev 809cba3758564acd4def17928f95de7b4c913c45)

          • CHANGES
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/parquet/ParquetAppender.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/avro/AvroAppender.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/CSVFile.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RowFile.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/sequencefile/SequenceFileAppender.java
          Show
          hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-CODEGEN-build #181 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/181/ ) TAJO-1258 : Close() for classes derived from FileAppender should be robust. (Jongyoung Park via jinho) (jhkim: rev 809cba3758564acd4def17928f95de7b4c913c45) CHANGES tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/parquet/ParquetAppender.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/avro/AvroAppender.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/CSVFile.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RowFile.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/sequencefile/SequenceFileAppender.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-master-build #542 (See https://builds.apache.org/job/Tajo-master-build/542/)
          TAJO-1258: Close() for classes derived from FileAppender should be robust. (Jongyoung Park via jinho) (jhkim: rev 809cba3758564acd4def17928f95de7b4c913c45)

          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/parquet/ParquetAppender.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java
          • CHANGES
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/avro/AvroAppender.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/CSVFile.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/sequencefile/SequenceFileAppender.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RowFile.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #542 (See https://builds.apache.org/job/Tajo-master-build/542/ ) TAJO-1258 : Close() for classes derived from FileAppender should be robust. (Jongyoung Park via jinho) (jhkim: rev 809cba3758564acd4def17928f95de7b4c913c45) tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/parquet/ParquetAppender.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java CHANGES tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/avro/AvroAppender.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/CSVFile.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/sequencefile/SequenceFileAppender.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RowFile.java
          Hide
          jhkim Jinho Kim added a comment -

          I've just committed it.
          Thanks!

          Show
          jhkim Jinho Kim added a comment - I've just committed it. Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tajo/pull/340

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/340
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/340#issuecomment-69210267

          +1 Looks good to me.
          I'll commit it after the TravisCI finished

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/340#issuecomment-69210267 +1 Looks good to me. I'll commit it after the TravisCI finished
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/340#issuecomment-69206083

          Thank you for your contribution.
          I'm reviewing the patch.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/340#issuecomment-69206083 Thank you for your contribution. I'm reviewing the patch.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user eminency opened a pull request:

          https://github.com/apache/tajo/pull/340

          TAJO-1258: Close() for classes derived from FileAppender should be robust

          TAJO-1258 branch is newly created and PR is re-created.

          I modified it using IOUtils.cleanup.

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

          $ git pull https://github.com/eminency/tajo TAJO-1258

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

          https://github.com/apache/tajo/pull/340.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 #340


          commit c0f3f4f0f996352737ffa40e428437d77f1a2cd2
          Author: Jongyoung Park <eminency@gmail.com>
          Date: 2015-01-08T16:22:44Z

          IOUtils.cleanup is used instead of direct close() in appenders.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user eminency opened a pull request: https://github.com/apache/tajo/pull/340 TAJO-1258 : Close() for classes derived from FileAppender should be robust TAJO-1258 branch is newly created and PR is re-created. I modified it using IOUtils.cleanup. You can merge this pull request into a Git repository by running: $ git pull https://github.com/eminency/tajo TAJO-1258 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/340.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 #340 commit c0f3f4f0f996352737ffa40e428437d77f1a2cd2 Author: Jongyoung Park <eminency@gmail.com> Date: 2015-01-08T16:22:44Z IOUtils.cleanup is used instead of direct close() in appenders.

            People

            • Assignee:
              eminency Jongyoung Park
              Reporter:
              eminency Jongyoung Park
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development