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

IllegalArgumentException: RawFileScanner

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.9.0
    • Fix Version/s: 0.9.0
    • Component/s: Data Shuffle, Storage
    • Labels:
      None

      Description

      When I ran my customer query and TPCH-Q9, got this error.

      Q9
      select 
        nation, o_year, sum(amount) as sum_profit
      from 
        (
      select 
        n_name as nation, substr(o_orderdate, 1, 4) as o_year, 
        l_extendedprice * (1 - l_discount) -  ps_supplycost * l_quantity as amount
          from
            orders o join
            (select l_extendedprice, l_discount, l_quantity, l_orderkey, n_name, ps_supplycost 
             from part p join
               (select l_extendedprice, l_discount, l_quantity, l_partkey, l_orderkey, 
                       n_name, ps_supplycost 
                from partsupp ps join
                  (select l_suppkey, l_extendedprice, l_discount, l_quantity, l_partkey, 
                          l_orderkey, n_name 
                   from
                     (select s_suppkey, n_name 
                      from nation n join supplier s on n.n_nationkey = s.s_nationkey
                     ) s1 join lineitem l on s1.s_suppkey = l.l_suppkey
                  ) l1 on ps.ps_suppkey = l1.l_suppkey and ps.ps_partkey = l1.l_partkey
               ) l2 on p.p_name like '%green%' and p.p_partkey = l2.l_partkey
           ) l3 on o.o_orderkey = l3.l_orderkey
        )profit
      group by nation, o_year
      order by nation, o_year desc;
      
      ERROR: java.lang.IllegalArgumentException
      java.io.IOException: java.lang.IllegalArgumentException
      	at org.apache.tajo.engine.planner.physical.HashShuffleFileWriteExec.next(HashShuffleFileWriteExec.java:152)
      	at org.apache.tajo.worker.Task.run(Task.java:446)
      	at org.apache.tajo.worker.TaskRunner$1.run(TaskRunner.java:276)
      	at java.lang.Thread.run(Thread.java:745)
      Caused by: java.lang.IllegalArgumentException
      	at java.nio.Buffer.limit(Buffer.java:267)
      	at org.apache.tajo.storage.RawFile$RawFileScanner.fillBuffer(RawFile.java:154)
      	at org.apache.tajo.storage.RawFile$RawFileScanner.next(RawFile.java:263)
      	at org.apache.tajo.storage.MergeScanner.next(MergeScanner.java:95)
      	at org.apache.tajo.engine.planner.physical.SeqScanExec.next(SeqScanExec.java:268)
      	at org.apache.tajo.engine.planner.physical.HashJoinExec.next(HashJoinExec.java:117)
      	at org.apache.tajo.engine.planner.physical.ProjectionExec.next(ProjectionExec.java:54)
      	at org.apache.tajo.engine.planner.physical.ProjectionExec.next(ProjectionExec.java:54)
      	at org.apache.tajo.engine.planner.physical.HashShuffleFileWriteExec.next(HashShuffleFileWriteExec.java:107)
      	... 3 more
      
      1. TAJO-1097_2.jinho.patch
        6 kB
        Jinho Kim
      2. TAJO-1097.jinho.patch
        2 kB
        Jinho Kim

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user mhthanh opened a pull request:

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

        TAJO-1097: IllegalArgumentException: RawFileScanner

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

        $ git pull https://github.com/mhthanh/tajo TAJO-1097

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

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


        commit de9f7112731e4053613252e4e7e2d924020f5b0a
        Author: mhthanh <maihaithanh@gmail.com>
        Date: 2014-10-07T05:43:35Z

        TAJO-1097


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user mhthanh opened a pull request: https://github.com/apache/tajo/pull/183 TAJO-1097 : IllegalArgumentException: RawFileScanner You can merge this pull request into a Git repository by running: $ git pull https://github.com/mhthanh/tajo TAJO-1097 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/183.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 #183 commit de9f7112731e4053613252e4e7e2d924020f5b0a Author: mhthanh <maihaithanh@gmail.com> Date: 2014-10-07T05:43:35Z TAJO-1097
        Hide
        mhthanh Mai Hai Thanh added a comment -

        Hi Jinho Kim, please check your queries again with my patch in the pull request above.

        Show
        mhthanh Mai Hai Thanh added a comment - Hi Jinho Kim , please check your queries again with my patch in the pull request above.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/183#discussion_r18511422

        — Diff: tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java —
        @@ -151,7 +151,11 @@ private boolean fillBuffer() throws IOException {
        long realRemaining = fragment.getEndKey() - numBytesRead;
        numBytesRead += bytesRead;
        if (realRemaining < bufferSize) {

        • buffer.limit(currentDataSize + (int) realRemaining);
          + int newLimit = currentDataSize + (int) realRemaining;
          + if(newLimit > bufferSize) { + newLimit = bufferSize; + }

          + buffer.limit(newLimit);

            • End diff –

        I think we don't need buffer.limit. we already check the EOF condition in next().

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/183#discussion_r18511422 — Diff: tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java — @@ -151,7 +151,11 @@ private boolean fillBuffer() throws IOException { long realRemaining = fragment.getEndKey() - numBytesRead; numBytesRead += bytesRead; if (realRemaining < bufferSize) { buffer.limit(currentDataSize + (int) realRemaining); + int newLimit = currentDataSize + (int) realRemaining; + if(newLimit > bufferSize) { + newLimit = bufferSize; + } + buffer.limit(newLimit); End diff – I think we don't need buffer.limit. we already check the EOF condition in next().
        Hide
        jhkim Jinho Kim added a comment -

        Mai Hai Thanh
        Thank you for contribution!
        I've update the patch. Could you review and merge?

        Show
        jhkim Jinho Kim added a comment - Mai Hai Thanh Thank you for contribution! I've update the patch. Could you review and merge?
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12673337/TAJO-1097.jinho.patch
        against master revision d0f9ebc.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 349 release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-storage.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/513//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/513//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/513//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/513//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12673337/TAJO-1097.jinho.patch against master revision d0f9ebc. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 349 release audit warnings. +1 core tests. The patch passed unit tests in tajo-storage. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/513//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/513//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/513//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/513//console This message is automatically generated.
        Hide
        mhthanh Mai Hai Thanh added a comment -

        Jinho Kim, I think that we cannot remove the part of setting buffer limit in fillBuffer(). The reason is that the fragment we need to scan may end before the end of the physical file (that is, fragment.startOffset + fragment.length < fragment's physical file's file size). If we remove, there may be no Exception but the query's result may not be correct.

        Show
        mhthanh Mai Hai Thanh added a comment - Jinho Kim , I think that we cannot remove the part of setting buffer limit in fillBuffer(). The reason is that the fragment we need to scan may end before the end of the physical file (that is, fragment.startOffset + fragment.length < fragment's physical file's file size). If we remove, there may be no Exception but the query's result may not be correct.
        Hide
        jhkim Jinho Kim added a comment -

        Mai Hai Thanh
        OK, I understood. I missed fragment length of TAJO-983
        Could you add test case with fragments in TestStorage?

        Show
        jhkim Jinho Kim added a comment - Mai Hai Thanh OK, I understood. I missed fragment length of TAJO-983 Could you add test case with fragments in TestStorage?
        Hide
        mhthanh Mai Hai Thanh added a comment -

        Jinho Kim, I updated my patch (in the pull request) with 2 modifications from your patch. Your modification at function reset() clearly prevents a potential hidden bug. Could you review it ?
        I also thought about the unit test. However, this bug is a very special case and I have no idea how to test it without modifying the RawFile's interface, which is not good. Do you think that it is really necessary to add a unit test here ? If yes, can you give me an advice ?

        Show
        mhthanh Mai Hai Thanh added a comment - Jinho Kim , I updated my patch (in the pull request) with 2 modifications from your patch. Your modification at function reset() clearly prevents a potential hidden bug. Could you review it ? I also thought about the unit test. However, this bug is a very special case and I have no idea how to test it without modifying the RawFile's interface, which is not good. Do you think that it is really necessary to add a unit test here ? If yes, can you give me an advice ?
        Hide
        jhkim Jinho Kim added a comment -

        Mai Hai Thanh
        termination condition of RawFile was changed, so we should need the unit test.
        I will add the unit test from your patch.

        Show
        jhkim Jinho Kim added a comment - Mai Hai Thanh termination condition of RawFile was changed, so we should need the unit test. I will add the unit test from your patch.
        Hide
        jhkim Jinho Kim added a comment -

        Mai Hai Thanh
        your patch looks good to me. I've added the unit test from your patch.
        Could you merge this patch?

        Show
        jhkim Jinho Kim added a comment - Mai Hai Thanh your patch looks good to me. I've added the unit test from your patch. Could you merge this patch?
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12673517/TAJO-1097_2.jinho.patch
        against master revision 0dfa397.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 350 release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-storage.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/514//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/514//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/514//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/514//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12673517/TAJO-1097_2.jinho.patch against master revision 0dfa397. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 350 release audit warnings. +1 core tests. The patch passed unit tests in tajo-storage. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/514//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/514//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/514//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/514//console This message is automatically generated.
        Hide
        mhthanh Mai Hai Thanh added a comment -

        Jinho Kim, thank you for your help. I merged your unit test to my patch. (But, I omit the updating of tableStats in fillBuffer() because it will be done in close()).

        Show
        mhthanh Mai Hai Thanh added a comment - Jinho Kim , thank you for your help. I merged your unit test to my patch. (But, I omit the updating of tableStats in fillBuffer() because it will be done in close()).
        Hide
        jhkim Jinho Kim added a comment -

        +1
        OK, I will create new issue(TAJO-1108)
        Thank you for your contribution!!

        Show
        jhkim Jinho Kim added a comment - +1 OK, I will create new issue( TAJO-1108 ) Thank you for your contribution!!
        Hide
        jhkim Jinho Kim added a comment -

        committed it !

        Show
        jhkim Jinho Kim added a comment - committed it !
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

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

        SUCCESS: Integrated in Tajo-master-build #397 (See https://builds.apache.org/job/Tajo-master-build/397/)
        TAJO-1097: IllegalArgumentException: RawFileScanner. (Mai Hai Thanh via jinho) (jhkim: rev 0b2ea889b06166539aabdc0fdd9029008e5f1f7a)

        • CHANGES
        • tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java
        • tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #397 (See https://builds.apache.org/job/Tajo-master-build/397/ ) TAJO-1097 : IllegalArgumentException: RawFileScanner. (Mai Hai Thanh via jinho) (jhkim: rev 0b2ea889b06166539aabdc0fdd9029008e5f1f7a) CHANGES tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-CODEGEN-build #39 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/39/)
        TAJO-1097: IllegalArgumentException: RawFileScanner. (Mai Hai Thanh via jinho) (jhkim: rev 0b2ea889b06166539aabdc0fdd9029008e5f1f7a)

        • tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java
        • CHANGES
        • tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-CODEGEN-build #39 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/39/ ) TAJO-1097 : IllegalArgumentException: RawFileScanner. (Mai Hai Thanh via jinho) (jhkim: rev 0b2ea889b06166539aabdc0fdd9029008e5f1f7a) tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java CHANGES tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-block_iteration-branch-build #15 (See https://builds.apache.org/job/Tajo-block_iteration-branch-build/15/)
        TAJO-1097: IllegalArgumentException: RawFileScanner. (Mai Hai Thanh via jinho) (jhkim: rev 0b2ea889b06166539aabdc0fdd9029008e5f1f7a)

        • tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
        • tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-block_iteration-branch-build #15 (See https://builds.apache.org/job/Tajo-block_iteration-branch-build/15/ ) TAJO-1097 : IllegalArgumentException: RawFileScanner. (Mai Hai Thanh via jinho) (jhkim: rev 0b2ea889b06166539aabdc0fdd9029008e5f1f7a) tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java CHANGES

          People

          • Assignee:
            mhthanh Mai Hai Thanh
            Reporter:
            jhkim Jinho Kim
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development