Pig
  1. Pig
  2. PIG-3515

Shell commands are limited from OS buffer

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.12.0, 0.11.1
    • Fix Version/s: 0.13.0
    • Component/s: parser
    • Environment:

      Centos 6.5, Mac OS X 10.8.5

      Description

      Executing shell commands in pig scripts, may stuck due to Java buffer limitations.

      Example:
      %declare VAR `cat 100kbytes.txt`

      Produce:
      2013-10-09 15:25:56,825 [main] INFO org.apache.pig.tools.parameters.PreprocessorContext - Executing command : cat 100kbytes.txt

      The execution hungs, so you have to interrupt the program.

      ^C
      2013-10-09 15:26:10,066 [main] ERROR org.apache.pig.Main - ERROR 2999: Unexpected internal error. Error executing shell command: cat 100kbytes.txt. Command exit with exit code of 130

      Explanation:
      The problem lies in org.apache.pig.tools.parameters.PreprocessorContext#executeShellCommand method, line: 191 (trunk, at revision 1531874).

      We wait for the process to complete before we get all the output, but the process waits for an indefinite amount of time, due to the fact that there is no space left in the buffer and the output can't be delivered.

      References:
      [1] http://www.javaworld.com/javaworld/jw-12-2000/jw-1229-traps.html?page=2
      [2] http://vyvaks.wordpress.com/2006/05/27/does-runtimeexec-hangs-in-java/

      Solution:
      I attach a test case that illustrates the problem and a patch as a solution. I would really like some comments.

      1. PIG-3515-test.patch
        2 kB
        Anastasis Andronidis
      2. PIG-3515.patch
        2 kB
        Anastasis Andronidis

        Activity

        Hide
        Anastasis Andronidis added a comment -

        Test and solution in different files.

        Show
        Anastasis Andronidis added a comment - Test and solution in different files.
        Hide
        Aniket Mokashi added a comment -

        Looks good to me. Will commit this to trunk.

        Show
        Aniket Mokashi added a comment - Looks good to me. Will commit this to trunk.
        Hide
        Aniket Mokashi added a comment -

        Anastasis Andronidis, can you please send a note on pig-dev@ mailing-list to add yourself to contributors list?

        Show
        Aniket Mokashi added a comment - Anastasis Andronidis, can you please send a note on pig-dev@ mailing-list to add yourself to contributors list?
        Hide
        Cheolsoo Park added a comment -

        Assigning to Anastasis.

        Show
        Cheolsoo Park added a comment - Assigning to Anastasis.
        Hide
        Cheolsoo Park added a comment -

        I noticed that the Apache header is missing in the test file, and 2 spaces is used instead of 4. Aniket Mokashi, do you mind fixing them when you commit this? Thanks!

        Show
        Cheolsoo Park added a comment - I noticed that the Apache header is missing in the test file, and 2 spaces is used instead of 4. Aniket Mokashi , do you mind fixing them when you commit this? Thanks!
        Hide
        Aniket Mokashi added a comment -

        Cheolsoo Park, thanks for the comments. I will do that for sure. If you have more context on PreprocessorContext, can you also take a quick look at the patch?

        Show
        Aniket Mokashi added a comment - Cheolsoo Park , thanks for the comments. I will do that for sure. If you have more context on PreprocessorContext, can you also take a quick look at the patch?
        Hide
        Cheolsoo Park added a comment -

        Hi Aniket, sorry for the late reply. It looks good to me too. Anastasis Andronidis, thank you for the good catch! I didn't know this until now. lol

        Since I was already running the test, I went ahead and committed the patch to trunk. I didn't commit the test case because I thought this bug doesn't need a unit test. Please let me know if you want me to commit the test case.

        Show
        Cheolsoo Park added a comment - Hi Aniket, sorry for the late reply. It looks good to me too. Anastasis Andronidis , thank you for the good catch! I didn't know this until now. lol Since I was already running the test, I went ahead and committed the patch to trunk. I didn't commit the test case because I thought this bug doesn't need a unit test. Please let me know if you want me to commit the test case.
        Hide
        Anastasis Andronidis added a comment -

        I don't mind. I am just thinking that every test case is illustrating something, so maybe someone else can benefit from reading this code.

        Show
        Anastasis Andronidis added a comment - I don't mind. I am just thinking that every test case is illustrating something, so maybe someone else can benefit from reading this code.

          People

          • Assignee:
            Anastasis Andronidis
            Reporter:
            Anastasis Andronidis
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development