Details

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

      Description

      The substitution feature i.e X=$X:/tmp doesnt work as expected.

      This issue completes the feature mentioned in HADOOP-2838. HADOOP-2838 provided a way to set env variables in child process. This issue provides a way to inherit tt's env variables and append or reset it. So now
      X=$X:y will inherit X (if there) and append y to it.

      1. hadoop5981-branch-20-example.patch
        7 kB
        Amar Kamat
      2. HADOOP-5981-v2.0.patch
        7 kB
        Amar Kamat
      3. HADOOP-5981-v1.1.patch
        2 kB
        Amar Kamat
      4. HADOOP-5981-v1.1-branch-20.patch
        2 kB
        Amar Kamat

        Issue Links

          Activity

          Hide
          Amar Kamat added a comment -

          Attaching a patch [HADOOP-5981-v1.1.patch] that fixes the issue. The issue got masked because of the improper merge of the two testcases. Tested this patch to see if it works. The testcase now passes. Running test-patch now.

          Note that [HADOOP-5981-v1.1-branch-20.patch] is an example patch not to be committed to branch 20.

          Show
          Amar Kamat added a comment - Attaching a patch [HADOOP-5981-v1.1.patch] that fixes the issue. The issue got masked because of the improper merge of the two testcases. Tested this patch to see if it works. The testcase now passes. Running test-patch now. Note that [HADOOP-5981-v1.1-branch-20.patch] is an example patch not to be committed to branch 20.
          Hide
          Amar Kamat added a comment -

          Result of test-patch
          [exec] +1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 3 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

          Show
          Amar Kamat added a comment - Result of test-patch [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          Hide
          Sreekanth Ramakrishnan added a comment -

          The changes look fine to me.

          Show
          Sreekanth Ramakrishnan added a comment - The changes look fine to me.
          Hide
          Amar Kamat added a comment -

          Ant tests passed on my box.

          Show
          Amar Kamat added a comment - Ant tests passed on my box.
          Hide
          Hemanth Yamijala added a comment -

          Hmm. I don't fully understand this patch.

          So, what is the requirement with respect to variable substitution in mapred.child.env ? Currently, it looks like unless the key has LD_LIBRARY_PATH, it is not going to do anything useful.

          For e.g. if I have mapred.child.env with the value as "key=$value", where 'value' is in the environment of the Tasktracker, since 'key' is not added to the 'env' variable in the taskrunner method, it will not work, right ? I thought the use case would be that if any thing in the value should be picked from the tasktracker's environment, it should be replaced. Isn't this the use case ?

          Show
          Hemanth Yamijala added a comment - Hmm. I don't fully understand this patch. So, what is the requirement with respect to variable substitution in mapred.child.env ? Currently, it looks like unless the key has LD_LIBRARY_PATH, it is not going to do anything useful. For e.g. if I have mapred.child.env with the value as "key=$value", where 'value' is in the environment of the Tasktracker, since 'key' is not added to the 'env' variable in the taskrunner method, it will not work, right ? I thought the use case would be that if any thing in the value should be picked from the tasktracker's environment, it should be replaced. Isn't this the use case ?
          Hide
          Amar Kamat added a comment -

          Hemanth,
          This is just a bugfix to do with HADOOP-2838. Currently we can pass

          1. x=:/tmp [sets x to :/tmp]
          2. x=$x:/tmp [appends :/tmp to x]

          We can support x=$y:/tmp but I didnt have any usecase for that. Hemanth, if you think thats a valid usecase then plz file a jira. Was this your concern?

          Show
          Amar Kamat added a comment - Hemanth, This is just a bugfix to do with HADOOP-2838 . Currently we can pass x=:/tmp [sets x to :/tmp] x=$x:/tmp [appends :/tmp to x] We can support x=$y:/tmp but I didnt have any usecase for that. Hemanth, if you think thats a valid usecase then plz file a jira. Was this your concern?
          Hide
          Amar Kamat added a comment -

          Attaching a new patch that

          1. inherits the tastracker's env variable value for substitution
          2. fixes the substitution code
          3. fixes the testcase
          4. adds meaningful diagnostic information upon faulty env

          Result of test-patch
          [exec] +1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 3 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

          Tested this patch on a single node cluster and works as expected. Here is what I did

          No Param Output
          1 A Error seen on webui and job fails
          2 A=B Child env has a parameter A set to B
          2 A=$A:B Child env has a parameter A set to :B
          2 LD_LIBRARY_PATH=$LD_LIBRARY_PATH:B Child env has B appended to the modified LD (work dir)
          2 PATH=$PATH:B Child env has PATH inherited from tt and appended B

          Running ant test now

          Show
          Amar Kamat added a comment - Attaching a new patch that inherits the tastracker's env variable value for substitution fixes the substitution code fixes the testcase adds meaningful diagnostic information upon faulty env Result of test-patch [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. Tested this patch on a single node cluster and works as expected. Here is what I did No Param Output 1 A Error seen on webui and job fails 2 A=B Child env has a parameter A set to B 2 A=$A:B Child env has a parameter A set to :B 2 LD_LIBRARY_PATH=$LD_LIBRARY_PATH:B Child env has B appended to the modified LD (work dir) 2 PATH=$PATH:B Child env has PATH inherited from tt and appended B Running ant test now
          Hide
          Sreekanth Ramakrishnan added a comment -

          Changes look fine with me, with respect to the blocked issue HADOOP-5980, verified the fix with respect to LinuxTaskController

          Show
          Sreekanth Ramakrishnan added a comment - Changes look fine with me, with respect to the blocked issue HADOOP-5980 , verified the fix with respect to LinuxTaskController
          Hide
          Amar Kamat added a comment -

          Ant tests passed on my box.

          Show
          Amar Kamat added a comment - Ant tests passed on my box.
          Hide
          Sharad Agarwal added a comment -

          I just committed this. Thanks Amar!

          Show
          Sharad Agarwal added a comment - I just committed this. Thanks Amar!
          Hide
          Amar Kamat added a comment -

          Attaching an example patch for 20 branch (not to be committed).

          Show
          Amar Kamat added a comment - Attaching an example patch for 20 branch (not to be committed).
          Hide
          Robert Chansler added a comment -

          Editorial pass over all release notes prior to publication of 0.21.

          Show
          Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21.

            People

            • Assignee:
              Amar Kamat
              Reporter:
              Amar Kamat
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development