Pig
  1. Pig
  2. PIG-1231

Default DataBagIterator.hasNext() should be idempotent in all cases

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.6.0
    • Fix Version/s: 0.6.0
    • Component/s: impl
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      DefaultDataBagIterator.hasNext() is not repeatable when the below conditions met:
      1. There is no more tuple in the last spill file
      2. There is no tuples in memory (all contents are spilled to files)

      This is not acceptable cuz the name hasNext() implies that it is idempotent. In BagFormat, we do misuse DataBagIterator.hasNext() because of the assumption that hasNext() is always idempotent, which leads to some mysterious errors.

      Condition 2 seems to be very restrictive, but when the databag is really big, the memory can hold less than a couple of tuples, the chance to hit 2. is high enough.

      Here is one error we saw:

      Caused by: java.io.IOException: Stream closed
      at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:145)
      at java.io.BufferedInputStream.fill(BufferedInputStream.java:189)
      at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
      at java.io.DataInputStream.readByte(DataInputStream.java:248)
      at org.apache.pig.data.DefaultTuple.readFields(DefaultTuple.java:278)
      at org.apache.pig.data.DefaultDataBag$DefaultDataBagIterator.readFromFile(DefaultDataBag.java:237)
      ... 20 more

      This happens because: we call hasNext(), which reach EOF and we close the file. Then we call hasNext() again in the assumption that it is idempotent. However, the stream is closed so we get this error message.

      1. PIG-1231-2.patch
        0.8 kB
        Daniel Dai
      2. PIG-1231-1.patch
        3 kB
        Daniel Dai

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        5h 8m 1 Daniel Dai 09/Feb/10 00:59
        Resolved Resolved Reopened Reopened
        2d 5h 55m 1 Daniel Dai 12/Feb/10 01:41
        Reopened Reopened Patch Available Patch Available
        34s 1 Daniel Dai 12/Feb/10 01:42
        Patch Available Patch Available Resolved Resolved
        1d 13h 8m 2 Daniel Dai 12/Feb/10 20:03
        Resolved Resolved Closed Closed
        40d 2h 12m 1 Alan Gates 24/Mar/10 22:16
        Alan Gates made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Daniel Dai made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Hide
        Daniel Dai added a comment -

        Patch committed to both trunk and 0.6 branch.

        Show
        Daniel Dai added a comment - Patch committed to both trunk and 0.6 branch.
        Hide
        Daniel Dai added a comment -

        Seems hudson is not running the testing process at all. Manual test success in both trunk and 0.6 branch. Did not include new testcase since it is a fix to existing testcase.

        Show
        Daniel Dai added a comment - Seems hudson is not running the testing process at all. Manual test success in both trunk and 0.6 branch. Did not include new testcase since it is a fix to existing testcase.
        Hide
        Alan Gates added a comment -

        +1 for patch 2.

        Show
        Alan Gates added a comment - +1 for patch 2.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12435645/PIG-1231-2.patch
        against trunk revision 909210.

        +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 tests are needed for this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/202/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/202/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/202/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435645/PIG-1231-2.patch against trunk revision 909210. +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 tests are needed for this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/202/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/202/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/202/console This message is automatically generated.
        Daniel Dai made changes -
        Status Reopened [ 4 ] Patch Available [ 10002 ]
        Hadoop Flags [Reviewed]
        Daniel Dai made changes -
        Attachment PIG-1231-2.patch [ 12435645 ]
        Daniel Dai made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Daniel Dai added a comment -

        There is unit case failure in 0.6 branch.

        Show
        Daniel Dai added a comment - There is unit case failure in 0.6 branch.
        Daniel Dai made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Hide
        Daniel Dai added a comment -

        Patch committed to both trunk and 0.6 branch.

        Show
        Daniel Dai added a comment - Patch committed to both trunk and 0.6 branch.
        Hide
        Alan Gates added a comment -

        +1 Changes look good.

        Show
        Alan Gates added a comment - +1 Changes look good.
        Hide
        Daniel Dai added a comment -

        testCompressed1: java.lang.IllegalArgumentException: port out of range:-1. Not a real problem. Manual test passes.

        Show
        Daniel Dai added a comment - testCompressed1: java.lang.IllegalArgumentException: port out of range:-1. Not a real problem. Manual test passes.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12435230/PIG-1231-1.patch
        against trunk revision 907760.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/206/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/206/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/206/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435230/PIG-1231-1.patch against trunk revision 907760. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/206/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/206/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/206/console This message is automatically generated.
        Daniel Dai made changes -
        Summary DataBagIterator.hasNext() should be idempotent Default DataBagIterator.hasNext() should be idempotent in all cases
        Description DataBagIterator.hasNext() is not repeatable in some situations. This is not acceptable cuz the name hasNext() implies that it is idempotent. While hasNext() returns true, it is repeatable, but if hasNext() returns false, it is not. In BagFormat, we do misuse DataBagIterator.hasNext() because of the assumption that hasNext() is always idempotent, which leads to some mysterious errors. Here is one error we saw:

        Caused by: java.io.IOException: Stream closed
                at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:145)
                at java.io.BufferedInputStream.fill(BufferedInputStream.java:189)
                at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
                at java.io.DataInputStream.readByte(DataInputStream.java:248)
                at org.apache.pig.data.DefaultTuple.readFields(DefaultTuple.java:278)
                at org.apache.pig.data.DefaultDataBag$DefaultDataBagIterator.readFromFile(DefaultDataBag.java:237)
                ... 20 more

        This happens because: we call hasNext(), which reach EOF and we close the file. Then we call hasNext() again in the assumption that it is idempotent. However, the stream is closed so we get this error message.

        This fix will go to DefaultDataBagIterator, DistinctDataBagIterator, CachedBagIterator, SortedDataBagIterator.
        DefaultDataBagIterator.hasNext() is not repeatable when the below conditions met:
        1. There is no more tuple in the last spill file
        2. There is no tuples in memory (all contents are spilled to files)

        This is not acceptable cuz the name hasNext() implies that it is idempotent. In BagFormat, we do misuse DataBagIterator.hasNext() because of the assumption that hasNext() is always idempotent, which leads to some mysterious errors.

        Condition 2 seems to be very restrictive, but when the databag is really big, the memory can hold less than a couple of tuples, the chance to hit 2. is high enough.

        Here is one error we saw:

        Caused by: java.io.IOException: Stream closed
                at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:145)
                at java.io.BufferedInputStream.fill(BufferedInputStream.java:189)
                at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
                at java.io.DataInputStream.readByte(DataInputStream.java:248)
                at org.apache.pig.data.DefaultTuple.readFields(DefaultTuple.java:278)
                at org.apache.pig.data.DefaultDataBag$DefaultDataBagIterator.readFromFile(DefaultDataBag.java:237)
                ... 20 more

        This happens because: we call hasNext(), which reach EOF and we close the file. Then we call hasNext() again in the assumption that it is idempotent. However, the stream is closed so we get this error message.
        Daniel Dai made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Daniel Dai made changes -
        Attachment PIG-1231-1.patch [ 12435230 ]
        Hide
        Daniel Dai added a comment -

        DefaultDataBagIterator is the only DataBag has this problem. Other databag handles this through different mechanisms.

        Show
        Daniel Dai added a comment - DefaultDataBagIterator is the only DataBag has this problem. Other databag handles this through different mechanisms.
        Daniel Dai made changes -
        Field Original Value New Value
        Description Current implementation of DataBagIterator.hasNext() will actually fetch the next tuple every time. So if we call hasNext() consecutively, more than 1 tuples will be fetched. This is confusing cuz the name hasNext() implies that it is idempotent. In BagFormat, we do misuse DataBagIterator.hasNext() because of this, which leads to some mysterious errors. Here is one error we saw:

        Caused by: java.io.IOException: Stream closed
                at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:145)
                at java.io.BufferedInputStream.fill(BufferedInputStream.java:189)
                at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
                at java.io.DataInputStream.readByte(DataInputStream.java:248)
                at org.apache.pig.data.DefaultTuple.readFields(DefaultTuple.java:278)
                at org.apache.pig.data.DefaultDataBag$DefaultDataBagIterator.readFromFile(DefaultDataBag.java:237)
                ... 20 more

        This happens because: we call hasNext(), which reach EOF and we close the file. Then we call hasNext() again in the assumption that it is idempotent. However, the stream is closed so we get this error message.

        This fix will go to DefaultDataBagIterator, DistinctDataBagIterator, CachedBagIterator, SortedDataBagIterator.
        DataBagIterator.hasNext() is not repeatable in some situations. This is not acceptable cuz the name hasNext() implies that it is idempotent. While hasNext() returns true, it is repeatable, but if hasNext() returns false, it is not. In BagFormat, we do misuse DataBagIterator.hasNext() because of the assumption that hasNext() is always idempotent, which leads to some mysterious errors. Here is one error we saw:

        Caused by: java.io.IOException: Stream closed
                at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:145)
                at java.io.BufferedInputStream.fill(BufferedInputStream.java:189)
                at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
                at java.io.DataInputStream.readByte(DataInputStream.java:248)
                at org.apache.pig.data.DefaultTuple.readFields(DefaultTuple.java:278)
                at org.apache.pig.data.DefaultDataBag$DefaultDataBagIterator.readFromFile(DefaultDataBag.java:237)
                ... 20 more

        This happens because: we call hasNext(), which reach EOF and we close the file. Then we call hasNext() again in the assumption that it is idempotent. However, the stream is closed so we get this error message.

        This fix will go to DefaultDataBagIterator, DistinctDataBagIterator, CachedBagIterator, SortedDataBagIterator.
        Daniel Dai created issue -

          People

          • Assignee:
            Daniel Dai
            Reporter:
            Daniel Dai
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development